iommufd: Add vIOMMU infrastructure (Part-4 HW QUEUE)
Subsystems (8)
v9 cover iommufd: Add vIOMMU infrastructure (Part-4 HW QUEUE)
The vIOMMU object is designed to represent a slice of an IOMMU HW for its virtualization features shared with or passed to user space (a VM mostly) in a way of HW acceleration. This extended the HWPT-based design for more advanced virtualization feature.
HW QUEUE introduced by this series as a part of the vIOMMU infrastructure represents a HW accelerated queue/buffer for VM to use exclusively, e.g.
- NVIDIA's Virtual Command Queue
- AMD vIOMMU's Command Buffer, Event Log Buffer, and PPR Log Buffer
each of which allows its IOMMU HW to directly access a queue memory owned by a guest VM and allows a guest OS to control the HW queue direclty, to avoid VM Exit overheads to improve the performance.
Introduce IOMMUFD_OBJ_HW_QUEUE and its pairing IOMMUFD_CMD_HW_QUEUE_ALLOC allowing VMM to forward the IOMMU-specific queue info, such as queue base address, size, and etc.
Meanwhile, a guest-owned queue needs the guest kernel to control the queue by reading/writing its consumer and producer indexes, via MMIO acceses to the hardware MMIO registers. Introduce an mmap infrastructure for iommufd to support passing through a piece of MMIO region from the host physical address space to the guest physical address space. The mmap info (offset/ length) used by an mmap syscall must be pre-allocated and returned to the user space via an output driver-data during an IOMMUFD_CMD_HW_QUEUE_ALLOC call. Thus, it requires a driver-specific user data support in the vIOMMU allocation flow.
As a real-world use case, this series implements a HW QUEUE support in the tegra241-cmdqv driver for VCMDQs on NVIDIA Grace CPU. In another word, it is also the Tegra CMDQV series Part-2 (user-space support), reworked from Previous RFCv1:
https://lore.kernel.org/all/[email protected]/
This enables the HW accelerated feature for NVIDIA Grace CPU. Compared to the standard SMMUv3 operating in the nested translation mode trapping CMDQ for TLBI and ATC_INV commands, this gives a huge performance improvement:
70% to 90% reductions of invalidation time were measured by various DMA unmap tests running in a guest OS.
// Unmap latencies from "dma_map_benchmark -g @granule -t @threads", // by toggling "/sys/kernel/debug/iommu/tegra241_cmdqv/bypass_vcmdq" @granule | @threads | bypass_vcmdq=1 | bypass_vcmdq=0
4KB 1 35.7 us 5.3 us
16KB 1 41.8 us 6.8 us
64KB 1 68.9 us 9.9 us
128KB 1 109.0 us 12.6 us
256KB 1 187.1 us 18.0 us
4KB 2 96.9 us 6.8 us
16KB 2 97.8 us 7.5 us
64KB 2 151.5 us 10.7 us
128KB 2 257.8 us 12.7 us
256KB 2 443.0 us 17.9 usThis is on Github:
https://github.com/nicolinc/iommufd/commits/iommufd_hw_queue-v9
Paring QEMU branch for testing (reusing v8):
https://github.com/nicolinc/qemu/commits/wip/for_iommufd_hw_queue-v8
Changelog
v9 (attached git-diff v8..v9 at the end of this letter)
* Add Reviewed-by from Vasant and Jason
* [iommufd] Fix offset calculation
* [iommufd] Add unaligned iova/length selftest coverage for hw_queue
* [iommufd] Pass in aligned iova/length to iommufd_access_pin_pages()
* [smmu] Change "u32 *type" at arm_smmu_hw_info() in the header
v8
https://lore.kernel.org/all/[email protected]/
* Add Reviewed-by from Pranj, Kevin and Jason
* Improve kdoc and comments
* [iommufd] Skip selftest for no_viommu variants
* [iommufd] Add unmap coverage for non internal area
* [iommufd] Skip the first page when mtree_alloc_range()
* [iommufd] Correct the passed in index to mtree_erase()
* [iommufd] Correct variable types in iommufd_hw_queue_alloc_phys()
* [iommufd] Reject iopt_unmap_iova_range() if area->num_locks is set
* [tegra] Rename "SID replacement" with "SID mapping"
* [tegra] Unwrap useless _tegra241_vcmdq_hw_init helper
v7
https://lore.kernel.org/all/[email protected]/
* Rebased on Jason's for-next tree (iommufd_hw_queue-prep series)
* Add Reviewed-by from Baolu, Jason, Pranjal
* Update kdocs and notes
* [iommu] Replace "u32" with "enum iommu_hw_info_type"
* [iommufd] Rename vdev->id to vdev->virt_id
* [iommufd] Replace macros with inline helpers
* [iommufd] Report unmapped_bytes in error path
* [iommufd] Add iommufd_access_is_internal helper
* [iommufd] Do not drop ops->unmap check for mdevs
* [iommufd] Store physical addresses in immap structure
* [iommufd] Reorder access and hw_queue object allocations
* [iommufd] Scan for an internal access before any unmap call
* [iommufd] Drop unused ictx pointer in struct iommufd_hw_queue
* [iommufd] Use kcalloc to avoid failure due to memory fragmentation
* [tegra] Use "else"
* [tegra] Lock destroy() using lvcmdq_mutex
v6
https://lore.kernel.org/all/[email protected]/
* Rebase on iommufd_hw_queue-prep-v2
* Add Reviewed-by from Kevin and Jason
* [iommufd] Update kdocs and notes
* [iommufd] Drop redundant pages[i] check
* [iommufd] Allow nesting_parent_iova to be 0
* [iommufd] Add iommufd_hw_queue_alloc_phys()
* [iommufd] Revise iommufd_viommu_alloc/destroy_mmap APIs
* [iommufd] Move destroy ops to vdevice/hw_queue structures
* [iommufd] Add union in hw_info struct to share out_data_type field
* [iommufd] Replace iopt_pin/unpin_pages() with internal access APIs
* [iommufd] Replace vdevice_alloc with vdevice_size and vdevice_init
* [iommufd] Replace hw_queue_alloc with get_hw_queue_size/hw_queue_init
* [iommufd] Replace IOMMUFD_VIOMMU_FLAG_HW_QUEUE_READS_PA with init_phys
* [smmu] Drop arm_smmu_domain_ipa_to_pa
* [smmu] Update arm_smmu_impl_ops changes for vsmmu_init
* [tegra] Add a vdev_to_vsid macro
* [tegra] Add lvcmdq_mutex to protect multi queues
* [tegra] Drop duplicated kcalloc for vintf->lvcmdqs (memory leak)
v5
https://lore.kernel.org/all/[email protected]/
* Rebase on v6.15-rc6
* Add Reviewed-by from Jason and Kevin
* Correct typos in kdoc and update commit logs
* [iommufd] Add a cosmetic fix
* [iommufd] Drop unused num_pfns
* [iommufd] Drop unnecessary check
* [iommufd] Reorder patch sequence
* [iommufd] Use io_remap_pfn_range()
* [iommufd] Use success oriented flow
* [iommufd] Fix max_npages calculation
* [iommufd] Add more selftest coverage
* [iommufd] Drop redundant static_assert
* [iommufd] Fix mmap pfn range validation
* [iommufd] Reject unmap on pinned iovas
* [iommufd] Drop redundant vm_flags_set()
* [iommufd] Drop iommufd_struct_destroy()
* [iommufd] Drop redundant queue iova test
* [iommufd] Use "mmio_addr" and "mmio_pfn"
* [iommufd] Rename to "nesting_parent_iova"
* [iommufd] Make iopt_pin_pages call option
* [iommufd] Add ictx comparison in depend()
* [iommufd] Add iommufd_object_alloc_ucmd()
* [iommufd] Move kcalloc() after validations
* [iommufd] Replace ictx setting with WARN_ON
* [iommufd] Make hw_info's type bidirectional
* [smmu] Add supported_vsmmu_type in impl_ops
* [smmu] Drop impl report in smmu vendor struct
* [tegra] Add IOMMU_HW_INFO_TYPE_TEGRA241_CMDQV
* [tegra] Replace "number of VINTFs" with a note
* [tegra] Drop the redundant lvcmdq pointer setting
* [tegra] Flag IOMMUFD_VIOMMU_FLAG_HW_QUEUE_READS_PA
* [tegra] Use "vintf_alloc_vsid" for vdevice_alloc op
v4
https://lore.kernel.org/all/[email protected]/
* Rebase on v6.15-rc5
* Add Reviewed-by from Vasant
* Rename "vQUEUE" to "HW QUEUE"
* Use "offset" and "length" for all mmap-related variables
* [iommufd] Use u64 for guest PA
* [iommufd] Fix typo in uAPI doc
* [iommufd] Rename immap_id to offset
* [iommufd] Drop the partial-size mmap support
* [iommufd] Do not replace WARN_ON with WARN_ON_ONCE
* [iommufd] Use "u64 base_addr" for queue base address
* [iommufd] Use u64 base_pfn/num_pfns for immap structure
* [iommufd] Correct the size passed in to mtree_alloc_range()
* [iommufd] Add IOMMUFD_VIOMMU_FLAG_HW_QUEUE_READS_PA to viommu_ops
v3
https://lore.kernel.org/all/[email protected]/
* Add Reviewed-by from Baolu, Pranjal, and Alok
* Revise kdocs, uAPI docs, and commit logs
* Rename "vCMDQ" back to "vQUEUE" for AMD cases
* [tegra] Add tegra241_vcmdq_hw_flush_timeout()
* [tegra] Rename vsmmu_alloc to alloc_vintf_user
* [tegra] Use writel for SID replacement registers
* [tegra] Move mmap removal call to vsmmu_destroy op
* [tegra] Fix revert in tegra241_vintf_alloc_lvcmdq_user()
* [iommufd] Replace "& ~PAGE_MASK" with PAGE_ALIGNED()
* [iommufd] Add an object-type "owner" to immap structure
* [iommufd] Drop the ictx input in the new for-driver APIs
* [iommufd] Add iommufd_vma_ops to keep track of mmap lifecycle
* [iommufd] Add viommu-based iommufd_viommu_alloc/destroy_mmap helpers
* [iommufd] Rename iommufd_ctx_alloc/free_mmap to _iommufd_alloc/destroy_mmap
v2
https://lore.kernel.org/all/[email protected]/
* Add Reviewed-by from Jason
* [smmu] Fix vsmmu initial value
* [smmu] Support impl for hw_info
* [tegra] Rename "slot" to "vsid"
* [tegra] Update kdocs and commit logs
* [tegra] Map/unmap LVCMDQ dynamically
* [tegra] Refcount the previous LVCMDQ
* [tegra] Return -EEXIST if LVCMDQ exists
* [tegra] Simplify VINTF cleanup routine
* [tegra] Use vmid and s2_domain in vsmmu
* [tegra] Rename "mmap_pgoff" to "immap_id"
* [tegra] Add more addr and length validation
* [iommufd] Add more narrative to mmap's kdoc
* [iommufd] Add iommufd_struct_depend/undepend()
* [iommufd] Rename vcmdq_free op to vcmdq_destroy
* [iommufd] Fix bug in iommu_copy_struct_to_user()
* [iommufd] Drop is_io from iommufd_ctx_alloc_mmap()
* [iommufd] Test the queue memory for its contiguity
* [iommufd] Return -ENXIO if address or length fails
* [iommufd] Do not change @min_last in mock_viommu_alloc()
* [iommufd] Generalize TEGRA241_VCMDQ data in core structure
* [iommufd] Add selftest coverage for IOMMUFD_CMD_VCMDQ_ALLOC
* [iommufd] Add iopt_pin_pages() to prevent queue memory from unmapping
v1
https://lore.kernel.org/all/[email protected]/
Thanks
Nicolin
Nicolin Chen (29):
iommufd: Report unmapped bytes in the error path of
iopt_unmap_iova_range
iommufd: Correct virt_id kdoc at struct iommu_vdevice_alloc iommufd/viommu: Explicitly define vdev->virt_id
iommu: Use enum iommu_hw_info_type for type in hw_info op
iommu: Add iommu_copy_struct_to_user helper
iommu: Pass in a driver-level user data structure to viommu_init op iommufd/viommu: Allow driver-specific user data for a vIOMMU object iommufd/selftest: Support user_data in mock_viommu_alloc iommufd/selftest: Add coverage for viommu data iommufd/access: Add internal APIs for HW queue to use iommufd/access: Bypass access->ops->unmap for internal use iommufd/viommu: Add driver-defined vDEVICE support iommufd/viommu: Introduce IOMMUFD_OBJ_HW_QUEUE and its related struct iommufd/viommu: Add IOMMUFD_CMD_HW_QUEUE_ALLOC ioctl iommufd/driver: Add iommufd_hw_queue_depend/undepend() helpers iommufd/selftest: Add coverage for IOMMUFD_CMD_HW_QUEUE_ALLOC
iommufd: Add mmap interface
iommufd/selftest: Add coverage for the new mmap interface
Documentation: userspace-api: iommufd: Update HW QUEUE
iommu: Allow an input type in hw_info op
iommufd: Allow an input data_type via iommu_hw_info iommufd/selftest: Update hw_info coverage for an input data_type iommu/arm-smmu-v3-iommufd: Add vsmmu_size/type and vsmmu_init impl ops iommu/arm-smmu-v3-iommufd: Add hw_info to impl_ops iommu/tegra241-cmdqv: Use request_threaded_irq iommu/tegra241-cmdqv: Simplify deinit flow in
tegra241_cmdqv_remove_vintf()
iommu/tegra241-cmdqv: Do not statically map LVCMDQs iommu/tegra241-cmdqv: Add user-space use support iommu/tegra241-cmdqv: Add IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV support
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 25 +- drivers/iommu/iommufd/io_pagetable.h | 5 +- drivers/iommu/iommufd/iommufd_private.h | 46 +- drivers/iommu/iommufd/iommufd_test.h | 20 + include/linux/iommu.h | 50 +- include/linux/iommufd.h | 160 ++++++ include/uapi/linux/iommufd.h | 147 +++++- tools/testing/selftests/iommu/iommufd_utils.h | 89 +++- .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 28 +- .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 477 +++++++++++++++++- drivers/iommu/intel/iommu.c | 7 +- drivers/iommu/iommufd/device.c | 87 +++- drivers/iommu/iommufd/driver.c | 82 ++- drivers/iommu/iommufd/io_pagetable.c | 13 +- drivers/iommu/iommufd/main.c | 69 +++ drivers/iommu/iommufd/pages.c | 12 +- drivers/iommu/iommufd/selftest.c | 153 +++++- drivers/iommu/iommufd/viommu.c | 218 +++++++- tools/testing/selftests/iommu/iommufd.c | 141 +++++- .../selftests/iommu/iommufd_fail_nth.c | 15 +- Documentation/userspace-api/iommufd.rst | 12 + 21 files changed, 1745 insertions(+), 111 deletions(-)
Signature · 1 line
2.43.0
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +2 -1
@@ -1045,7 +1045,8 @@ struct arm_vsmmu { };#if IS_ENABLED (CONFIG_ARM_SMMU_V3_IOMMUFD ) -void *arm_smmu_hw_info (struct device *dev,u32 *length,u32 *type); +void *arm_smmu_hw_info (struct device *dev,u32 *length, +enum iommu_hw_info_type *type);size_t arm_smmu_get_viommu_size (struct device *dev,enum iommu_viommu_type viommu_type); int arm_vsmmu_init(struct iommufd_viommu *viommu,
drivers/iommu/iommufd/viommu.c +13 -10
@@ -206,7 +206,11 @@ static void iommufd_hw_queue_destroy_access(struct iommufd_ctx *ictx,struct iommufd_access *access, u64 base_iova, size_t length) { -iommufd_access_unpin_pages (access ,base_iova ,length ); + u64 aligned_iova = PAGE_ALIGN_DOWN(base_iova); +u64 offset = base_iova - aligned_iova; + +iommufd_access_unpin_pages (access, aligned_iova, +PAGE_ALIGN (length + offset));iommufd_access_detach_internal (access);iommufd_access_destroy_internal (ictx, access); }
@@ -239,22 +243,23 @@ static struct iommufd_access * iommufd_hw_queue_alloc_phys(struct iommu_hw_queue_alloc *cmd, struct iommufd_viommu *viommu, phys_addr_t *base_pa) { + u64 aligned_iova = PAGE_ALIGN_DOWN(cmd->nesting_parent_iova); +u64 offset = cmd->nesting_parent_iova - aligned_iova;struct iommufd_access *access;struct page **pages;size_t max_npages;size_t length; -u64 offset;size_t i;int rc; - offset = - cmd->nesting_parent_iova -PAGE_ALIGN (cmd->nesting_parent_iova); -/* DIV_ROUND_UP(offset + cmd->length, PAGE_SIZE) */ +/* max_npages = DIV_ROUND_UP(offset + cmd->length, PAGE_SIZE) */ if (check_add_overflow (offset, cmd->length, &length))return ERR_PTR (-ERANGE );if (check_add_overflow (length,PAGE_SIZE -1 , &length))return ERR_PTR (-ERANGE ); max_npages = length /PAGE_SIZE ; +/* length needs to be page aligned too */ + length = max_npages *PAGE_SIZE ;/* * Use kvcalloc() to avoid memory fragmentation for a large page array.
@@ -274,8 +279,7 @@ iommufd_hw_queue_alloc_phys(struct iommu_hw_queue_alloc *cmd,if (rc) goto out_destroy; - rc =iommufd_access_pin_pages (access, cmd->nesting_parent_iova, - cmd->length, pages,0 ); + rc =iommufd_access_pin_pages (access, aligned_iova, length, pages,0 );if (rc) goto out_detach;
@@ -287,13 +291,12 @@ iommufd_hw_queue_alloc_phys(struct iommu_hw_queue_alloc *cmd, goto out_unpin; } - *base_pa =page_to_pfn (pages[0 ]) <<PAGE_SHIFT ; + *base_pa = (page_to_pfn (pages[0 ]) <<PAGE_SHIFT ) + offset;kfree (pages);return access; out_unpin: -iommufd_access_unpin_pages (access, cmd->nesting_parent_iova, - cmd->length); +iommufd_access_unpin_pages (access, aligned_iova, length); out_detach:iommufd_access_detach_internal (access); out_destroy:
tools/testing/selftests/iommu/iommufd.c +5 -4
@@ -3104,17 +3104,18 @@ TEST_F(iommufd_viommu, hw_queue)/* Allocate index=0, declare ownership of the iova */ test_cmd_hw_queue_alloc (viommu_id,IOMMU_HW_QUEUE_TYPE_SELFTEST ,0 , iova,PAGE_SIZE , &hw_queue_id[0 ]); -/* Fail duplicate */ +/* Fail duplicated index */ test_err_hw_queue_alloc (EEXIST , viommu_id,IOMMU_HW_QUEUE_TYPE_SELFTEST ,0 , iova,PAGE_SIZE , &hw_queue_id[0 ]);/* Fail unmap, due to iova ownership */ test_err_ioctl_ioas_unmap (EBUSY , iova,PAGE_SIZE );/* The 2nd page is not pinned, so it can be unmmap */ -test_ioctl_ioas_unmap (iova +PAGE_SIZE ,PAGE_SIZE ); +test_ioctl_ioas_unmap (iova2,PAGE_SIZE ); -/* Allocate index=1 */ +/* Allocate index=1, with an unaligned case */ test_cmd_hw_queue_alloc (viommu_id,IOMMU_HW_QUEUE_TYPE_SELFTEST ,1 , - iova,PAGE_SIZE , &hw_queue_id[1 ]); + iova +PAGE_SIZE /2 ,PAGE_SIZE /2 , + &hw_queue_id[1 ]);/* Fail to destroy, due to dependency */ EXPECT_ERRNO (EBUSY ,_test_ioctl_destroy (self->fd, hw_queue_id[0 ]));
PATCH v9 1/29 iommufd: Report unmapped bytes in the error path of iopt_unmap_iova_range
There are callers that read the unmapped bytes even when rc != 0. Thus, do not forget to report it in the error path too.
--- drivers/iommu/iommufd/io_pagetable.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/iommu/iommufd/io_pagetable.c +5 -2
@@ -743,8 +743,10 @@ static int iopt_unmap_iova_range(struct io_pagetable *iopt, unsigned long start,iommufd_access_notify_unmap (iopt, area_first, length);/* Something is not responding to unmap requests. */ tries++; -if (WARN_ON (tries >100 )) -return -EDEADLOCK ; +if (WARN_ON (tries >100 )) { + rc = -EDEADLOCK ; + goto out_unmapped; + } goto again; }
@@ -766,6 +768,7 @@ static int iopt_unmap_iova_range(struct io_pagetable *iopt, unsigned long start, out_unlock_iova:up_write (&iopt->iova_rwsem);up_read (&iopt->domains_rwsem); +out_unmapped:if (unmapped) *unmapped = unmapped_bytes;return rc;
Signature · 1 line
2.43.0
PATCH v9 2/29 iommufd: Correct virt_id kdoc at struct iommu_vdevice_alloc
In reply to Nicolin ChenThe userspace-api iommufd.rst has described it correctly but the uAPI doc was remained uncorrected. Thus, fix it.
--- include/uapi/linux/iommufd.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
include/uapi/linux/iommufd.h +1 -1
@@ -995,7 +995,7 @@ struct iommu_viommu_alloc {* @dev_id: The physical device to allocate a virtual instance on the vIOMMU * @out_vdevice_id: Object handle for the vDevice. Pass to IOMMU_DESTORY * @virt_id: Virtual device ID per vIOMMU, e.g. vSID of ARM SMMUv3, vDeviceID -* of AMD IOMMU, and vRID of a nested Intel VT-d to a Context Table +* of AMD IOMMU, and vRID of Intel VT-d * * Allocate a virtual device instance (for a physical device) against a vIOMMU. * This instance holds the device's information (related to its vIOMMU) in a VM.
Signature · 1 line
2.43.0
PATCH v9 3/29 iommufd/viommu: Explicitly define vdev->virt_id
In reply to Nicolin ChenThe "id" is too general to get its meaning easily. Rename it explicitly to "virt_id" and update the kdocs for readability. No functional changes.
--- drivers/iommu/iommufd/iommufd_private.h | 7 ++++++- drivers/iommu/iommufd/driver.c | 2 +- drivers/iommu/iommufd/viommu.c | 4 ++-- 3 files changed, 9 insertions(+), 4 deletions(-)
drivers/iommu/iommufd/iommufd_private.h +6 -1
@@ -634,7 +634,12 @@ struct iommufd_vdevice {struct iommufd_object obj;struct iommufd_viommu *viommu;struct device *dev; -u64 id;/* per-vIOMMU virtual ID */ + +/* +* Virtual device ID per vIOMMU, e.g. vSID of ARM SMMUv3, vDeviceID of +* AMD IOMMU, and vRID of Intel VT-d +*/ +u64 virt_id; };#ifdef CONFIG_IOMMUFD_TEST
drivers/iommu/iommufd/driver.c +1 -1
@@ -30,7 +30,7 @@ int iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,xa_lock (&viommu->vdevs);xa_for_each (&viommu->vdevs, index, vdev) {if (vdev->dev == dev) { - *vdev_id = vdev->id; + *vdev_id = vdev->virt_id; rc =0 ;break ; }
drivers/iommu/iommufd/viommu.c +2 -2
@@ -111,7 +111,7 @@ void iommufd_vdevice_destroy(struct iommufd_object *obj)struct iommufd_viommu *viommu = vdev->viommu;/* xa_cmpxchg is okay to fail if alloc failed xa_cmpxchg previously */ -xa_cmpxchg (&viommu->vdevs, vdev->id, vdev,NULL ,GFP_KERNEL ); +xa_cmpxchg (&viommu->vdevs, vdev->virt_id, vdev,NULL ,GFP_KERNEL );refcount_dec (&viommu->obj.users);put_device (vdev->dev); }
@@ -150,7 +150,7 @@ int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_idev; } - vdev->id = virt_id; + vdev->virt_id = virt_id; vdev->dev = idev->dev;get_device (idev->dev); vdev->viommu = viommu;
Signature · 1 line
2.43.0
PATCH v9 4/29 iommu: Use enum iommu_hw_info_type for type in hw_info op
In reply to Nicolin ChenReplace u32 to make it clear. No functional changes.
Also simplify the kdoc since the type itself is clear enough.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 ++- include/linux/iommu.h | 6 +++--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 3 ++- drivers/iommu/intel/iommu.c | 3 ++- drivers/iommu/iommufd/selftest.c | 3 ++- 5 files changed, 11 insertions(+), 7 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +2 -1
@@ -1033,7 +1033,8 @@ struct arm_vsmmu { };#if IS_ENABLED (CONFIG_ARM_SMMU_V3_IOMMUFD ) -void *arm_smmu_hw_info (struct device *dev,u32 *length,u32 *type); +void *arm_smmu_hw_info (struct device *dev,u32 *length, +enum iommu_hw_info_type *type);size_t arm_smmu_get_viommu_size (struct device *dev,enum iommu_viommu_type viommu_type); int arm_vsmmu_init(struct iommufd_viommu *viommu,
include/linux/iommu.h +3 -3
@@ -563,8 +563,7 @@ iommu_copy_struct_from_full_user_array(void *kdst, size_t kdst_entry_size,* @capable: check capability * @hw_info: report iommu hardware information. The data buffer returned by this * op is allocated in the iommu driver and freed by the caller after -* use. The information type is one of enum iommu_hw_info_type defined -* in include/uapi/linux/iommufd.h. +* use. * @domain_alloc: Do not use in new drivers * @domain_alloc_identity: allocate an IDENTITY domain. Drivers should prefer to * use identity_domain instead. This should only be used
@@ -623,7 +622,8 @@ iommu_copy_struct_from_full_user_array(void *kdst, size_t kdst_entry_size,*/ struct iommu_ops {bool (*capable)(struct device *dev,enum iommu_cap ); -void *(*hw_info)(struct device *dev,u32 *length,u32 *type); +void *(*hw_info)(struct device *dev,u32 *length, +enum iommu_hw_info_type *type);/* Domain allocation and freeing by the iommu driver */ #if IS_ENABLED (CONFIG_FSL_PAMU )
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +2 -1
@@ -7,7 +7,8 @@#include "arm-smmu-v3.h" -void *arm_smmu_hw_info (struct device *dev,u32 *length,u32 *type) +void *arm_smmu_hw_info (struct device *dev,u32 *length, +enum iommu_hw_info_type *type) {struct arm_smmu_master *master =dev_iommu_priv_get (dev);struct iommu_hw_info_arm_smmuv3 *info;
drivers/iommu/intel/iommu.c +2 -1
@@ -4091,7 +4091,8 @@ static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,return ret; } -static void *intel_iommu_hw_info (struct device *dev,u32 *length,u32 *type) +static void *intel_iommu_hw_info (struct device *dev,u32 *length, +enum iommu_hw_info_type *type) {struct device_domain_info *info =dev_iommu_priv_get (dev);struct intel_iommu *iommu = info->iommu;
drivers/iommu/iommufd/selftest.c +2 -1
@@ -287,7 +287,8 @@ static struct iommu_domain mock_blocking_domain = { .ops = &mock_blocking_ops, }; -static void *mock_domain_hw_info (struct device *dev,u32 *length,u32 *type) +static void *mock_domain_hw_info (struct device *dev,u32 *length, +enum iommu_hw_info_type *type) {struct iommu_test_hw_info *info;
Signature · 1 line
2.43.0
PATCH v9 5/29 iommu: Add iommu_copy_struct_to_user helper
In reply to Nicolin ChenSimilar to the iommu_copy_struct_from_user helper receiving data from the user space, add an iommu_copy_struct_to_user helper to report output data back to the user space data pointer.
--- include/linux/iommu.h | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+)
include/linux/iommu.h +40 -0
@@ -558,6 +558,46 @@ iommu_copy_struct_from_full_user_array(void *kdst, size_t kdst_entry_size,return 0 ; } +/** +* __iommu_copy_struct_to_user - Report iommu driver specific user space data +* @dst_data: Pointer to a struct iommu_user_data for user space data location +* @src_data: Pointer to an iommu driver specific user data that is defined in +* include/uapi/linux/iommufd.h +* @data_type: The data type of the @src_data. Must match with @dst_data.type +* @data_len: Length of current user data structure, i.e. sizeof(struct _src) +* @min_len: Initial length of user data structure for backward compatibility. +* This should be offsetofend using the last member in the user data +* struct that was initially added to include/uapi/linux/iommufd.h +*/ +static inline int +__iommu_copy_struct_to_user (const struct iommu_user_data *dst_data, +void *src_data,unsigned int data_type, +size_t data_len,size_t min_len) +{ +if (WARN_ON (!dst_data || !src_data)) +return -EINVAL ; +if (dst_data->type != data_type) +return -EINVAL ; +if (dst_data->len < min_len || data_len < dst_data->len) +return -EINVAL ; +return copy_struct_to_user (dst_data->uptr, dst_data->len, src_data, + data_len,NULL ); +} + +/** +* iommu_copy_struct_to_user - Report iommu driver specific user space data +* @user_data: Pointer to a struct iommu_user_data for user space data location +* @ksrc: Pointer to an iommu driver specific user data that is defined in +* include/uapi/linux/iommufd.h +* @data_type: The data type of the @ksrc. Must match with @user_data->type +* @min_last: The last member of the data structure @ksrc points in the initial +* version. +* Return 0 for success, otherwise -error. +*/ +#define iommu_copy_struct_to_user (user_data, ksrc, data_type, min_last) \ + __iommu_copy_struct_to_user(user_data, ksrc, data_type, sizeof(*ksrc), \ + offsetofend(typeof(*ksrc), min_last)) +/** * struct iommu_ops - iommu ops and capabilities * @capable: check capability
Signature · 1 line
2.43.0
PATCH v9 6/29 iommu: Pass in a driver-level user data structure to viommu_init op
In reply to Nicolin ChenThe new type of vIOMMU for tegra241-cmdqv allows user space VM to use one of its virtual command queue HW resources exclusively. This requires user space to mmap the corresponding MMIO page from kernel space for direct HW control.
To forward the mmap info (offset and length), iommufd should add a driver specific data structure to the IOMMUFD_CMD_VIOMMU_ALLOC ioctl, for driver to output the info during the vIOMMU initialization back to user space.
Similar to the existing ioctls and their IOMMU handlers, add a user_data to viommu_init op to bridge between iommufd and drivers.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 ++- include/linux/iommu.h | 3 ++- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 3 ++- drivers/iommu/iommufd/selftest.c | 3 ++- drivers/iommu/iommufd/viommu.c | 2 +- 5 files changed, 9 insertions(+), 5 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +2 -1
@@ -1038,7 +1038,8 @@ void *arm_smmu_hw_info(struct device *dev, u32 *length,size_t arm_smmu_get_viommu_size (struct device *dev,enum iommu_viommu_type viommu_type);int arm_vsmmu_init (struct iommufd_viommu *viommu, -struct iommu_domain *parent_domain); +struct iommu_domain *parent_domain, +const struct iommu_user_data *user_data);int arm_smmu_attach_prepare_vmaster (struct arm_smmu_attach_state *state,struct arm_smmu_nested_domain *nested_domain);void arm_smmu_attach_commit_vmaster (struct arm_smmu_attach_state *state);
include/linux/iommu.h +2 -1
@@ -700,7 +700,8 @@ struct iommu_ops {size_t (*get_viommu_size)(struct device *dev,enum iommu_viommu_type viommu_type);int (*viommu_init)(struct iommufd_viommu *viommu, -struct iommu_domain *parent_domain); +struct iommu_domain *parent_domain, +const struct iommu_user_data *user_data);const struct iommu_domain_ops *default_domain_ops;unsigned long pgsize_bitmap;
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +2 -1
@@ -419,7 +419,8 @@ size_t arm_smmu_get_viommu_size(struct device *dev, }int arm_vsmmu_init (struct iommufd_viommu *viommu, -struct iommu_domain *parent_domain) +struct iommu_domain *parent_domain, +const struct iommu_user_data *user_data) { struct arm_vsmmu *vsmmu = container_of(viommu, struct arm_vsmmu, core); struct arm_smmu_device *smmu =
drivers/iommu/iommufd/selftest.c +2 -1
@@ -779,7 +779,8 @@ static size_t mock_get_viommu_size(struct device *dev, }static int mock_viommu_init (struct iommufd_viommu *viommu, -struct iommu_domain *parent_domain) +struct iommu_domain *parent_domain, +const struct iommu_user_data *user_data) {struct mock_iommu_device *mock_iommu =container_of ( viommu->iommu_dev, struct mock_iommu_device, iommu_dev);
drivers/iommu/iommufd/viommu.c +1 -1
@@ -84,7 +84,7 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)*/ viommu->iommu_dev =__iommu_get_iommu_dev (idev->dev); - rc = ops->viommu_init (viommu, hwpt_paging->common.domain); + rc = ops->viommu_init (viommu, hwpt_paging->common.domain,NULL );if (rc) goto out_put_hwpt;
Signature · 1 line
2.43.0
PATCH v9 7/29 iommufd/viommu: Allow driver-specific user data for a vIOMMU object
In reply to Nicolin ChenThe new type of vIOMMU for tegra241-cmdqv driver needs a driver-specific user data. So, add data_len/uptr to the iommu_viommu_alloc uAPI and pass it in via the viommu_init iommu op.
--- include/uapi/linux/iommufd.h | 6 ++++++ drivers/iommu/iommufd/viommu.c | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-)
include/uapi/linux/iommufd.h +6 -0
@@ -965,6 +965,9 @@ enum iommu_viommu_type {* @dev_id: The device's physical IOMMU will be used to back the virtual IOMMU * @hwpt_id: ID of a nesting parent HWPT to associate to * @out_viommu_id: Output virtual IOMMU ID for the allocated object +* @data_len: Length of the type specific data +* @__reserved: Must be 0 +* @data_uptr: User pointer to a driver-specific virtual IOMMU data * * Allocate a virtual IOMMU object, representing the underlying physical IOMMU's * virtualization support that is a security-isolated slice of the real IOMMU HW
@@ -985,6 +988,9 @@ struct iommu_viommu_alloc {__u32 dev_id;__u32 hwpt_id;__u32 out_viommu_id; +__u32 data_len; +__u32 __reserved; +__aligned_u64 data_uptr; };#define IOMMU_VIOMMU_ALLOC _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VIOMMU_ALLOC)
drivers/iommu/iommufd/viommu.c +7 -1
@@ -17,6 +17,11 @@ void iommufd_viommu_destroy(struct iommufd_object *obj)int iommufd_viommu_alloc_ioctl (struct iommufd_ucmd *ucmd) {struct iommu_viommu_alloc *cmd = ucmd->cmd; +const struct iommu_user_data user_data = { + .type = cmd->type, + .uptr =u64_to_user_ptr (cmd->data_uptr), + .len = cmd->data_len, + };struct iommufd_hwpt_paging *hwpt_paging;struct iommufd_viommu *viommu;struct iommufd_device *idev;
@@ -84,7 +89,8 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd)*/ viommu->iommu_dev =__iommu_get_iommu_dev (idev->dev); - rc = ops->viommu_init (viommu, hwpt_paging->common.domain,NULL ); + rc = ops->viommu_init (viommu, hwpt_paging->common.domain, + user_data.len ? &user_data :NULL );if (rc) goto out_put_hwpt;
Signature · 1 line
2.43.0
PATCH v9 8/29 iommufd/selftest: Support user_data in mock_viommu_alloc
In reply to Nicolin ChenAdd a simple user_data for an input-to-output loopback test.
--- drivers/iommu/iommufd/iommufd_test.h | 13 +++++++++++++ drivers/iommu/iommufd/selftest.c | 15 +++++++++++++++ 2 files changed, 28 insertions(+)
drivers/iommu/iommufd/iommufd_test.h +13 -0
@@ -227,6 +227,19 @@ struct iommu_hwpt_invalidate_selftest {#define IOMMU_VIOMMU_TYPE_SELFTEST 0xdeadbeef +/** +* struct iommu_viommu_selftest - vIOMMU data for Mock driver +* (IOMMU_VIOMMU_TYPE_SELFTEST) +* @in_data: Input random data from user space +* @out_data: Output data (matching @in_data) to user space +* +* Simply set @out_data=@in_data for a loopback test +*/ +struct iommu_viommu_selftest { +__u32 in_data; +__u32 out_data; +}; +/* Should not be equal to any defined value in enum iommu_viommu_invalidate_data_type */ #define IOMMU_VIOMMU_INVALIDATE_DATA_SELFTEST 0xdeadbeef#define IOMMU_VIOMMU_INVALIDATE_DATA_SELFTEST_INVALID 0xdadbeef
drivers/iommu/iommufd/selftest.c +15 -0
@@ -784,6 +784,21 @@ static int mock_viommu_init(struct iommufd_viommu *viommu, {struct mock_iommu_device *mock_iommu =container_of ( viommu->iommu_dev, struct mock_iommu_device, iommu_dev); +struct iommu_viommu_selftest data; +int rc; + +if (user_data) { + rc =iommu_copy_struct_from_user ( + &data, user_data,IOMMU_VIOMMU_TYPE_SELFTEST , out_data); +if (rc) +return rc; + + data.out_data = data.in_data; + rc =iommu_copy_struct_to_user ( + user_data, &data,IOMMU_VIOMMU_TYPE_SELFTEST , out_data); +if (rc) +return rc; + }refcount_inc (&mock_iommu->users); viommu->ops = &mock_viommu_ops;
Signature · 1 line
2.43.0
PATCH v9 9/29 iommufd/selftest: Add coverage for viommu data
In reply to Nicolin ChenExtend the existing test_cmd/err_viommu_alloc helpers to accept optional user data. And add a TEST_F for a loopback test.
--- tools/testing/selftests/iommu/iommufd_utils.h | 21 ++++++++----- tools/testing/selftests/iommu/iommufd.c | 31 ++++++++++++++----- .../selftests/iommu/iommufd_fail_nth.c | 5 +-- 3 files changed, 40 insertions(+), 17 deletions(-)
tools/testing/selftests/iommu/iommufd_utils.h +13 -8
@@ -897,7 +897,8 @@ static int _test_cmd_trigger_iopf(int fd, __u32 device_id, __u32 pasid, pasid, fault_fd))static int _test_cmd_viommu_alloc (int fd,__u32 device_id,__u32 hwpt_id, -__u32 type,__u32 flags,__u32 *viommu_id) +__u32 flags,__u32 type,void *data, +__u32 data_len,__u32 *viommu_id) {struct iommu_viommu_alloc cmd = { .size =sizeof (cmd),
@@ -905,6 +906,8 @@ static int _test_cmd_viommu_alloc(int fd, __u32 device_id, __u32 hwpt_id, .type = type, .dev_id = device_id, .hwpt_id = hwpt_id, + .data_uptr = (uint64_t)data, + .data_len = data_len, };int ret;
@@ -916,13 +919,15 @@ static int _test_cmd_viommu_alloc(int fd, __u32 device_id, __u32 hwpt_id,return 0 ; } -#define test_cmd_viommu_alloc (device_id, hwpt_id, type, viommu_id) \ - ASSERT_EQ(0, _test_cmd_viommu_alloc(self->fd, device_id, hwpt_id, \ - type, 0, viommu_id)) -#define test_err_viommu_alloc (_errno, device_id, hwpt_id, type, viommu_id) \ - EXPECT_ERRNO(_errno, \ - _test_cmd_viommu_alloc(self->fd, device_id, hwpt_id, \ - type, 0, viommu_id)) +#define test_cmd_viommu_alloc (device_id, hwpt_id, type, data, data_len, \ + viommu_id) \ + ASSERT_EQ(0, _test_cmd_viommu_alloc(self->fd, device_id, hwpt_id, 0, \ + type, data, data_len, viommu_id)) +#define test_err_viommu_alloc (_errno, device_id, hwpt_id, type, data, \ + data_len, viommu_id) \ + EXPECT_ERRNO(_errno, \ + _test_cmd_viommu_alloc(self->fd, device_id, hwpt_id, 0, \ + type, data, data_len, viommu_id))static int _test_cmd_vdevice_alloc (int fd,__u32 viommu_id,__u32 idev_id,__u64 virt_id,__u32 *vdev_id)
tools/testing/selftests/iommu/iommufd.c +24 -7
@@ -2688,7 +2688,7 @@ FIXTURE_SETUP(iommufd_viommu)/* Allocate a vIOMMU taking refcount of the parent hwpt */ test_cmd_viommu_alloc (self->device_id, self->hwpt_id, -IOMMU_VIOMMU_TYPE_SELFTEST , +IOMMU_VIOMMU_TYPE_SELFTEST ,NULL ,0 , &self->viommu_id);/* Allocate a regular nested hwpt */
@@ -2727,24 +2727,27 @@ TEST_F(iommufd_viommu, viommu_negative_tests)if (self->device_id) {/* Negative test -- invalid hwpt (hwpt_id=0) */ test_err_viommu_alloc (ENOENT , device_id,0 , -IOMMU_VIOMMU_TYPE_SELFTEST ,NULL ); +IOMMU_VIOMMU_TYPE_SELFTEST ,NULL ,0 , +NULL );/* Negative test -- not a nesting parent hwpt */ test_cmd_hwpt_alloc (device_id, ioas_id,0 , &hwpt_id);test_err_viommu_alloc (EINVAL , device_id, hwpt_id, -IOMMU_VIOMMU_TYPE_SELFTEST ,NULL ); +IOMMU_VIOMMU_TYPE_SELFTEST ,NULL ,0 , +NULL );test_ioctl_destroy (hwpt_id);/* Negative test -- unsupported viommu type */ test_err_viommu_alloc (EOPNOTSUPP , device_id, self->hwpt_id, -0xdead ,NULL ); +0xdead ,NULL ,0 ,NULL );EXPECT_ERRNO (EBUSY ,_test_ioctl_destroy (self->fd, self->hwpt_id));EXPECT_ERRNO (EBUSY ,_test_ioctl_destroy (self->fd, self->viommu_id)); }else {test_err_viommu_alloc (ENOENT , self->device_id, self->hwpt_id, -IOMMU_VIOMMU_TYPE_SELFTEST ,NULL ); +IOMMU_VIOMMU_TYPE_SELFTEST ,NULL ,0 , +NULL ); } }
@@ -2791,6 +2794,21 @@ TEST_F(iommufd_viommu, viommu_alloc_nested_iopf) } } +TEST_F(iommufd_viommu, viommu_alloc_with_data) +{ +struct iommu_viommu_selftest data = { + .in_data =0xbeef , + }; + +if (!self->device_id) +SKIP (return,"Skipping test for variant no_viommu" ); + +test_cmd_viommu_alloc (self->device_id, self->hwpt_id, +IOMMU_VIOMMU_TYPE_SELFTEST , &data,sizeof (data), + &self->viommu_id); +ASSERT_EQ (data.out_data, data.in_data); +} + TEST_F(iommufd_viommu, vdevice_alloc) {uint32_t viommu_id = self->viommu_id;
@@ -3105,8 +3123,7 @@ TEST_F(iommufd_device_pasid, pasid_attach)/* Allocate a regular nested hwpt based on viommu */ test_cmd_viommu_alloc (self->device_id, parent_hwpt_id, -IOMMU_VIOMMU_TYPE_SELFTEST , - &viommu_id); +IOMMU_VIOMMU_TYPE_SELFTEST ,NULL ,0 , &viommu_id); test_cmd_hwpt_alloc_nested(self->device_id, viommu_id, IOMMU_HWPT_ALLOC_PASID, &nested_hwpt_id[2],
tools/testing/selftests/iommu/iommufd_fail_nth.c +3 -2
@@ -688,8 +688,9 @@ TEST_FAIL_NTH(basic_fail_nth, device) IOMMU_HWPT_DATA_NONE, 0, 0))return -1 ; -if (_test_cmd_viommu_alloc (self->fd, idev_id, hwpt_id, -IOMMU_VIOMMU_TYPE_SELFTEST ,0 , &viommu_id)) +if (_test_cmd_viommu_alloc (self->fd, idev_id, hwpt_id,0 , +IOMMU_VIOMMU_TYPE_SELFTEST ,NULL ,0 , + &viommu_id))return -1 ;if (_test_cmd_vdevice_alloc (self->fd, viommu_id, idev_id,0 , &vdev_id))
Signature · 1 line
2.43.0
PATCH v9 10/29 iommufd/access: Add internal APIs for HW queue to use
In reply to Nicolin ChenThe new HW queue object, as an internal iommufd object, wants to reuse the struct iommufd_access to pin some iova range in the iopt.
However, an access generally takes the refcount of an ictx. So, in such an internal case, a deadlock could happen when the release of the ictx has to wait for the release of the access first when releasing a hw_queue object, which could wait for the release of the ictx that is refcounted:
ictx --releases--> hw_queue --releases--> access
^ |
|_________________releases________________vTo address this, add a set of lightweight internal APIs to unlink the ictx and the access, i.e. no ictx refcounting by the access:
ictx --releases--> hw_queue --releases--> access
Then, there's no point in setting the access->ictx. So simply define !ictx as an flag for an internal use and add an inline helper.
--- drivers/iommu/iommufd/iommufd_private.h | 23 ++++++++++ drivers/iommu/iommufd/device.c | 59 +++++++++++++++++++++---- 2 files changed, 73 insertions(+), 9 deletions(-)
drivers/iommu/iommufd/iommufd_private.h +23 -0
@@ -484,6 +484,29 @@ void iopt_remove_access(struct io_pagetable *iopt, struct iommufd_access *access, u32 iopt_access_list_id);void iommufd_access_destroy_object (struct iommufd_object *obj); +/* iommufd_access for internal use */ +static inline bool iommufd_access_is_internal (struct iommufd_access *access) +{ +return !access->ictx; +} + +struct iommufd_access *iommufd_access_create_internal (struct iommufd_ctx *ictx); + +static inline void +iommufd_access_destroy_internal (struct iommufd_ctx *ictx, +struct iommufd_access *access) +{ +iommufd_object_destroy_user (ictx, &access->obj); +} + +int iommufd_access_attach_internal (struct iommufd_access *access, +struct iommufd_ioas *ioas); + +static inline void iommufd_access_detach_internal (struct iommufd_access *access) +{ +iommufd_access_detach (access); +} +struct iommufd_eventq {struct iommufd_object obj;struct iommufd_ctx *ictx;
drivers/iommu/iommufd/device.c +50 -9
87 lines not on this page · raw message
Signature · 1 line
2.43.0
PATCH v9 11/29 iommufd/access: Bypass access->ops->unmap for internal use
In reply to Nicolin ChenThe access object has been used externally by VFIO mdev devices, allowing them to pin/unpin physical pages (via needs_pin_pages). Meanwhile, a racy unmap can occur in this case, so these devices usually implement an unmap handler, invoked by iommufd_access_notify_unmap().
The new HW queue object will need the same pin/unpin feature, although it (unlike the mdev case) wants to reject any unmap attempt, during its life cycle. Instead, it would not implement an unmap handler. Thus, bypass any access->ops->unmap access call when the access is marked as internal.
Also, an area being pinned by an internal access should reject any unmap request. This cannot be done inside iommufd_access_notify_unmap() as it's a per-iopt action. Add a "num_locks" counter in the struct iopt_area, set that in iopt_area_add_access() when the caller is an internal access.
--- drivers/iommu/iommufd/io_pagetable.h | 5 +++-- drivers/iommu/iommufd/device.c | 18 ++++++++++++------ drivers/iommu/iommufd/io_pagetable.c | 6 ++++++ drivers/iommu/iommufd/pages.c | 12 ++++++++++-- 4 files changed, 31 insertions(+), 10 deletions(-)
drivers/iommu/iommufd/io_pagetable.h +3 -2
@@ -48,6 +48,7 @@ struct iopt_area {int iommu_prot; bool prevent_access : 1;unsigned int num_accesses; +unsigned int num_locks; };struct iopt_allowed {
@@ -238,9 +239,9 @@ void iopt_pages_unfill_xarray(struct iopt_pages *pages, unsigned long start,int iopt_area_add_access (struct iopt_area *area,unsigned long start,unsigned long last,struct page **out_pages, -unsigned int flags); +unsigned int flags,bool lock_area);void iopt_area_remove_access (struct iopt_area *area,unsigned long start, -unsigned long last); +unsigned long last,bool unlock_area);int iopt_pages_rw_access (struct iopt_pages *pages,unsigned long start_byte,void *data,unsigned long length,unsigned int flags);
drivers/iommu/iommufd/device.c +12 -6
@@ -1048,7 +1048,7 @@ static int iommufd_access_change_ioas(struct iommufd_access *access, }if (cur_ioas) { -if (access->ops->unmap) { +if (!iommufd_access_is_internal (access) && access->ops->unmap) {mutex_unlock (&access->ioas_lock); access->ops->unmap (access->data,0 ,ULONG_MAX );mutex_lock (&access->ioas_lock);
@@ -1255,7 +1255,8 @@ void iommufd_access_notify_unmap(struct io_pagetable *iopt, unsigned long iova,xa_lock (&ioas->iopt.access_list);xa_for_each (&ioas->iopt.access_list, index, access) { -if (!iommufd_lock_obj (&access->obj)) +if (!iommufd_lock_obj (&access->obj) || +iommufd_access_is_internal (access))continue ;xa_unlock (&ioas->iopt.access_list);
@@ -1279,6 +1280,7 @@ void iommufd_access_notify_unmap(struct io_pagetable *iopt, unsigned long iova,void iommufd_access_unpin_pages (struct iommufd_access *access,unsigned long iova,unsigned long length) { +bool internal =iommufd_access_is_internal (access);struct iopt_area_contig_iter iter;struct io_pagetable *iopt;unsigned long last_iova;
@@ -1305,7 +1307,8 @@ void iommufd_access_unpin_pages(struct iommufd_access *access, area,iopt_area_iova_to_index (area, iter.cur_iova),iopt_area_iova_to_index ( area, - min(last_iova, iopt_area_last_iova(area)))); +min (last_iova,iopt_area_last_iova (area))), + internal);WARN_ON (!iopt_area_contig_done (&iter));up_read (&iopt->iova_rwsem);mutex_unlock (&access->ioas_lock);
@@ -1354,6 +1357,7 @@ int iommufd_access_pin_pages(struct iommufd_access *access, unsigned long iova, unsigned long length, struct page **out_pages, unsigned int flags) { + bool internal = iommufd_access_is_internal(access);struct iopt_area_contig_iter iter;struct io_pagetable *iopt;unsigned long last_iova;
@@ -1362,7 +1366,8 @@ int iommufd_access_pin_pages(struct iommufd_access *access, unsigned long iova,/* Driver's ops don't support pin_pages */ if (IS_ENABLED (CONFIG_IOMMUFD_TEST ) && -WARN_ON (access->iova_alignment !=PAGE_SIZE || !access->ops->unmap)) +WARN_ON (access->iova_alignment !=PAGE_SIZE || + (!internal && !access->ops->unmap)))return -EINVAL ;if (!length)
@@ -1396,7 +1401,7 @@ int iommufd_access_pin_pages(struct iommufd_access *access, unsigned long iova, } rc =iopt_area_add_access (area, index, last_index, out_pages, - flags); + flags, internal);if (rc) goto err_remove; out_pages += last_index - index +1 ;
@@ -1419,7 +1424,8 @@ int iommufd_access_pin_pages(struct iommufd_access *access, unsigned long iova,iopt_area_iova_to_index (area, iter.cur_iova),iopt_area_iova_to_index ( area,min (last_iova, - iopt_area_last_iova(area)))); +iopt_area_last_iova (area))), + internal); }up_read (&iopt->iova_rwsem);mutex_unlock (&access->ioas_lock);
drivers/iommu/iommufd/io_pagetable.c +6 -0
@@ -719,6 +719,12 @@ static int iopt_unmap_iova_range(struct io_pagetable *iopt, unsigned long start, goto out_unlock_iova; } +/* The area is locked by an object that has not been destroyed */ +if (area->num_locks) { + rc = -EBUSY ; + goto out_unlock_iova; + } +if (area_first < start || area_last > last) { rc = -ENOENT ; goto out_unlock_iova;
drivers/iommu/iommufd/pages.c +10 -2
@@ -2111,7 +2111,7 @@ iopt_pages_get_exact_access(struct iopt_pages *pages, unsigned long index,*/ int iopt_area_add_access (struct iopt_area *area,unsigned long start_index,unsigned long last_index,struct page **out_pages, -unsigned int flags) +unsigned int flags,bool lock_area) {struct iopt_pages *pages = area->pages;struct iopt_pages_access *access;
@@ -2124,6 +2124,8 @@ int iopt_area_add_access(struct iopt_area *area, unsigned long start_index, access =iopt_pages_get_exact_access (pages, start_index, last_index);if (access) { area->num_accesses++; +if (lock_area) + area->num_locks++; access->users++;iopt_pages_fill_from_xarray (pages, start_index, last_index, out_pages);
@@ -2145,6 +2147,8 @@ int iopt_area_add_access(struct iopt_area *area, unsigned long start_index, access->node.last = last_index; access->users =1 ; area->num_accesses++; +if (lock_area) + area->num_locks++;interval_tree_insert (&access->node, &pages->access_itree);mutex_unlock (&pages->mutex);return 0 ;
@@ -2166,7 +2170,7 @@ int iopt_area_add_access(struct iopt_area *area, unsigned long start_index,* must stop using the PFNs before calling this. */ void iopt_area_remove_access (struct iopt_area *area,unsigned long start_index, -unsigned long last_index) +unsigned long last_index,bool unlock_area) {struct iopt_pages *pages = area->pages;struct iopt_pages_access *access;
@@ -2177,6 +2181,10 @@ void iopt_area_remove_access(struct iopt_area *area, unsigned long start_index, goto out_unlock;WARN_ON (area->num_accesses ==0 || access->users ==0 ); +if (unlock_area) { +WARN_ON (area->num_locks ==0 ); + area->num_locks--; + } area->num_accesses--; access->users--;if (access->users)
Signature · 1 line
2.43.0
PATCH v9 12/29 iommufd/viommu: Add driver-defined vDEVICE support
In reply to Nicolin ChenNVIDIA VCMDQ driver will have a driver-defined vDEVICE structure and do some HW configurations with that.
To allow IOMMU drivers to define their own vDEVICE structures, move the struct iommufd_vdevice to the public header and provide a pair of viommu ops, similar to get_viommu_size and viommu_init.
Doing this, however, creates a new window between the vDEVICE allocation and its driver-level initialization, during which an abort could happen but it can't invoke a driver destroy function from the struct viommu_ops since the driver structure isn't initialized yet. vIOMMU object doesn't have this problem, since its destroy op is set via the viommu_ops by the driver viommu_init function. Thus, vDEVICE should do something similar:
add a destroy function pointer inside the struct iommufd_vdevice instead of the struct iommufd_viommu_ops.
Note that there is unlikely a use case for a type dependent vDEVICE, so a static vdevice_size is probably enough for the near term instead of a get_vdevice_size function op.
--- drivers/iommu/iommufd/iommufd_private.h | 12 ---------- include/linux/iommufd.h | 31 +++++++++++++++++++++++++ drivers/iommu/iommufd/viommu.c | 26 ++++++++++++++++++++- 3 files changed, 56 insertions(+), 13 deletions(-)
drivers/iommu/iommufd/iommufd_private.h +0 -12
@@ -653,18 +653,6 @@ void iommufd_viommu_destroy(struct iommufd_object *obj);int iommufd_vdevice_alloc_ioctl (struct iommufd_ucmd *ucmd);void iommufd_vdevice_destroy (struct iommufd_object *obj); -struct iommufd_vdevice { -struct iommufd_object obj; -struct iommufd_viommu *viommu; -struct device *dev; - -/* -* Virtual device ID per vIOMMU, e.g. vSID of ARM SMMUv3, vDeviceID of -* AMD IOMMU, and vRID of Intel VT-d -*/ -u64 virt_id; -}; -#ifdef CONFIG_IOMMUFD_TEST int iommufd_test (struct iommufd_ucmd *ucmd);void iommufd_selftest_destroy (struct iommufd_object *obj);
include/linux/iommufd.h +31 -0
@@ -104,6 +104,21 @@ struct iommufd_viommu {enum iommu_viommu_type type; }; +struct iommufd_vdevice { +struct iommufd_object obj; +struct iommufd_viommu *viommu; +struct device *dev; + +/* +* Virtual device ID per vIOMMU, e.g. vSID of ARM SMMUv3, vDeviceID of +* AMD IOMMU, and vRID of Intel VT-d +*/ +u64 virt_id; + +/* Clean up all driver-specific parts of an iommufd_vdevice */ +void (*destroy)(struct iommufd_vdevice *vdev); +}; +/** * struct iommufd_viommu_ops - vIOMMU specific operations * @destroy: Clean up all driver-specific parts of an iommufd_viommu. The memory
@@ -120,6 +135,14 @@ struct iommufd_viommu {* array->entry_num to report the number of handled requests. * The data structure of the array entry must be defined in * include/uapi/linux/iommufd.h +* @vdevice_size: Size of the driver-defined vDEVICE structure per this vIOMMU +* @vdevice_init: Initialize the driver-level structure of a vDEVICE object, or +* related HW procedure. @vdev is already initialized by iommufd +* core: vdev->dev and vdev->viommu pointers; vdev->id carries a +* per-vIOMMU virtual ID (refer to struct iommu_vdevice_alloc in +* include/uapi/linux/iommufd.h) +* If driver has a deinit function to revert what vdevice_init op +* does, it should set it to the @vdev->destroy function pointer */ struct iommufd_viommu_ops {void (*destroy)(struct iommufd_viommu *viommu);
@@ -128,6 +151,8 @@ struct iommufd_viommu_ops { const struct iommu_user_data *user_data);int (*cache_invalidate)(struct iommufd_viommu *viommu,struct iommu_user_data_array *array); +const size_t vdevice_size; +int (*vdevice_init)(struct iommufd_vdevice *vdev); };#if IS_ENABLED (CONFIG_IOMMUFD )
@@ -224,4 +249,10 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, BUILD_BUG_ON_ZERO(offsetof(drv_struct, member)) + \ BUILD_BUG_ON_ZERO(!__same_type(struct iommufd_viommu, \ ((drv_struct *)NULL)->member))) + +#define VDEVICE_STRUCT_SIZE (drv_struct, member) \ + (sizeof(drv_struct) + \ + BUILD_BUG_ON_ZERO(offsetof(drv_struct, member)) + \ + BUILD_BUG_ON_ZERO(!__same_type(struct iommufd_vdevice, \ + ((drv_struct *)NULL)->member)))#endif
drivers/iommu/iommufd/viommu.c +25 -1
@@ -116,6 +116,8 @@ void iommufd_vdevice_destroy(struct iommufd_object *obj) container_of(obj, struct iommufd_vdevice, obj);struct iommufd_viommu *viommu = vdev->viommu; +if (vdev->destroy) + vdev->destroy (vdev);/* xa_cmpxchg is okay to fail if alloc failed xa_cmpxchg previously */ xa_cmpxchg (&viommu->vdevs, vdev->virt_id, vdev,NULL ,GFP_KERNEL );refcount_dec (&viommu->obj.users);
@@ -126,6 +128,7 @@ int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd) {struct iommu_vdevice_alloc *cmd = ucmd->cmd;struct iommufd_vdevice *vdev, *curr; +size_t vdev_size =sizeof (*vdev);struct iommufd_viommu *viommu;struct iommufd_device *idev;u64 virt_id = cmd->virt_id;
@@ -150,7 +153,22 @@ int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_idev; } - vdev =iommufd_object_alloc_ucmd (ucmd, vdev,IOMMUFD_OBJ_VDEVICE ); +if (viommu->ops && viommu->ops->vdevice_size) { +/* +* It is a driver bug for: +* - ops->vdevice_size smaller than the core structure size +* - not implementing a pairing ops->vdevice_init op +*/ +if (WARN_ON_ONCE (viommu->ops->vdevice_size < vdev_size || + !viommu->ops->vdevice_init)) { + rc = -EOPNOTSUPP ; + goto out_put_idev; + } + vdev_size = viommu->ops->vdevice_size; + } + + vdev = (struct iommufd_vdevice *)_iommufd_object_alloc_ucmd ( + ucmd, vdev_size,IOMMUFD_OBJ_VDEVICE );if (IS_ERR (vdev)) { rc =PTR_ERR (vdev); goto out_put_idev;
@@ -168,6 +186,12 @@ int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_idev; } +if (viommu->ops && viommu->ops->vdevice_init) { + rc = viommu->ops->vdevice_init (vdev); +if (rc) + goto out_put_idev; + } + cmd->out_vdevice_id = vdev->obj.id; rc =iommufd_ucmd_respond (ucmd,sizeof (*cmd));
Signature · 1 line
2.43.0
PATCH v9 13/29 iommufd/viommu: Introduce IOMMUFD_OBJ_HW_QUEUE and its related struct
In reply to Nicolin ChenAdd IOMMUFD_OBJ_HW_QUEUE with an iommufd_hw_queue structure, representing a HW-accelerated queue type of IOMMU's physical queue that can be passed through to a user space VM for direct hardware control, such as:
- NVIDIA's Virtual Command Queue
- AMD vIOMMU's Command Buffer, Event Log Buffers, and PPR Log Buffers
Add new viommu ops for iommufd to communicate with IOMMU drivers to fetch supported HW queue structure size and to forward user space ioctls to the IOMMU drivers for initialization/destroy.
As the existing HWs, NVIDIA's VCMDQs access the guest memory via physical addresses, while AMD's Buffers access the guest memory via guest physical addresses (i.e. iova of the nesting parent HWPT). Separate two mutually exclusive hw_queue_init and hw_queue_init_phys ops to indicate whether a vIOMMU HW accesses the guest queue in the guest physical space (via iova) or the host physical space (via pa).
In a latter case, the iommufd core will validate the physical pages of a given guest queue, to ensure the underlying physical pages are contiguous and pinned.
Since this is introduced with NVIDIA's VCMDQs, add hw_queue_init_phys for now, and leave some notes for hw_queue_init in the near future (for AMD).
Either NVIDIA's or AMD's HW is a multi-queue model: NVIDIA's will be only one type in enum iommu_hw_queue_type, while AMD's will be three different types (two of which will have multi queues). Compared to letting the core manage multiple queues with three types per vIOMMU object, it'd be easier for the driver to manage that by having three different driver-structure arrays per vIOMMU object. Thus, pass in the index to the init op.
--- include/linux/iommufd.h | 42 ++++++++++++++++++++++++++++++++++++ include/uapi/linux/iommufd.h | 9 ++++++++ 2 files changed, 51 insertions(+)
include/linux/iommufd.h +42 -0
@@ -37,6 +37,7 @@ enum iommufd_object_type {IOMMUFD_OBJ_VIOMMU ,IOMMUFD_OBJ_VDEVICE ,IOMMUFD_OBJ_VEVENTQ , +IOMMUFD_OBJ_HW_QUEUE , #ifdef CONFIG_IOMMUFD_TEST IOMMUFD_OBJ_SELFTEST, #endif
@@ -119,6 +120,19 @@ struct iommufd_vdevice {void (*destroy)(struct iommufd_vdevice *vdev); }; +struct iommufd_hw_queue { +struct iommufd_object obj; +struct iommufd_viommu *viommu; + +u64 base_addr;/* in guest physical address space */ +size_t length; + +enum iommu_hw_queue_type type; + +/* Clean up all driver-specific parts of an iommufd_hw_queue */ +void (*destroy)(struct iommufd_hw_queue *hw_queue); +}; +/** * struct iommufd_viommu_ops - vIOMMU specific operations * @destroy: Clean up all driver-specific parts of an iommufd_viommu. The memory
@@ -143,6 +157,22 @@ struct iommufd_vdevice {* include/uapi/linux/iommufd.h) * If driver has a deinit function to revert what vdevice_init op * does, it should set it to the @vdev->destroy function pointer +* @get_hw_queue_size: Get the size of a driver-defined HW queue structure for a +* given @viommu corresponding to @queue_type. Driver should +* return 0 if HW queue aren't supported accordingly. It is +* required for driver to use the HW_QUEUE_STRUCT_SIZE macro +* to sanitize the driver-level HW queue structure related +* to the core one +* @hw_queue_init_phys: Initialize the driver-level structure of a HW queue that +* is initialized with its core-level structure that holds +* all the info about a guest queue memory. +* Driver providing this op indicates that HW accesses the +* guest queue memory via physical addresses. +* @index carries the logical HW QUEUE ID per vIOMMU in a +* guest VM, for a multi-queue model. @base_addr_pa carries +* the physical location of the guest queue +* If driver has a deinit function to revert what this op +* does, it should set it to the @hw_queue->destroy pointer */ struct iommufd_viommu_ops {void (*destroy)(struct iommufd_viommu *viommu);
@@ -153,6 +183,11 @@ struct iommufd_viommu_ops { struct iommu_user_data_array *array);const size_t vdevice_size;int (*vdevice_init)(struct iommufd_vdevice *vdev); +size_t (*get_hw_queue_size)(struct iommufd_viommu *viommu, +enum iommu_hw_queue_type queue_type); +/* AMD's HW will add hw_queue_init simply using @hw_queue->base_addr */ +int (*hw_queue_init_phys)(struct iommufd_hw_queue *hw_queue,u32 index, +phys_addr_t base_addr_pa); };#if IS_ENABLED (CONFIG_IOMMUFD )
@@ -255,4 +290,11 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, BUILD_BUG_ON_ZERO(offsetof(drv_struct, member)) + \ BUILD_BUG_ON_ZERO(!__same_type(struct iommufd_vdevice, \ ((drv_struct *)NULL)->member))) + +#define HW_QUEUE_STRUCT_SIZE (drv_struct, member) \ + (sizeof(drv_struct) + \ + BUILD_BUG_ON_ZERO(offsetof(drv_struct, member)) + \ + BUILD_BUG_ON_ZERO(!__same_type(struct iommufd_hw_queue, \ + ((drv_struct *)NULL)->member))) +#endif
include/uapi/linux/iommufd.h +9 -0
@@ -1147,4 +1147,13 @@ struct iommu_veventq_alloc { __u32 __reserved; };#define IOMMU_VEVENTQ_ALLOC _IO(IOMMUFD_TYPE, IOMMUFD_CMD_VEVENTQ_ALLOC) + +/** +* enum iommu_hw_queue_type - HW Queue Type +* @IOMMU_HW_QUEUE_TYPE_DEFAULT: Reserved for future use +*/ +enum iommu_hw_queue_type { +IOMMU_HW_QUEUE_TYPE_DEFAULT =0 , +}; +#endif
Signature · 1 line
2.43.0
PATCH v9 14/29 iommufd/viommu: Add IOMMUFD_CMD_HW_QUEUE_ALLOC ioctl
In reply to Nicolin ChenIntroduce a new IOMMUFD_CMD_HW_QUEUE_ALLOC ioctl for user space to allocate a HW QUEUE object for a vIOMMU specific HW-accelerated queue, e.g.:
- NVIDIA's Virtual Command Queue
- AMD vIOMMU's Command Buffer, Event Log Buffers, and PPR Log Buffers
Since this is introduced with NVIDIA's VCMDQs that access the guest memory in the physical address space, add an iommufd_hw_queue_alloc_phys() helper that will create an access object to the queue memory in the IOAS, to avoid the mappings of the guest memory from being unmapped, during the life cycle of the HW queue object.
AMD's HW will need an hw_queue_init op that is mutually exclusive with the hw_queue_init_phys op, and their case will bypass the access part, i.e. no iommufd_hw_queue_alloc_phys() call.
--- drivers/iommu/iommufd/iommufd_private.h | 2 + include/linux/iommufd.h | 1 + include/uapi/linux/iommufd.h | 33 +++++ drivers/iommu/iommufd/main.c | 6 + drivers/iommu/iommufd/viommu.c | 180 ++++++++++++++++++++++++ 5 files changed, 222 insertions(+)
drivers/iommu/iommufd/iommufd_private.h +2 -0
@@ -652,6 +652,8 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd);void iommufd_viommu_destroy (struct iommufd_object *obj);int iommufd_vdevice_alloc_ioctl (struct iommufd_ucmd *ucmd);void iommufd_vdevice_destroy (struct iommufd_object *obj); +int iommufd_hw_queue_alloc_ioctl (struct iommufd_ucmd *ucmd); +void iommufd_hw_queue_destroy (struct iommufd_object *obj);#ifdef CONFIG_IOMMUFD_TEST int iommufd_test (struct iommufd_ucmd *ucmd);
include/linux/iommufd.h +1 -0
@@ -123,6 +123,7 @@ struct iommufd_vdevice {struct iommufd_hw_queue {struct iommufd_object obj;struct iommufd_viommu *viommu; +struct iommufd_access *access;u64 base_addr;/* in guest physical address space */ size_t length;
include/uapi/linux/iommufd.h +33 -0
@@ -56,6 +56,7 @@ enum {IOMMUFD_CMD_VDEVICE_ALLOC =0x91 ,IOMMUFD_CMD_IOAS_CHANGE_PROCESS =0x92 ,IOMMUFD_CMD_VEVENTQ_ALLOC =0x93 , + IOMMUFD_CMD_HW_QUEUE_ALLOC = 0x94, };/**
@@ -1156,4 +1157,36 @@ enum iommu_hw_queue_type { IOMMU_HW_QUEUE_TYPE_DEFAULT = 0, }; +/** +* struct iommu_hw_queue_alloc - ioctl(IOMMU_HW_QUEUE_ALLOC) +* @size: sizeof(struct iommu_hw_queue_alloc) +* @flags: Must be 0 +* @viommu_id: Virtual IOMMU ID to associate the HW queue with +* @type: One of enum iommu_hw_queue_type +* @index: The logical index to the HW queue per virtual IOMMU for a multi-queue +* model +* @out_hw_queue_id: The ID of the new HW queue +* @nesting_parent_iova: Base address of the queue memory in the guest physical +* address space +* @length: Length of the queue memory +* +* Allocate a HW queue object for a vIOMMU-specific HW-accelerated queue, which +* allows HW to access a guest queue memory described using @nesting_parent_iova +* and @length. +* +* A vIOMMU can allocate multiple queues, but it must use a different @index per +* type to separate each allocation, e.g. +* Type1 HW queue0, Type1 HW queue1, Type2 HW queue0, ... +*/ +struct iommu_hw_queue_alloc { +__u32 size; +__u32 flags; +__u32 viommu_id; +__u32 type; +__u32 index; +__u32 out_hw_queue_id; +__aligned_u64 nesting_parent_iova; +__aligned_u64 length; +}; +#define IOMMU_HW_QUEUE_ALLOC _IO(IOMMUFD_TYPE, IOMMUFD_CMD_HW_QUEUE_ALLOC)#endif
drivers/iommu/iommufd/main.c +6 -0
@@ -354,6 +354,7 @@ union ucmd_buffer {struct iommu_destroy destroy;struct iommu_fault_alloc fault;struct iommu_hw_info info; +struct iommu_hw_queue_alloc hw_queue;struct iommu_hwpt_alloc hwpt;struct iommu_hwpt_get_dirty_bitmap get_dirty_bitmap;struct iommu_hwpt_invalidate cache;
@@ -396,6 +397,8 @@ static const struct iommufd_ioctl_op iommufd_ioctl_ops[] = { struct iommu_fault_alloc, out_fault_fd),IOCTL_OP (IOMMU_GET_HW_INFO ,iommufd_get_hw_info ,struct iommu_hw_info ,__reserved ), +IOCTL_OP (IOMMU_HW_QUEUE_ALLOC ,iommufd_hw_queue_alloc_ioctl , +struct iommu_hw_queue_alloc ,length ),IOCTL_OP (IOMMU_HWPT_ALLOC ,iommufd_hwpt_alloc ,struct iommu_hwpt_alloc , __reserved), IOCTL_OP(IOMMU_HWPT_GET_DIRTY_BITMAP, iommufd_hwpt_get_dirty_bitmap,
@@ -559,6 +562,9 @@ static const struct iommufd_object_ops iommufd_object_ops[] = { [IOMMUFD_OBJ_FAULT] = { .destroy = iommufd_fault_destroy, }, + [IOMMUFD_OBJ_HW_QUEUE] = { + .destroy = iommufd_hw_queue_destroy, + }, [IOMMUFD_OBJ_HWPT_PAGING] = { .destroy = iommufd_hwpt_paging_destroy, .abort = iommufd_hwpt_paging_abort,
drivers/iommu/iommufd/viommu.c +180 -0
183 lines not on this page · raw message
Signature · 1 line
2.43.0
PATCH v9 15/29 iommufd/driver: Add iommufd_hw_queue_depend/undepend() helpers
In reply to Nicolin ChenNVIDIA Virtual Command Queue is one of the iommufd users exposing vIOMMU features to user space VMs. Its hardware has a strict rule when mapping and unmapping multiple global CMDQVs to/from a VM-owned VINTF, requiring mappings in ascending order and unmappings in descending order.
The tegra241-cmdqv driver can apply the rule for a mapping in the LVCMDQ allocation handler. However, it can't do the same for an unmapping since user space could start random destroy calls breaking the rule, while the destroy op in the driver level can't reject a destroy call as it returns void.
Add iommufd_hw_queue_depend/undepend for-driver helpers, allowing LVCMDQ allocator to refcount_inc() a sibling LVCMDQ object and LVCMDQ destroyer to refcount_dec(), so that iommufd core will help block a random destroy call that breaks the rule.
This is a bit of compromise, because a driver might end up with abusing the API that deadlocks the objects. So restrict the API to a dependency between two driver-allocated objects of the same type, as iommufd would unlikely build any core-level dependency in this case. And encourage to use the macro version that currently supports the HW QUEUE objects only.
--- include/linux/iommufd.h | 44 ++++++++++++++++++++++++++++++++++ drivers/iommu/iommufd/driver.c | 28 ++++++++++++++++++++++ 2 files changed, 72 insertions(+)
include/linux/iommufd.h +44 -0
@@ -251,6 +251,10 @@ static inline int iommufd_vfio_compat_set_no_iommu(struct iommufd_ctx *ictx)#endif /* CONFIG_IOMMUFD */ #if IS_ENABLED (CONFIG_IOMMUFD_DRIVER_CORE ) +int _iommufd_object_depend (struct iommufd_object *obj_dependent, +struct iommufd_object *obj_depended); +void _iommufd_object_undepend (struct iommufd_object *obj_dependent, +struct iommufd_object *obj_depended);struct device *iommufd_viommu_find_dev (struct iommufd_viommu *viommu,unsigned long vdev_id); int iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
@@ -259,6 +263,18 @@ int iommufd_viommu_report_event(struct iommufd_viommu *viommu, enum iommu_veventq_type type, void *event_data, size_t data_len);#else /* !CONFIG_IOMMUFD_DRIVER_CORE */ +static inline int _iommufd_object_depend (struct iommufd_object *obj_dependent, +struct iommufd_object *obj_depended) +{ +return -EOPNOTSUPP ; +} + +static inline void +_iommufd_object_undepend (struct iommufd_object *obj_dependent, +struct iommufd_object *obj_depended) +{ +} +static inline struct device *iommufd_viommu_find_dev (struct iommufd_viommu *viommu,unsigned long vdev_id) {
@@ -298,4 +314,32 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, BUILD_BUG_ON_ZERO(!__same_type(struct iommufd_hw_queue, \ ((drv_struct *)NULL)->member))) +/* +* Helpers for IOMMU driver to build/destroy a dependency between two sibling +* structures created by one of the allocators above +*/ +#define iommufd_hw_queue_depend (dependent, depended, member) \ + ({ \ + int ret = -EINVAL; \ + \ + static_assert(__same_type(struct iommufd_hw_queue, \ + dependent->member)); \ + static_assert(__same_type(typeof(*dependent), *depended)); \ + if (!WARN_ON_ONCE(dependent->member.viommu != \ + depended->member.viommu)) \ + ret = _iommufd_object_depend(&dependent->member.obj, \ + &depended->member.obj); \ + ret; \ + }) + +#define iommufd_hw_queue_undepend (dependent, depended, member) \ + ({ \ + static_assert(__same_type(struct iommufd_hw_queue, \ + dependent->member)); \ + static_assert(__same_type(typeof(*dependent), *depended)); \ + WARN_ON_ONCE(dependent->member.viommu != \ + depended->member.viommu); \ + _iommufd_object_undepend(&dependent->member.obj, \ + &depended->member.obj); \ + })#endif
drivers/iommu/iommufd/driver.c +28 -0
@@ -3,6 +3,34 @@*/ #include "iommufd_private.h" +/* Driver should use a per-structure helper in include/linux/iommufd.h */ +int _iommufd_object_depend (struct iommufd_object *obj_dependent, +struct iommufd_object *obj_depended) +{ +/* Reject self dependency that dead locks */ +if (obj_dependent == obj_depended) +return -EINVAL ; +/* Only support dependency between two objects of the same type */ +if (obj_dependent->type != obj_depended->type) +return -EINVAL ; + +refcount_inc (&obj_depended->users); +return 0 ; +} +EXPORT_SYMBOL_NS_GPL (_iommufd_object_depend,"IOMMUFD" ); + +/* Driver should use a per-structure helper in include/linux/iommufd.h */ +void _iommufd_object_undepend (struct iommufd_object *obj_dependent, +struct iommufd_object *obj_depended) +{ +if (WARN_ON_ONCE (obj_dependent == obj_depended || + obj_dependent->type != obj_depended->type)) +return ; + +refcount_dec (&obj_depended->users); +} +EXPORT_SYMBOL_NS_GPL (_iommufd_object_undepend,"IOMMUFD" ); +/* Caller should xa_lock(&viommu->vdevs) to protect the return value */ struct device *iommufd_viommu_find_dev (struct iommufd_viommu *viommu,unsigned long vdev_id)
Signature · 1 line
2.43.0
PATCH v9 16/29 iommufd/selftest: Add coverage for IOMMUFD_CMD_HW_QUEUE_ALLOC
In reply to Nicolin ChenSome simple tests for IOMMUFD_CMD_HW_QUEUE_ALLOC infrastructure covering the new iommufd_hw_queue_depend/undepend() helpers.
--- drivers/iommu/iommufd/iommufd_test.h | 3 + tools/testing/selftests/iommu/iommufd_utils.h | 31 ++++++ drivers/iommu/iommufd/selftest.c | 97 +++++++++++++++++++ tools/testing/selftests/iommu/iommufd.c | 59 +++++++++++ .../selftests/iommu/iommufd_fail_nth.c | 6 ++ 5 files changed, 196 insertions(+)
drivers/iommu/iommufd/iommufd_test.h +3 -0
@@ -265,4 +265,7 @@ struct iommu_viommu_event_selftest { __u32 virt_id; }; +#define IOMMU_HW_QUEUE_TYPE_SELFTEST 0xdeadbeef +#define IOMMU_TEST_HW_QUEUE_MAX 2 +#endif
tools/testing/selftests/iommu/iommufd_utils.h +31 -0
@@ -956,6 +956,37 @@ static int _test_cmd_vdevice_alloc(int fd, __u32 viommu_id, __u32 idev_id, _test_cmd_vdevice_alloc(self->fd, viommu_id, idev_id, \ virt_id, vdev_id)) +static int _test_cmd_hw_queue_alloc (int fd,__u32 viommu_id,__u32 type, +__u32 idx,__u64 base_addr,__u64 length, +__u32 *hw_queue_id) +{ +struct iommu_hw_queue_alloc cmd = { + .size =sizeof (cmd), + .viommu_id = viommu_id, + .type = type, + .index = idx, + .nesting_parent_iova = base_addr, + .length = length, + }; +int ret; + + ret =ioctl (fd,IOMMU_HW_QUEUE_ALLOC , &cmd); +if (ret) +return ret; +if (hw_queue_id) + *hw_queue_id = cmd.out_hw_queue_id; +return 0 ; +} + +#define test_cmd_hw_queue_alloc (viommu_id, type, idx, base_addr, len, out_qid) \ + ASSERT_EQ(0, _test_cmd_hw_queue_alloc(self->fd, viommu_id, type, idx, \ + base_addr, len, out_qid)) +#define test_err_hw_queue_alloc (_errno, viommu_id, type, idx, base_addr, len, \ + out_qid) \ + EXPECT_ERRNO(_errno, \ + _test_cmd_hw_queue_alloc(self->fd, viommu_id, type, idx, \ + base_addr, len, out_qid)) +static int _test_cmd_veventq_alloc (int fd,__u32 viommu_id,__u32 type,__u32 *veventq_id,__u32 *veventq_fd) {
drivers/iommu/iommufd/selftest.c +97 -0
140 lines not on this page · raw message
tools/testing/selftests/iommu/iommufd.c +59 -0
65 lines not on this page · raw message
tools/testing/selftests/iommu/iommufd_fail_nth.c +6 -0
@@ -634,6 +634,7 @@ TEST_FAIL_NTH(basic_fail_nth, device)uint32_t idev_id;uint32_t hwpt_id;uint32_t viommu_id; +uint32_t hw_queue_id;uint32_t vdev_id;__u64 iova;
@@ -696,6 +697,11 @@ TEST_FAIL_NTH(basic_fail_nth, device)if (_test_cmd_vdevice_alloc (self->fd, viommu_id, idev_id,0 , &vdev_id))return -1 ; +if (_test_cmd_hw_queue_alloc (self->fd, viommu_id, +IOMMU_HW_QUEUE_TYPE_SELFTEST ,0 , iova, +PAGE_SIZE , &hw_queue_id)) +return -1 ; +if (_test_ioctl_fault_alloc (self->fd, &fault_id, &fault_fd))return -1 ;close (fault_fd);
Signature · 1 line
2.43.0
Re: [PATCH v9 16/29] iommufd/selftest: Add coverage for IOMMUFD_CMD_HW_QUEUE_ALLOC
Quoted 4 lines · Nicolin Chen
Some simple tests for
IOMMUFD_CMD_HW_QUEUE_ALLOCinfrastructure covering the new iommufd_hw_queue_depend/undepend() helpers.Signed-off-by Nicolin Chen <[email protected]>
Quoted 309 lines
--- drivers/iommu/iommufd/iommufd_test.h | 3 + tools/testing/selftests/iommu/iommufd_utils.h | 31 ++++++ drivers/iommu/iommufd/selftest.c | 97 +++++++++++++++++++ tools/testing/selftests/iommu/iommufd.c | 59 +++++++++++ .../selftests/iommu/iommufd_fail_nth.c | 6 ++ 5 files changed, 196 insertions(+)drivers/iommu/iommufd/iommufd_test.h +3
-0@@ -265,4 +265,7 @@ struct iommu_viommu_event_selftest { __u32 virt_id; }; +#define IOMMU_HW_QUEUE_TYPE_SELFTEST 0xdeadbeef +#define IOMMU_TEST_HW_QUEUE_MAX 2 +#endif tools/testing/selftests/iommu/iommufd_utils.h +31
-0@@ -956,6 +956,37 @@ static int _test_cmd_vdevice_alloc(int fd, __u32 viommu_id, __u32 idev_id, _test_cmd_vdevice_alloc(self->fd, viommu_id, idev_id, \ virt_id, vdev_id)) +static int _test_cmd_hw_queue_alloc (int fd,__u32 viommu_id,__u32 type, +__u32 idx,__u64 base_addr,__u64 length, +__u32 *hw_queue_id) +{ +struct iommu_hw_queue_alloc cmd = { + .size =sizeof (cmd), + .viommu_id = viommu_id, + .type = type, + .index = idx, + .nesting_parent_iova = base_addr, + .length = length, + }; +int ret; + + ret =ioctl (fd,IOMMU_HW_QUEUE_ALLOC , &cmd); +if (ret) +return ret; +if (hw_queue_id) + *hw_queue_id = cmd.out_hw_queue_id; +return 0 ; +} + +#define test_cmd_hw_queue_alloc (viommu_id, type, idx, base_addr, len, out_qid) \ + ASSERT_EQ(0, _test_cmd_hw_queue_alloc(self->fd, viommu_id, type, idx, \ + base_addr, len, out_qid)) +#define test_err_hw_queue_alloc (_errno, viommu_id, type, idx, base_addr, len, \ + out_qid) \ + EXPECT_ERRNO(_errno, \ + _test_cmd_hw_queue_alloc(self->fd, viommu_id, type, idx, \ + base_addr, len, out_qid)) +static int _test_cmd_veventq_alloc (int fd,__u32 viommu_id,__u32 type,__u32 *veventq_id,__u32 *veventq_fd) {drivers/iommu/iommufd/selftest.c +97
-0@@ -150,6 +150,8 @@ to_mock_nested(struct iommu_domain *domain)struct mock_viommu {struct iommufd_viommu core;struct mock_iommu_domain *s2_parent; +struct mock_hw_queue *hw_queue[IOMMU_TEST_HW_QUEUE_MAX ]; +struct mutex queue_mutex; };static inline struct mock_viommu *to_mock_viommu (struct iommufd_viommu *viommu)@@ -157,6 +159,19 @@ static inline struct mock_viommu *to_mock_viommu(struct iommufd_viommu *viommu) return container_of(viommu, struct mock_viommu, core); } +struct mock_hw_queue { +struct iommufd_hw_queue core; +struct mock_viommu *mock_viommu; +struct mock_hw_queue *prev; +u16 index; +}; + +static inline struct mock_hw_queue * +to_mock_hw_queue (struct iommufd_hw_queue *hw_queue) +{ + return container_of(hw_queue, struct mock_hw_queue, core); +} +enum selftest_obj_type {TYPE_IDEV , };@@ -670,9 +685,11 @@ static void mock_viommu_destroy(struct iommufd_viommu *viommu) {struct mock_iommu_device *mock_iommu =container_of ( viommu->iommu_dev, struct mock_iommu_device, iommu_dev); +struct mock_viommu *mock_viommu =to_mock_viommu (viommu);if (refcount_dec_and_test (&mock_iommu->users))complete (&mock_iommu->complete); +mutex_destroy (&mock_viommu->queue_mutex);/* iommufd core frees mock_viommu and viommu */ }@@ -764,10 +781,86 @@ static int mock_viommu_cache_invalidate(struct iommufd_viommu *viommu,return rc; } +static size_t mock_viommu_get_hw_queue_size (struct iommufd_viommu *viommu, +enum iommu_hw_queue_type queue_type) +{ +if (queue_type !=IOMMU_HW_QUEUE_TYPE_SELFTEST ) +return 0 ; + return HW_QUEUE_STRUCT_SIZE(struct mock_hw_queue, core); +} + +static void mock_hw_queue_destroy (struct iommufd_hw_queue *hw_queue) +{ +struct mock_hw_queue *mock_hw_queue =to_mock_hw_queue (hw_queue); +struct mock_viommu *mock_viommu = mock_hw_queue->mock_viommu; + +mutex_lock (&mock_viommu->queue_mutex); + mock_viommu->hw_queue[mock_hw_queue->index] =NULL ; +if (mock_hw_queue->prev) +iommufd_hw_queue_undepend (mock_hw_queue, mock_hw_queue->prev, + core); +mutex_unlock (&mock_viommu->queue_mutex); +} + +/* Test iommufd_hw_queue_depend/undepend() */ +static int mock_hw_queue_init_phys (struct iommufd_hw_queue *hw_queue,u32 index, +phys_addr_t base_addr_pa) +{ +struct mock_viommu *mock_viommu =to_mock_viommu (hw_queue->viommu); +struct mock_hw_queue *mock_hw_queue =to_mock_hw_queue (hw_queue); +struct mock_hw_queue *prev =NULL ; +int rc =0 ; + +if (index >=IOMMU_TEST_HW_QUEUE_MAX ) +return -EINVAL ; + +mutex_lock (&mock_viommu->queue_mutex); + +if (mock_viommu->hw_queue[index]) { + rc = -EEXIST ; + goto unlock; + } + +if (index) { + prev = mock_viommu->hw_queue[index -1 ]; +if (!prev) { + rc = -EIO ; + goto unlock; + } + } + +/* +* Test to catch a kernel bug if the core converted the physical address +* incorrectly. Let mock_domain_iova_to_phys() WARN_ON if it fails. +*/ +if (base_addr_pa !=iommu_iova_to_phys (&mock_viommu->s2_parent->domain, + hw_queue->base_addr)) { + rc = -EFAULT ; + goto unlock; + } + +if (prev) { + rc =iommufd_hw_queue_depend (mock_hw_queue, prev, core); +if (rc) + goto unlock; + } + + mock_hw_queue->prev = prev; + mock_hw_queue->mock_viommu = mock_viommu; + mock_viommu->hw_queue[index] = mock_hw_queue; + + hw_queue->destroy = &mock_hw_queue_destroy; +unlock: +mutex_unlock (&mock_viommu->queue_mutex); +return rc; +} +static struct iommufd_viommu_ops mock_viommu_ops = { .destroy = mock_viommu_destroy, .alloc_domain_nested = mock_viommu_alloc_domain_nested, .cache_invalidate = mock_viommu_cache_invalidate, + .get_hw_queue_size = mock_viommu_get_hw_queue_size, + .hw_queue_init_phys = mock_hw_queue_init_phys, }; static size_t mock_get_viommu_size(struct device *dev,@@ -784,6 +877,7 @@ static int mock_viommu_init(struct iommufd_viommu *viommu, {struct mock_iommu_device *mock_iommu =container_of ( viommu->iommu_dev, struct mock_iommu_device, iommu_dev); +struct mock_viommu *mock_viommu =to_mock_viommu (viommu);struct iommu_viommu_selftest data;int rc;@@ -801,6 +895,9 @@ static int mock_viommu_init(struct iommufd_viommu *viommu, }refcount_inc (&mock_iommu->users); +mutex_init (&mock_viommu->queue_mutex); + mock_viommu->s2_parent =to_mock_domain (parent_domain); + viommu->ops = &mock_viommu_ops;return 0 ; }tools/testing/selftests/iommu/iommufd.c +59
-0@@ -3032,6 +3032,65 @@ TEST_F(iommufd_viommu, vdevice_cache) } } +TEST_F(iommufd_viommu, hw_queue) +{ +__u64 iova =MOCK_APERTURE_START , iova2; +uint32_t viommu_id = self->viommu_id; +uint32_t hw_queue_id[2 ]; + +if (!viommu_id) +SKIP (return,"Skipping test for variant no_viommu" ); + +/* Fail IOMMU_HW_QUEUE_TYPE_DEFAULT */ +test_err_hw_queue_alloc (EOPNOTSUPP , viommu_id, +IOMMU_HW_QUEUE_TYPE_DEFAULT ,0 , iova,PAGE_SIZE , + &hw_queue_id[0 ]); +/* Fail queue addr and length */ +test_err_hw_queue_alloc (EINVAL , viommu_id,IOMMU_HW_QUEUE_TYPE_SELFTEST , +0 , iova,0 , &hw_queue_id[0 ]); +test_err_hw_queue_alloc (EOVERFLOW , viommu_id, +IOMMU_HW_QUEUE_TYPE_SELFTEST ,0 , ~(uint64_t )0 , +PAGE_SIZE , &hw_queue_id[0 ]); +/* Fail missing iova */ +test_err_hw_queue_alloc (ENOENT , viommu_id,IOMMU_HW_QUEUE_TYPE_SELFTEST , +0 , iova,PAGE_SIZE , &hw_queue_id[0 ]); + +/* Map iova */ +test_ioctl_ioas_map (buffer,PAGE_SIZE , &iova); +test_ioctl_ioas_map (buffer +PAGE_SIZE ,PAGE_SIZE , &iova2); + +/* Fail index=1 and =MAX; must start from index=0 */ +test_err_hw_queue_alloc (EIO , viommu_id,IOMMU_HW_QUEUE_TYPE_SELFTEST ,1 , + iova,PAGE_SIZE , &hw_queue_id[0 ]); +test_err_hw_queue_alloc (EINVAL , viommu_id,IOMMU_HW_QUEUE_TYPE_SELFTEST , +IOMMU_TEST_HW_QUEUE_MAX , iova,PAGE_SIZE , + &hw_queue_id[0 ]); + +/* Allocate index=0, declare ownership of the iova */ +test_cmd_hw_queue_alloc (viommu_id,IOMMU_HW_QUEUE_TYPE_SELFTEST ,0 , + iova,PAGE_SIZE , &hw_queue_id[0 ]); +/* Fail duplicated index */ +test_err_hw_queue_alloc (EEXIST , viommu_id,IOMMU_HW_QUEUE_TYPE_SELFTEST , +0 , iova,PAGE_SIZE , &hw_queue_id[0 ]); +/* Fail unmap, due to iova ownership */ +test_err_ioctl_ioas_unmap (EBUSY , iova,PAGE_SIZE ); +/* The 2nd page is not pinned, so it can be unmmap */ +test_ioctl_ioas_unmap (iova2,PAGE_SIZE ); + +/* Allocate index=1, with an unaligned case */ +test_cmd_hw_queue_alloc (viommu_id,IOMMU_HW_QUEUE_TYPE_SELFTEST ,1 , + iova +PAGE_SIZE /2 ,PAGE_SIZE /2 , + &hw_queue_id[1 ]); +/* Fail to destroy, due to dependency */ +EXPECT_ERRNO (EBUSY ,_test_ioctl_destroy (self->fd, hw_queue_id[0 ])); + +/* Destroy in descending order */ +test_ioctl_destroy (hw_queue_id[1 ]); +test_ioctl_destroy (hw_queue_id[0 ]); +/* Now it can unmap the first page */ +test_ioctl_ioas_unmap (iova,PAGE_SIZE ); +} +FIXTURE (iommufd_device_pasid) {int fd;tools/testing/selftests/iommu/iommufd_fail_nth.c +6
-0@@ -634,6 +634,7 @@ TEST_FAIL_NTH(basic_fail_nth, device)uint32_t idev_id;uint32_t hwpt_id;uint32_t viommu_id; +uint32_t hw_queue_id;uint32_t vdev_id;__u64 iova;@@ -696,6 +697,11 @@ TEST_FAIL_NTH(basic_fail_nth, device)if (_test_cmd_vdevice_alloc (self->fd, viommu_id, idev_id,0 , &vdev_id))return -1 ; +if (_test_cmd_hw_queue_alloc (self->fd, viommu_id, +IOMMU_HW_QUEUE_TYPE_SELFTEST ,0 , iova, +PAGE_SIZE , &hw_queue_id)) +return -1 ; +if (_test_ioctl_fault_alloc (self->fd, &fault_id, &fault_fd))return -1 ;close (fault_fd);Signature · 1 line
2.43.0
PATCH v9 17/29 iommufd: Add mmap interface
In reply to Nicolin ChenFor vIOMMU passing through HW resources to user space (VMs), allowing a VM to control the passed through HW directly by accessing hardware registers, add an mmap infrastructure to map the physical MMIO pages to user space.
Maintain a maple tree per ictx as a translation table managing mmappable regions, from an allocated for-user mmap offset to an iommufd_mmap struct, where it stores the real physical address range for io_remap_pfn_range().
Keep track of the lifecycle of the mmappable region by taking refcount of its owner, so as to enforce user space to unmap the region first before it can destroy its owner object.
To allow an IOMMU driver to add and delete mmappable regions onto/from the maple tree, add iommufd_viommu_alloc/destroy_mmap helpers.
--- drivers/iommu/iommufd/iommufd_private.h | 14 ++++++ include/linux/iommufd.h | 42 +++++++++++++++++ drivers/iommu/iommufd/driver.c | 52 ++++++++++++++++++++ drivers/iommu/iommufd/main.c | 63 +++++++++++++++++++++++++ 4 files changed, 171 insertions(+)
drivers/iommu/iommufd/iommufd_private.h +14 -0
@@ -7,6 +7,7 @@#include <linux/iommu.h> #include <linux/iommufd.h> #include <linux/iova_bitmap.h> +#include <linux/maple_tree.h> #include <linux/rwsem.h> #include <linux/uaccess.h> #include <linux/xarray.h>
@@ -44,6 +45,7 @@ struct iommufd_ctx {struct xarray groups;wait_queue_head_t destroy_wait;struct rw_semaphore ioas_creation_lock; +struct maple_tree mt_mmap;struct mutex sw_msi_lock;struct list_head sw_msi_list;
@@ -55,6 +57,18 @@ struct iommufd_ctx {struct iommufd_ioas *vfio_ioas; }; +/* Entry for iommufd_ctx::mt_mmap */ +struct iommufd_mmap { +struct iommufd_object *owner; + +/* Page-shifted start position in mt_mmap to validate vma->vm_pgoff */ +unsigned long vm_pgoff; + +/* Physical range for io_remap_pfn_range() */ +phys_addr_t mmio_addr; +size_t length; +}; +/* * The IOVA to PFN map. The map automatically copies the PFNs into multiple * domains and permits sharing of PFNs between io_pagetable instances. This
include/linux/iommufd.h +42 -0
58 lines not on this page · raw message
drivers/iommu/iommufd/driver.c +52 -0
58 lines not on this page · raw message
drivers/iommu/iommufd/main.c +63 -0
80 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH v9 17/29] iommufd: Add mmap interface
Quoted 13 lines · Nicolin Chen
EXPORT_SYMBOL_NS_GPL(_iommufd_object_undepend, "IOMMUFD");+/*
+ * Allocate an @offset to return to user space to use for anmmap()syscall
+ *
+ * Driver should use a per-structure helper ininclude/linux/iommufd.h
+ */
+int _iommufd_alloc_mmap(structiommufd_ctx*ictx, structiommufd_object*owner,include/linux/iommufd.h +3
-0+ phys_addr_t mmio_addr, size_t length, + unsigned long *offset) +{
...
drivers/iommu/iommufd/driver.c void _iommufd_object_undepend(struct iommufd_object *obj_dependent, · line 61
Quoted 5 lines
(quoted diff) +2
-0+ + /* Skip the first page to ease caller identifying the returned offset*/
drivers/iommu/iommufd/driver.c +2
-0+ rc =mtree_alloc_range (&ictx->mt_mmap, &startp, immap, immap->length, +PAGE_SIZE ,PHYS_ADDR_MAX ,GFP_KERNEL );
This produces a warning on 32-bit targets with a 64-bit phys_addr_t, in practice this would be ARM Cortex-A15 or -A17 systems:
In file included from include/linux/overflow.h:6,
...
from drivers/iommu/iommufd/driver.c:4:
drivers/iommu/iommufd/driver.c: In function '_iommufd_alloc_mmap':
include/linux/limits.h:11:25: error: conversion from 'long long unsigned int' to 'long unsigned int' changes value from '18446744073709551615' to '4294967295' [-Werror=overflow]
11 | #define PHYS_ADDR_MAX (~(phys_addr_t)0)
| ^~~~~~~~~~~~~~~~~drivers/iommu/iommufd/driver.c:61:43: note: in expansion of macro 'PHYS_ADDR_MAX'
61 | PAGE_SIZE, PHYS_ADDR_MAX, GFP_KERNEL);
| ^~~~~~~~~~~~~
The mtree_alloc_range() interface explicitly operates on 'unsigned long' address values, so I don't see an immediate workaround for this that would make it work on these machines. On the other hand,
At the moment, the only drivers that select CONFIG_IOMMUFD_DRIVER on 32-bit Arm systems are CONFIG_PDS_VFIO_PCI and CONFIG_MLX5_VFIO_PCI. It's probably fine to make all three symbols "depends on 64BIT" for now, but I don't know if there may be more drivers like this in the future that actually could make sense on embedded systems.
Arnd
Jason Gunthorpe
maintainerRe: [PATCH v9 17/29] iommufd: Add mmap interface
Quoted 3 lines · Arnd Bergmann
The
mtree_alloc_range()interface explicitly operates on 'unsigned long' address values, so I don't see an immediate workaround for this that would make it work on these machines. On the other hand,
I fixed this already, PHYS_ADDR_MAX was just wrong, it should be ULONG_MAX. The thing wasn't a physaddr in the first place.
Jason
PATCH v9 18/29 iommufd/selftest: Add coverage for the new mmap interface
In reply to Nicolin ChenExtend the loopback test to a new mmap page.
--- drivers/iommu/iommufd/iommufd_test.h | 4 +++ tools/testing/selftests/iommu/iommufd_utils.h | 4 +++ drivers/iommu/iommufd/selftest.c | 33 ++++++++++++++++++- tools/testing/selftests/iommu/iommufd.c | 19 +++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-)
drivers/iommu/iommufd/iommufd_test.h +4 -0
@@ -232,12 +232,16 @@ struct iommu_hwpt_invalidate_selftest {* (IOMMU_VIOMMU_TYPE_SELFTEST) * @in_data: Input random data from user space * @out_data: Output data (matching @in_data) to user space +* @out_mmap_offset: The offset argument for mmap syscall +* @out_mmap_length: The length argument for mmap syscall * * Simply set @out_data=@in_data for a loopback test */ struct iommu_viommu_selftest {__u32 in_data;__u32 out_data; +__aligned_u64 out_mmap_offset; +__aligned_u64 out_mmap_length; };/* Should not be equal to any defined value in enum iommu_viommu_invalidate_data_type */
tools/testing/selftests/iommu/iommufd_utils.h +4 -0
@@ -56,6 +56,10 @@ static unsigned long PAGE_SIZE;#define offsetofend (TYPE ,MEMBER ) \ (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER)) +#define test_err_mmap (_errno, length, offset) \ + EXPECT_ERRNO(_errno, (long)mmap(NULL, length, PROT_READ | PROT_WRITE, \ + MAP_SHARED, self->fd, offset)) +static inline void *memfd_mmap (size_t length,int prot,int flags,int *mfd_p) {int mfd_flags = (flags &MAP_HUGETLB ) ?MFD_HUGETLB :0 ;
drivers/iommu/iommufd/selftest.c +32 -1
@@ -152,6 +152,9 @@ struct mock_viommu {struct mock_iommu_domain *s2_parent;struct mock_hw_queue *hw_queue[IOMMU_TEST_HW_QUEUE_MAX ];struct mutex queue_mutex; + +unsigned long mmap_offset; +u32 *page;/* Mmap page to test u32 type of in_data */ };static inline struct mock_viommu *to_mock_viommu (struct iommufd_viommu *viommu)
@@ -689,6 +692,10 @@ static void mock_viommu_destroy(struct iommufd_viommu *viommu)if (refcount_dec_and_test (&mock_iommu->users))complete (&mock_iommu->complete); +if (mock_viommu->mmap_offset) +iommufd_viommu_destroy_mmap (&mock_viommu->core, + mock_viommu->mmap_offset); +free_page ((unsigned long )mock_viommu->page);mutex_destroy (&mock_viommu->queue_mutex);/* iommufd core frees mock_viommu and viommu */
@@ -887,11 +894,28 @@ static int mock_viommu_init(struct iommufd_viommu *viommu,if (rc)return rc; +/* Allocate two pages */ + mock_viommu->page = + (u32 *)__get_free_pages (GFP_KERNEL | __GFP_ZERO,1 ); +if (!mock_viommu->page) +return -ENOMEM ; + + rc =iommufd_viommu_alloc_mmap (&mock_viommu->core, +__pa (mock_viommu->page), +PAGE_SIZE *2 , + &mock_viommu->mmap_offset); +if (rc) + goto err_free_page; + +/* For loopback tests on both the page and out_data */ + *mock_viommu->page = data.in_data; data.out_data = data.in_data; + data.out_mmap_length =PAGE_SIZE *2 ; + data.out_mmap_offset = mock_viommu->mmap_offset; rc =iommu_copy_struct_to_user ( user_data, &data,IOMMU_VIOMMU_TYPE_SELFTEST , out_data);if (rc) -return rc; + goto err_destroy_mmap; }refcount_inc (&mock_iommu->users);
@@ -900,6 +924,13 @@ static int mock_viommu_init(struct iommufd_viommu *viommu, viommu->ops = &mock_viommu_ops;return 0 ; + +err_destroy_mmap: +iommufd_viommu_destroy_mmap (&mock_viommu->core, + mock_viommu->mmap_offset); +err_free_page: +free_page ((unsigned long )mock_viommu->page); +return rc; }static const struct iommu_ops mock_ops = {
tools/testing/selftests/iommu/iommufd.c +19 -0
@@ -2799,6 +2799,7 @@ TEST_F(iommufd_viommu, viommu_alloc_with_data)struct iommu_viommu_selftest data = { .in_data =0xbeef , }; +uint32_t *test;if (!self->device_id)SKIP (return,"Skipping test for variant no_viommu" );
@@ -2807,6 +2808,24 @@ TEST_F(iommufd_viommu, viommu_alloc_with_data)IOMMU_VIOMMU_TYPE_SELFTEST , &data,sizeof (data), &self->viommu_id);ASSERT_EQ (data.out_data, data.in_data); + +/* Negative mmap tests -- offset and length cannot be changed */ +test_err_mmap (ENXIO , data.out_mmap_length, + data.out_mmap_offset +PAGE_SIZE ); +test_err_mmap (ENXIO , data.out_mmap_length, + data.out_mmap_offset +PAGE_SIZE *2 ); +test_err_mmap (ENXIO , data.out_mmap_length /2 , data.out_mmap_offset); +test_err_mmap (ENXIO , data.out_mmap_length *2 , data.out_mmap_offset); + +/* Now do a correct mmap for a loopback test */ + test =mmap (NULL , data.out_mmap_length,PROT_READ |PROT_WRITE , +MAP_SHARED , self->fd, data.out_mmap_offset); +ASSERT_NE (MAP_FAILED , test); +ASSERT_EQ (data.in_data, *test); + +/* The owner of the mmap region should be blocked */ +EXPECT_ERRNO (EBUSY ,_test_ioctl_destroy (self->fd, self->viommu_id)); +munmap (test, data.out_mmap_length); }TEST_F (iommufd_viommu, vdevice_alloc)
Signature · 1 line
2.43.0
Re: [PATCH v9 18/29] iommufd/selftest: Add coverage for the new mmap interface
Quoted 3 lines · Nicolin Chen
Extend the loopback test to a new mmap page.
Signed-off-by Nicolin Chen <[email protected]>
Quoted 152 lines
--- drivers/iommu/iommufd/iommufd_test.h | 4 +++ tools/testing/selftests/iommu/iommufd_utils.h | 4 +++ drivers/iommu/iommufd/selftest.c | 33 ++++++++++++++++++- tools/testing/selftests/iommu/iommufd.c | 19 +++++++++++ 4 files changed, 59 insertions(+), 1 deletion(-)drivers/iommu/iommufd/iommufd_test.h +4
-0@@ -232,12 +232,16 @@ struct iommu_hwpt_invalidate_selftest {* (IOMMU_VIOMMU_TYPE_SELFTEST) * @in_data: Input random data from user space * @out_data: Output data (matching @in_data) to user space +* @out_mmap_offset: The offset argument for mmap syscall +* @out_mmap_length: The length argument for mmap syscall * * Simply set @out_data=@in_data for a loopback test */ struct iommu_viommu_selftest {__u32 in_data;__u32 out_data; +__aligned_u64 out_mmap_offset; +__aligned_u64 out_mmap_length; };/* Should not be equal to any defined value in enum iommu_viommu_invalidate_data_type */ tools/testing/selftests/iommu/iommufd_utils.h +4
-0@@ -56,6 +56,10 @@ static unsigned long PAGE_SIZE;#define offsetofend (TYPE ,MEMBER ) \ (offsetof(TYPE, MEMBER) + sizeof_field(TYPE, MEMBER)) +#define test_err_mmap (_errno, length, offset) \ + EXPECT_ERRNO(_errno, (long)mmap(NULL, length, PROT_READ | PROT_WRITE, \ + MAP_SHARED, self->fd, offset)) +static inline void *memfd_mmap (size_t length,int prot,int flags,int *mfd_p) {int mfd_flags = (flags &MAP_HUGETLB ) ?MFD_HUGETLB :0 ;drivers/iommu/iommufd/selftest.c +32
-1@@ -152,6 +152,9 @@ struct mock_viommu {struct mock_iommu_domain *s2_parent;struct mock_hw_queue *hw_queue[IOMMU_TEST_HW_QUEUE_MAX ];struct mutex queue_mutex; + +unsigned long mmap_offset; +u32 *page;/* Mmap page to test u32 type of in_data */ };static inline struct mock_viommu *to_mock_viommu (struct iommufd_viommu *viommu)@@ -689,6 +692,10 @@ static void mock_viommu_destroy(struct iommufd_viommu *viommu)if (refcount_dec_and_test (&mock_iommu->users))complete (&mock_iommu->complete); +if (mock_viommu->mmap_offset) +iommufd_viommu_destroy_mmap (&mock_viommu->core, + mock_viommu->mmap_offset); +free_page ((unsigned long )mock_viommu->page);mutex_destroy (&mock_viommu->queue_mutex);/* iommufd core frees mock_viommu and viommu */ @@ -887,11 +894,28 @@ static int mock_viommu_init(struct iommufd_viommu *viommu,if (rc)return rc; +/* Allocate two pages */ + mock_viommu->page = + (u32 *)__get_free_pages (GFP_KERNEL | __GFP_ZERO,1 ); +if (!mock_viommu->page) +return -ENOMEM ; + + rc =iommufd_viommu_alloc_mmap (&mock_viommu->core, +__pa (mock_viommu->page), +PAGE_SIZE *2 , + &mock_viommu->mmap_offset); +if (rc) + goto err_free_page; + +/* For loopback tests on both the page and out_data */ + *mock_viommu->page = data.in_data; data.out_data = data.in_data; + data.out_mmap_length =PAGE_SIZE *2 ; + data.out_mmap_offset = mock_viommu->mmap_offset; rc =iommu_copy_struct_to_user ( user_data, &data,IOMMU_VIOMMU_TYPE_SELFTEST , out_data);if (rc) -return rc; + goto err_destroy_mmap; }refcount_inc (&mock_iommu->users);@@ -900,6 +924,13 @@ static int mock_viommu_init(struct iommufd_viommu *viommu, viommu->ops = &mock_viommu_ops;return 0 ; + +err_destroy_mmap: +iommufd_viommu_destroy_mmap (&mock_viommu->core, + mock_viommu->mmap_offset); +err_free_page: +free_page ((unsigned long )mock_viommu->page); +return rc; }static const struct iommu_ops mock_ops = {tools/testing/selftests/iommu/iommufd.c +19
-0@@ -2799,6 +2799,7 @@ TEST_F(iommufd_viommu, viommu_alloc_with_data)struct iommu_viommu_selftest data = { .in_data =0xbeef , }; +uint32_t *test;if (!self->device_id)SKIP (return,"Skipping test for variant no_viommu" );@@ -2807,6 +2808,24 @@ TEST_F(iommufd_viommu, viommu_alloc_with_data)IOMMU_VIOMMU_TYPE_SELFTEST , &data,sizeof (data), &self->viommu_id);ASSERT_EQ (data.out_data, data.in_data); + +/* Negative mmap tests -- offset and length cannot be changed */ +test_err_mmap (ENXIO , data.out_mmap_length, + data.out_mmap_offset +PAGE_SIZE ); +test_err_mmap (ENXIO , data.out_mmap_length, + data.out_mmap_offset +PAGE_SIZE *2 ); +test_err_mmap (ENXIO , data.out_mmap_length /2 , data.out_mmap_offset); +test_err_mmap (ENXIO , data.out_mmap_length *2 , data.out_mmap_offset); + +/* Now do a correct mmap for a loopback test */ + test =mmap (NULL , data.out_mmap_length,PROT_READ |PROT_WRITE , +MAP_SHARED , self->fd, data.out_mmap_offset); +ASSERT_NE (MAP_FAILED , test); +ASSERT_EQ (data.in_data, *test); + +/* The owner of the mmap region should be blocked */ +EXPECT_ERRNO (EBUSY ,_test_ioctl_destroy (self->fd, self->viommu_id)); +munmap (test, data.out_mmap_length); }TEST_F (iommufd_viommu, vdevice_alloc)Signature · 1 line
2.43.0
PATCH v9 19/29 Documentation: userspace-api: iommufd: Update HW QUEUE
In reply to Nicolin ChenWith the introduction of the new object and its infrastructure, update the doc to reflect that.
--- Documentation/userspace-api/iommufd.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Documentation/userspace-api/iommufd.rst +12 -0
@@ -124,6 +124,17 @@ Following IOMMUFD objects are exposed to userspace: used to allocate a vEVENTQ. Each vIOMMU can support multiple types of vEVENTS, but is confined to one vEVENTQ per vEVENTQ type. +- IOMMUFD_OBJ_HW_QUEUE, representing a hardware accelerated queue, as a subset + of IOMMU's virtualization features, for the IOMMU HW to directly read or write + the virtual queue memory owned by a guest OS. This HW-acceleration feature can + allow VM to work with the IOMMU HW directly without a VM Exit, so as to reduce + overhead from the hypercalls. Along with the HW QUEUE object, iommufd provides + user space an mmap interface for VMM to mmap a physical MMIO region from the + host physical address space to the guest physical address space, allowing the + guest OS to directly control the allocated HW QUEUE. Thus, when allocating a + HW QUEUE, the VMM must request a pair of mmap info (offset/length) and pass in + exactly to an mmap syscall via its offset and length arguments. + All user-visible objects are destroyed via the IOMMU_DESTROY uAPI. The diagrams below show relationships between user-visible objects and kernel
@@ -270,6 +281,7 @@ User visible objects are backed by following datastructures: - iommufd_viommu for IOMMUFD_OBJ_VIOMMU. - iommufd_vdevice for IOMMUFD_OBJ_VDEVICE. - iommufd_veventq for IOMMUFD_OBJ_VEVENTQ. +- iommufd_hw_queue for IOMMUFD_OBJ_HW_QUEUE. Several terminologies when looking at these datastructures:
Signature · 1 line
2.43.0
PATCH v9 20/29 iommu: Allow an input type in hw_info op
In reply to Nicolin ChenThe hw_info uAPI will support a bidirectional data_type field that can be used as an input field for user space to request for a specific info data.
To prepare for the uAPI update, change the iommu layer first:
- Add a new IOMMU_HW_INFO_TYPE_DEFAULT as an input, for which driver can output its only (or firstly) supported type
- Update the kdoc accordingly
- Roll out the type validation in the existing drivers
--- include/linux/iommu.h | 3 ++- include/uapi/linux/iommufd.h | 4 +++- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 4 ++++ drivers/iommu/intel/iommu.c | 4 ++++ drivers/iommu/iommufd/device.c | 3 +++ drivers/iommu/iommufd/selftest.c | 4 ++++ 6 files changed, 20 insertions(+), 2 deletions(-)
include/linux/iommu.h +2 -1
@@ -603,7 +603,8 @@ __iommu_copy_struct_to_user(const struct iommu_user_data *dst_data,* @capable: check capability * @hw_info: report iommu hardware information. The data buffer returned by this * op is allocated in the iommu driver and freed by the caller after -* use. +* use. @type can input a requested type and output a supported type. +* Driver should reject an unsupported data @type input * @domain_alloc: Do not use in new drivers * @domain_alloc_identity: allocate an IDENTITY domain. Drivers should prefer to * use identity_domain instead. This should only be used
include/uapi/linux/iommufd.h +3 -1
@@ -593,13 +593,15 @@ struct iommu_hw_info_arm_smmuv3 {/** * enum iommu_hw_info_type - IOMMU Hardware Info Types -* @IOMMU_HW_INFO_TYPE_NONE: Used by the drivers that do not report hardware +* @IOMMU_HW_INFO_TYPE_NONE: Output by the drivers that do not report hardware * info +* @IOMMU_HW_INFO_TYPE_DEFAULT: Input to request for a default type * @IOMMU_HW_INFO_TYPE_INTEL_VTD: Intel VT-d iommu info type * @IOMMU_HW_INFO_TYPE_ARM_SMMUV3: ARM SMMUv3 iommu info type */ enum iommu_hw_info_type {IOMMU_HW_INFO_TYPE_NONE =0 , +IOMMU_HW_INFO_TYPE_DEFAULT =0 ,IOMMU_HW_INFO_TYPE_INTEL_VTD =1 ,IOMMU_HW_INFO_TYPE_ARM_SMMUV3 =2 , };
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +4 -0
@@ -15,6 +15,10 @@ void *arm_smmu_hw_info(struct device *dev, u32 *length,u32 __iomem *base_idr;unsigned int i; +if (*type !=IOMMU_HW_INFO_TYPE_DEFAULT && + *type !=IOMMU_HW_INFO_TYPE_ARM_SMMUV3 ) +return ERR_PTR (-EOPNOTSUPP ); + info =kzalloc (sizeof (*info),GFP_KERNEL );if (!info)return ERR_PTR (-ENOMEM );
drivers/iommu/intel/iommu.c +4 -0
@@ -4098,6 +4098,10 @@ static void *intel_iommu_hw_info(struct device *dev, u32 *length,struct intel_iommu *iommu = info->iommu;struct iommu_hw_info_vtd *vtd; +if (*type !=IOMMU_HW_INFO_TYPE_DEFAULT && + *type !=IOMMU_HW_INFO_TYPE_INTEL_VTD ) +return ERR_PTR (-EOPNOTSUPP ); + vtd =kzalloc (sizeof (*vtd),GFP_KERNEL );if (!vtd)return ERR_PTR (-ENOMEM );
drivers/iommu/iommufd/device.c +3 -0
@@ -1512,6 +1512,9 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd) cmd->__reserved[2])return -EOPNOTSUPP ; +/* Clear the type field since drivers don't support a random input */ + cmd->out_data_type =IOMMU_HW_INFO_TYPE_DEFAULT ; + idev =iommufd_get_device (ucmd, cmd->dev_id);if (IS_ERR (idev))return PTR_ERR (idev);
drivers/iommu/iommufd/selftest.c +4 -0
@@ -310,6 +310,10 @@ static void *mock_domain_hw_info(struct device *dev, u32 *length, {struct iommu_test_hw_info *info; +if (*type !=IOMMU_HW_INFO_TYPE_DEFAULT && + *type !=IOMMU_HW_INFO_TYPE_SELFTEST ) +return ERR_PTR (-EOPNOTSUPP ); + info =kzalloc (sizeof (*info),GFP_KERNEL );if (!info)return ERR_PTR (-ENOMEM );
Signature · 1 line
2.43.0
PATCH v9 21/29 iommufd: Allow an input data_type via iommu_hw_info
In reply to Nicolin ChenThe iommu_hw_info can output via the out_data_type field the vendor data type from a driver, but this only allows driver to report one data type.
Now, with SMMUv3 having a Tegra241 CMDQV implementation, it has two sets of types and data structs to report.
One way to support that is to use the same type field bidirectionally.
Reuse the same field by adding an "in_data_type", allowing user space to request for a specific type and to get the corresponding data.
For backward compatibility, since the ioctl handler has never checked an input value, add an IOMMU_HW_INFO_FLAG_INPUT_TYPE to switch between the old output-only field and the new bidirectional field.
--- include/uapi/linux/iommufd.h | 20 +++++++++++++++++++- drivers/iommu/iommufd/device.c | 9 ++++++--- 2 files changed, 25 insertions(+), 4 deletions(-)
include/uapi/linux/iommufd.h +19 -1
@@ -628,6 +628,15 @@ enum iommufd_hw_capabilities { IOMMU_HW_CAP_PCI_PASID_PRIV = 1 << 2, }; +/** +* enum iommufd_hw_info_flags - Flags for iommu_hw_info +* @IOMMU_HW_INFO_FLAG_INPUT_TYPE: If set, @in_data_type carries an input type +* for user space to request for a specific info +*/ +enum iommufd_hw_info_flags { +IOMMU_HW_INFO_FLAG_INPUT_TYPE =1 <<0 , +}; +/** * struct iommu_hw_info - ioctl(IOMMU_GET_HW_INFO) * @size: sizeof(struct iommu_hw_info)
@@ -637,6 +646,12 @@ enum iommufd_hw_capabilities {* data that kernel supports * @data_uptr: User pointer to a user-space buffer used by the kernel to fill * the iommu type specific hardware information data +* @in_data_type: This shares the same field with @out_data_type, making it be +* a bidirectional field. When IOMMU_HW_INFO_FLAG_INPUT_TYPE is +* set, an input type carried via this @in_data_type field will +* be valid, requesting for the info data to the given type. If +* IOMMU_HW_INFO_FLAG_INPUT_TYPE is unset, any input value will +* be seen as IOMMU_HW_INFO_TYPE_DEFAULT * @out_data_type: Output the iommu hardware info type as defined in the enum * iommu_hw_info_type. * @out_capabilities: Output the generic iommu capability info type as defined
@@ -666,7 +681,10 @@ struct iommu_hw_info {__u32 dev_id;__u32 data_len;__aligned_u64 data_uptr; -__u32 out_data_type; +union { +__u32 in_data_type; +__u32 out_data_type; + };__u8 out_max_pasid_log2;__u8 __reserved[3 ];__aligned_u64 out_capabilities;
drivers/iommu/iommufd/device.c +6 -3
@@ -1499,6 +1499,7 @@ EXPORT_SYMBOL_NS_GPL(iommufd_access_rw, "IOMMUFD");int iommufd_get_hw_info (struct iommufd_ucmd *ucmd) { +const u32 SUPPORTED_FLAGS =IOMMU_HW_INFO_FLAG_INPUT_TYPE ;struct iommu_hw_info *cmd = ucmd->cmd;void __user *user_ptr =u64_to_user_ptr (cmd->data_uptr);const struct iommu_ops *ops;
@@ -1508,12 +1509,14 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd)void *data;int rc; -if (cmd->flags || cmd->__reserved[0 ] || cmd->__reserved[1 ] || - cmd->__reserved[2 ]) +if (cmd->flags & ~SUPPORTED_FLAGS ) +return -EOPNOTSUPP ; +if (cmd->__reserved[0 ] || cmd->__reserved[1 ] || cmd->__reserved[2 ])return -EOPNOTSUPP ;/* Clear the type field since drivers don't support a random input */ - cmd->out_data_type =IOMMU_HW_INFO_TYPE_DEFAULT ; +if (!(cmd->flags &IOMMU_HW_INFO_FLAG_INPUT_TYPE )) + cmd->in_data_type =IOMMU_HW_INFO_TYPE_DEFAULT ; idev =iommufd_get_device (ucmd, cmd->dev_id);if (IS_ERR (idev))
Signature · 1 line
2.43.0
PATCH v9 22/29 iommufd/selftest: Update hw_info coverage for an input data_type
In reply to Nicolin ChenTest both IOMMU_HW_INFO_TYPE_DEFAULT and IOMMU_HW_INFO_TYPE_SELFTEST, and add a negative test for an unsupported type.
Also drop the unused mask in test_cmd_get_hw_capabilities() as checkpatch is complaining.
--- tools/testing/selftests/iommu/iommufd_utils.h | 33 +++++++++++-------- tools/testing/selftests/iommu/iommufd.c | 32 +++++++++++++----- .../selftests/iommu/iommufd_fail_nth.c | 4 +-- 3 files changed, 46 insertions(+), 23 deletions(-)
tools/testing/selftests/iommu/iommufd_utils.h +20 -13
@@ -761,20 +761,24 @@ static void teardown_iommufd(int fd, struct __test_metadata *_metadata)#endif /* @data can be NULL */ -static int _test_cmd_get_hw_info (int fd,__u32 device_id,void *data, -size_t data_len,uint32_t *capabilities, -uint8_t *max_pasid) +static int _test_cmd_get_hw_info (int fd,__u32 device_id,__u32 data_type, +void *data,size_t data_len, +uint32_t *capabilities,uint8_t *max_pasid) {struct iommu_test_hw_info *info = (struct iommu_test_hw_info *)data;struct iommu_hw_info cmd = { .size =sizeof (cmd), .dev_id = device_id, .data_len = data_len, + .in_data_type = data_type, .data_uptr = (uint64_t )data, .out_capabilities =0 , };int ret; +if (data_type !=IOMMU_HW_INFO_TYPE_DEFAULT ) + cmd.flags |=IOMMU_HW_INFO_FLAG_INPUT_TYPE ; + ret =ioctl (fd,IOMMU_GET_HW_INFO , &cmd);if (ret)return ret;
@@ -817,20 +821,23 @@ static int _test_cmd_get_hw_info(int fd, __u32 device_id, void *data,return 0 ; } -#define test_cmd_get_hw_info (device_id, data, data_len) \ - ASSERT_EQ(0, _test_cmd_get_hw_info(self->fd, device_id, data, \ - data_len, NULL, NULL)) +#define test_cmd_get_hw_info (device_id, data_type, data, data_len) \ + ASSERT_EQ(0, _test_cmd_get_hw_info(self->fd, device_id, data_type, \ + data, data_len, NULL, NULL)) -#define test_err_get_hw_info (_errno, device_id, data, data_len) \ - EXPECT_ERRNO(_errno, _test_cmd_get_hw_info(self->fd, device_id, data, \ - data_len, NULL, NULL)) +#define test_err_get_hw_info (_errno, device_id, data_type, data, data_len) \ + EXPECT_ERRNO(_errno, \ + _test_cmd_get_hw_info(self->fd, device_id, data_type, \ + data, data_len, NULL, NULL)) -#define test_cmd_get_hw_capabilities (device_id, caps, mask) \ - ASSERT_EQ(0, _test_cmd_get_hw_info(self->fd, device_id, NULL, \ +#define test_cmd_get_hw_capabilities (device_id, caps) \ + ASSERT_EQ(0, _test_cmd_get_hw_info(self->fd, device_id, \ + IOMMU_HW_INFO_TYPE_DEFAULT, NULL, \ 0, &caps, NULL)) -#define test_cmd_get_hw_info_pasid (device_id, max_pasid) \ - ASSERT_EQ(0, _test_cmd_get_hw_info(self->fd, device_id, NULL, \ +#define test_cmd_get_hw_info_pasid (device_id, max_pasid) \ + ASSERT_EQ(0, _test_cmd_get_hw_info(self->fd, device_id, \ + IOMMU_HW_INFO_TYPE_DEFAULT, NULL, \ 0, NULL, max_pasid))static int _test_ioctl_fault_alloc (int fd,__u32 *fault_id,__u32 *fault_fd)
tools/testing/selftests/iommu/iommufd.c +24 -8
@@ -764,19 +764,34 @@ TEST_F(iommufd_ioas, get_hw_info)uint8_t max_pasid =0 ;/* Provide a zero-size user_buffer */ -test_cmd_get_hw_info (self->device_id,NULL ,0 ); +test_cmd_get_hw_info (self->device_id, +IOMMU_HW_INFO_TYPE_DEFAULT ,NULL ,0 );/* Provide a user_buffer with exact size */ -test_cmd_get_hw_info (self->device_id, &buffer_exact,sizeof (buffer_exact)); +test_cmd_get_hw_info (self->device_id, +IOMMU_HW_INFO_TYPE_DEFAULT , &buffer_exact, +sizeof (buffer_exact)); + +/* Request for a wrong data_type, and a correct one */ +test_err_get_hw_info (EOPNOTSUPP , self->device_id, +IOMMU_HW_INFO_TYPE_SELFTEST +1 , + &buffer_exact,sizeof (buffer_exact)); +test_cmd_get_hw_info (self->device_id, +IOMMU_HW_INFO_TYPE_SELFTEST , &buffer_exact, +sizeof (buffer_exact));/* * Provide a user_buffer with size larger than the exact size to check if * kernel zero the trailing bytes. */ -test_cmd_get_hw_info (self->device_id, &buffer_larger,sizeof (buffer_larger)); +test_cmd_get_hw_info (self->device_id, +IOMMU_HW_INFO_TYPE_DEFAULT , &buffer_larger, +sizeof (buffer_larger));/* * Provide a user_buffer with size smaller than the exact size to check if * the fields within the size range still gets updated. */ -test_cmd_get_hw_info (self->device_id, &buffer_smaller,sizeof (buffer_smaller)); +test_cmd_get_hw_info (self->device_id, +IOMMU_HW_INFO_TYPE_DEFAULT , + &buffer_smaller,sizeof (buffer_smaller));test_cmd_get_hw_info_pasid (self->device_id, &max_pasid);ASSERT_EQ (0 , max_pasid);if (variant->pasid_capable) {
@@ -786,9 +801,11 @@ TEST_F(iommufd_ioas, get_hw_info) } }else {test_err_get_hw_info (ENOENT , self->device_id, - &buffer_exact,sizeof (buffer_exact)); +IOMMU_HW_INFO_TYPE_DEFAULT , &buffer_exact, +sizeof (buffer_exact));test_err_get_hw_info (ENOENT , self->device_id, - &buffer_larger,sizeof (buffer_larger)); +IOMMU_HW_INFO_TYPE_DEFAULT , &buffer_larger, +sizeof (buffer_larger)); } }
@@ -2175,8 +2192,7 @@ TEST_F(iommufd_dirty_tracking, device_dirty_capability)test_cmd_hwpt_alloc (self->idev_id, self->ioas_id,0 , &hwpt_id);test_cmd_mock_domain (hwpt_id, &stddev_id,NULL ,NULL ); -test_cmd_get_hw_capabilities (self->idev_id, caps, -IOMMU_HW_CAP_DIRTY_TRACKING ); +test_cmd_get_hw_capabilities (self->idev_id, caps);ASSERT_EQ (IOMMU_HW_CAP_DIRTY_TRACKING , caps &IOMMU_HW_CAP_DIRTY_TRACKING );
tools/testing/selftests/iommu/iommufd_fail_nth.c +2 -2
@@ -667,8 +667,8 @@ TEST_FAIL_NTH(basic_fail_nth, device) &self->stdev_id, NULL, &idev_id))return -1 ; -if (_test_cmd_get_hw_info (self->fd, idev_id, &info, -sizeof (info),NULL ,NULL )) +if (_test_cmd_get_hw_info (self->fd, idev_id,IOMMU_HW_INFO_TYPE_DEFAULT , + &info,sizeof (info),NULL ,NULL ))return -1 ; if (_test_cmd_hwpt_alloc(self->fd, idev_id, ioas_id, 0,
Signature · 1 line
2.43.0
Re: [PATCH v9 22/29] iommufd/selftest: Update hw_info coverage for an input data_type
Quoted 7 lines · Nicolin Chen
Test both
IOMMU_HW_INFO_TYPE_DEFAULTandIOMMU_HW_INFO_TYPE_SELFTEST, and add a negative test for an unsupported type.Also drop the unused mask in
test_cmd_get_hw_capabilities()as checkpatch is complaining.Signed-off-by Nicolin Chen <[email protected]>
Note: I see a couple of warnings while building the selftests on Jason's tree, but these seem unrelated to this series:
make -C tools/testing/selftests TARGETS=iommu
make: Entering directory '/usr/local/google/home/praan/master/iommufd/iommufd/tools/testing/selftests'
CC iommufd
In file included from iommufd.c:12:
In function ‘_test_ioctl_ioas_map_file’,
inlined from ‘iommufd_mock_domain_all_aligns’ at iommufd.c:1789:5,
inlined from ‘wrapper_iommufd_mock_domain_all_aligns’ at iommufd.c:1757:1:
iommufd_utils.h:679:36: warning: ‘mfd’ may be used uninitialized [-Wmaybe-uninitialized]
679 | struct iommu_ioas_map_file cmd = {
| ^~~iommufd.c: In function ‘wrapper_iommufd_mock_domain_all_aligns’:
iommufd.c:1766:13: note: ‘mfd’ was declared here
1766 | int mfd;
| ^~~
In function ‘_test_ioctl_ioas_map_file’,
inlined from ‘iommufd_mock_domain_all_aligns_copy’ at iommufd.c:1843:5,
inlined from ‘wrapper_iommufd_mock_domain_all_aligns_copy’ at iommufd.c:1809:1:
iommufd_utils.h:679:36: warning: ‘mfd’ may be used uninitialized [-Wmaybe-uninitialized]
679 | struct iommu_ioas_map_file cmd = {
| ^~~iommufd.c: In function ‘wrapper_iommufd_mock_domain_all_aligns_copy’:
iommufd.c:1818:13: note: ‘mfd’ was declared here
1818 | int mfd;
| ^~~
In file included from iommufd_utils.h:11:
In function ‘_test_cmd_get_hw_info’,
inlined from ‘iommufd_ioas_get_hw_info’ at iommufd.c:792:3,
inlined from ‘wrapper_iommufd_ioas_get_hw_info’ at iommufd.c:752:1:
iommufd_utils.h:812:37: warning: array subscript ‘struct iommu_test_hw_info[0]’ is partly outside array bounds of ‘struct iommu_test_hw_info_buffer_smaller[1]’ [-Warray-bounds=]
812 | assert(!info->flags);
| ~~~~^~~~~~~
iommufd.c: In function ‘wrapper_iommufd_ioas_get_hw_info’:
iommufd.c:761:11: note: object ‘buffer_smaller’ of size 4
761 | } buffer_smaller;
| ^~~~~~~~~~~~~~ CC iommufd_fail_nth
make: Leaving directory '/usr/local/google/home/praan/master/iommufd/iommufd/tools/testing/selftests'
Quoted 6 lines
--- tools/testing/selftests/iommu/iommufd_utils.h | 33 +++++++++++-------- tools/testing/selftests/iommu/iommufd.c | 32 +++++++++++++----- .../selftests/iommu/iommufd_fail_nth.c | 4 +-- 3 files changed, 46 insertions(+), 23 deletions(-)
[...]
Quoted 3 lines
Signature · 1 line
2.43.0
Thanks,
Praan
Jason Gunthorpe
maintainerRe: [PATCH v9 22/29] iommufd/selftest: Update hw_info coverage for an input data_type
Quoted 7 lines · Pranjal Shrivastava
Quoted 6 lines · Nicolin Chen
Test both
IOMMU_HW_INFO_TYPE_DEFAULTandIOMMU_HW_INFO_TYPE_SELFTEST, and add a negative test for an unsupported type.Also drop the unused mask in
test_cmd_get_hw_capabilities()as checkpatch is complaining.
Quoted 6 lines
Quoted 1 line
Signed-off-by Nicolin Chen <[email protected]>Reviewed-by Pranjal Shrivastava <[email protected]>Note: I see a couple of warnings while building the selftests on Jason's tree, but these seem unrelated to this series:
This is fixed in v6.16-rc5
Jason
Re: [PATCH v9 22/29] iommufd/selftest: Update hw_info coverage for an input data_type
Quoted 8 lines · Jason Gunthorpe
Quoted 7 lines · Pranjal Shrivastava
Quoted 6 lines · Nicolin Chen
Test both
IOMMU_HW_INFO_TYPE_DEFAULTandIOMMU_HW_INFO_TYPE_SELFTEST, and add a negative test for an unsupported type.Also drop the unused mask in
test_cmd_get_hw_capabilities()as checkpatch is complaining.
Quoted 9 lines
Quoted 6 lines
Quoted 1 line
Signed-off-by Nicolin Chen <[email protected]>Reviewed-by Pranjal Shrivastava <[email protected]>Note: I see a couple of warnings while building the selftests on Jason's tree, but these seem unrelated to this series:
This is fixed in v6.16-rc5
Alright, this was on the for-next branch when the head was at:3e2a9811f6a9cefd310cc33cab73d5435b4a4caa
iommufd: Apply the new iommufd_object_alloc_ucmd helper
But I see that on for-rc [1] the fixes are merged.
Quoted 1 line
Jason
Thanks
Praan
[1] https://git.kernel.org/pub/scm/linux/kernel/git/jgg/iommufd.git/commit/?h=for-rc
Jason Gunthorpe
maintainerRe: [PATCH v9 22/29] iommufd/selftest: Update hw_info coverage for an input data_type
Quoted 6 lines · Pranjal Shrivastava
On Thu, Jul 10, 2025 at 12:32:02PM -0300, Jason Gunthorpe wrote:
Alright, this was on thefor-nextbranch when the head was at:3e2a9811f6a9cefd310cc33cab73d5435b4a4caa
iommufd: Apply the newiommufd_object_alloc_ucmdhelperBut I see that on
for-rc[1] the fixes are merged.
Yes, Linus discourages back merging rc without a going forward justification, like following patches need the rc patches. Fixes some bug is not a justification...
Jason
Re: [PATCH v9 22/29] iommufd/selftest: Update hw_info coverage for an input data_type
Quoted 6 lines · Jason Gunthorpe
Quoted 5 lines · Pranjal Shrivastava
On Thu, Jul 10, 2025 at 12:32:02PM -0300, Jason Gunthorpe wrote:
Alright, this was on thefor-nextbranch when the head was at:3e2a9811f6a9cefd310cc33cab73d5435b4a4caa
iommufd: Apply the newiommufd_object_alloc_ucmdhelper
Quoted 6 lines
Quoted 1 line
But I see that on
for-rc[1] the fixes are merged.Yes, Linus discourages back merging rc without a going forward justification, like following patches need the rc patches. Fixes some bug is not a justification...
Ack. Thanks for the clarifying!
Quoted 1 line
Jason
Praan
PATCH v9 23/29 iommu/arm-smmu-v3-iommufd: Add vsmmu_size/type and vsmmu_init impl ops
In reply to Nicolin ChenAn impl driver might want to allocate its own type of vIOMMU object or the standard IOMMU_VIOMMU_TYPE_ARM_SMMUV3 by setting up its own SW/HW bits, as the tegra241-cmdqv driver will add IOMMU_VIOMMU_TYPE_TEGRA241_CMDQV.
Add vsmmu_size/type and vsmmu_init to struct arm_smmu_impl_ops. Prioritize them in arm_smmu_get_viommu_size() and arm_vsmmu_init().
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 5 +++++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 8 ++++++++ 2 files changed, 13 insertions(+)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +5 -0
@@ -16,6 +16,7 @@#include <linux/sizes.h> struct arm_smmu_device ; +struct arm_vsmmu ;/* MMIO registers */ #define ARM_SMMU_IDR0 0x0
@@ -720,6 +721,10 @@ struct arm_smmu_impl_ops {int (*init_structures)(struct arm_smmu_device *smmu);struct arm_smmu_cmdq *(*get_secondary_cmdq)(struct arm_smmu_device *smmu,struct arm_smmu_cmdq_ent *ent); +const size_t vsmmu_size; +const enum iommu_viommu_type vsmmu_type; +int (*vsmmu_init)(struct arm_vsmmu *vsmmu, +const struct iommu_user_data *user_data); };/* An SMMUv3 instance */
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +8 -0
@@ -416,6 +416,10 @@ size_t arm_smmu_get_viommu_size(struct device *dev, !(smmu->features & ARM_SMMU_FEAT_S2FWB))return 0 ; +if (smmu->impl_ops && smmu->impl_ops->vsmmu_size && + viommu_type == smmu->impl_ops->vsmmu_type) +return smmu->impl_ops->vsmmu_size; +if (viommu_type !=IOMMU_VIOMMU_TYPE_ARM_SMMUV3 )return 0 ;
@@ -439,6 +443,10 @@ int arm_vsmmu_init(struct iommufd_viommu *viommu,/* FIXME Move VMID allocation from the S2 domain allocation to here */ vsmmu->vmid = s2_parent->s2_cfg.vmid; +if (smmu->impl_ops && smmu->impl_ops->vsmmu_init && + viommu->type == smmu->impl_ops->vsmmu_type) +return smmu->impl_ops->vsmmu_init (vsmmu, user_data); + viommu->ops = &arm_vsmmu_ops;return 0 ; }
Signature · 1 line
2.43.0
Will Deacon
maintainerRe: [PATCH v9 23/29] iommu/arm-smmu-v3-iommufd: Add vsmmu_size/type and vsmmu_init impl ops
Hey Nicolin,
Quoted 29 lines · Nicolin Chen
An impl driver might want to allocate its own type of vIOMMU object or the standard
IOMMU_VIOMMU_TYPE_ARM_SMMUV3by setting up its own SW/HW bits, as the tegra241-cmdqv driver will addIOMMU_VIOMMU_TYPE_TEGRA241_CMDQV.Add vsmmu_size/type and
vsmmu_initto structarm_smmu_impl_ops. Prioritize them inarm_smmu_get_viommu_size()andarm_vsmmu_init().Reviewed-by Pranjal Shrivastava <[email protected]>Reviewed-by Kevin Tian <[email protected]>Reviewed-by Jason Gunthorpe <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 5 +++++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 8 ++++++++ 2 files changed, 13 insertions(+)drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +1
-0@@ -16,6 +16,7 @@#include <linux/sizes.h> struct arm_smmu_device ; +struct arm_vsmmu ;/* MMIO registers */ #define ARM_SMMU_IDR0 0x0
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h struct arm_smmu_impl_ops { · line 727
Quoted 7 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +4
-0@@ -720,6 +721,10 @@ struct arm_smmu_impl_ops {int (*init_structures)(struct arm_smmu_device *smmu);struct arm_smmu_cmdq *(*get_secondary_cmdq)(struct arm_smmu_device *smmu,struct arm_smmu_cmdq_ent *ent); +const size_t vsmmu_size; +const enum iommu_viommu_type vsmmu_type; +int (*vsmmu_init)(struct arm_vsmmu *vsmmu, +const struct iommu_user_data *user_data);
It would be nice to avoid adding data members to the ops structure, if at all possible. The easiest thing would probably be to add a function for getting the vsmmu size and then pushing the two checks against 'vsmmu_type' down into the impl_ops callbacks so that:
1. If the type is IOMMU_VIOMMU_TYPE_ARM_SMMUV3, we don't bother with the impl_ops at all in arm_vsmmu_init() and arm_smmu_get_viommu_size()
2. Otherwise, we pass the type into the impl_ops and they can check it
Of course, that can be a patch on top of the series as there's no point respinning the whole just for this.
Cheers,
Will
Re: [PATCH v9 23/29] iommu/arm-smmu-v3-iommufd: Add vsmmu_size/type and vsmmu_init impl ops
Hi Will,
Sorry for the late response.
Quoted 4 lines · Will Deacon
Quoted 4 lines
/* MMIO registers */
#defineARM_SMMU_IDR00x0drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +0
-0@@ -720,6 +721,10 @@ struct arm_smmu_impl_ops {int (*init_structures)(struct arm_smmu_device *smmu);
Quoted 8 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h struct arm_smmu_impl_ops { · line 727
Quoted 6 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +4
-0struct arm_smmu_cmdq *(*get_secondary_cmdq)(struct arm_smmu_device *smmu,struct arm_smmu_cmdq_ent *ent); +const size_t vsmmu_size; +const enum iommu_viommu_type vsmmu_type; +int (*vsmmu_init)(struct arm_vsmmu *vsmmu, +const struct iommu_user_data *user_data);It would be nice to avoid adding data members to the ops structure, if
You mean the "vsmmu_size" and "vsmmu_type" right?
So you want them to be removed, by having two impl_ops:
size_t get_vsmmu_size(enum iommu_viommu_type vsmmu_type); int (*vsmmu_init)(struct arm_vsmmu *vsmmu, const struct iommu_user_data *user_data);
right?
Quoted 6 lines
at all possible. The easiest thing would probably be to add a function for getting the vsmmu size and then pushing the two checks against '
vsmmu_type' down into theimpl_opscallbacks so that:1. If the type is
IOMMU_VIOMMU_TYPE_ARM_SMMUV3, we don't bother with theimpl_opsat all inarm_vsmmu_init()andarm_smmu_get_viommu_size()
Hmm, I was hoping for an implementation could support the default IOMMU_VIOMMU_TYPE_ARM_SMMUV3 while having its own viommu_ops or so. But I think your suggestion is fine since there is no such a use case at this moment :)
Quoted 4 lines
2. Otherwise, we pass the type into the
impl_opsand they can check itOf course, that can be a patch on top of the series as there's no point respinning the whole just for this.
Thanks for that! I can draft a patch to send later this week once the requirements are confirmed.
Nicolin
Will Deacon
maintainerRe: [PATCH v9 23/29] iommu/arm-smmu-v3-iommufd: Add vsmmu_size/type and vsmmu_init impl ops
Hi Nicolin,
Quoted 1 line · Nicolin Chen
Sorry for the late response.
No problem at all.
Quoted 14 lines · Will Deacon
Quoted 4 lines
Quoted 4 lines
/* MMIO registers */
#defineARM_SMMU_IDR00x0drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +0
-0@@ -720,6 +721,10 @@ struct arm_smmu_impl_ops {int (*init_structures)(struct arm_smmu_device *smmu);Quoted 8 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h struct arm_smmu_impl_ops { · line 727
Quoted 6 lines
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +4
-0struct arm_smmu_cmdq *(*get_secondary_cmdq)(struct arm_smmu_device *smmu,struct arm_smmu_cmdq_ent *ent); +const size_t vsmmu_size; +const enum iommu_viommu_type vsmmu_type; +int (*vsmmu_init)(struct arm_vsmmu *vsmmu, +const struct iommu_user_data *user_data);It would be nice to avoid adding data members to the ops structure, if
Quoted 1 line
You mean the "
vsmmu_size" and "vsmmu_type" right?
Yup.
Quoted 6 lines
So you want them to be removed, by having two
impl_ops:size_t get_vsmmu_size(enum iommu_viommu_type vsmmu_type); int (*vsmmu_init)(struct arm_vsmmu *vsmmu, const struct iommu_user_data *user_data);right?
Yes, please.
Quoted 12 lines
Quoted 6 lines
at all possible. The easiest thing would probably be to add a function for getting the vsmmu size and then pushing the two checks against '
vsmmu_type' down into theimpl_opscallbacks so that:1. If the type is
IOMMU_VIOMMU_TYPE_ARM_SMMUV3, we don't bother with theimpl_opsat all inarm_vsmmu_init()andarm_smmu_get_viommu_size()Hmm, I was hoping for an implementation could support the default
IOMMU_VIOMMU_TYPE_ARM_SMMUV3while having its ownviommu_opsor so. But I think your suggestion is fine since there is no such a use case at this moment :)
Quoted 7 lines
Quoted 4 lines
2. Otherwise, we pass the type into the
impl_opsand they can check itOf course, that can be a patch on top of the series as there's no point respinning the whole just for this.
Thanks for that! I can draft a patch to send later this week once the requirements are confirmed.
Thank you!
Will
PATCH v9 24/29 iommu/arm-smmu-v3-iommufd: Add hw_info to impl_ops
In reply to Nicolin ChenThis will be used by Tegra241 CMDQV implementation to report a non-default HW info data.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 7 +++++++ drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 8 ++++++-- 2 files changed, 13 insertions(+), 2 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +7 -0
@@ -721,6 +721,13 @@ struct arm_smmu_impl_ops {int (*init_structures)(struct arm_smmu_device *smmu);struct arm_smmu_cmdq *(*get_secondary_cmdq)(struct arm_smmu_device *smmu,struct arm_smmu_cmdq_ent *ent); +/* +* An implementation should define its own type other than the default +* IOMMU_HW_INFO_TYPE_ARM_SMMUV3. And it must validate the input @type +* to return its own structure. +*/ +void *(*hw_info)(struct arm_smmu_device *smmu,u32 *length, +enum iommu_hw_info_type *type);const size_t vsmmu_size;const enum iommu_viommu_type vsmmu_type; int (*vsmmu_init)(struct arm_vsmmu *vsmmu,
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +6 -2
@@ -11,13 +11,17 @@ void *arm_smmu_hw_info(struct device *dev, u32 *length, enum iommu_hw_info_type *type) {struct arm_smmu_master *master =dev_iommu_priv_get (dev); +const struct arm_smmu_impl_ops *impl_ops = master->smmu->impl_ops;struct iommu_hw_info_arm_smmuv3 *info;u32 __iomem *base_idr;unsigned int i;if (*type !=IOMMU_HW_INFO_TYPE_DEFAULT && - *type !=IOMMU_HW_INFO_TYPE_ARM_SMMUV3 ) -return ERR_PTR (-EOPNOTSUPP ); + *type !=IOMMU_HW_INFO_TYPE_ARM_SMMUV3 ) { +if (!impl_ops || !impl_ops->hw_info) +return ERR_PTR (-EOPNOTSUPP ); +return impl_ops->hw_info (master->smmu, length, type); + } info =kzalloc (sizeof (*info),GFP_KERNEL );if (!info)
Signature · 1 line
2.43.0
PATCH v9 25/29 iommu/tegra241-cmdqv: Use request_threaded_irq
In reply to Nicolin ChenA vEVENT can be reported only from a threaded IRQ context. Change to using request_threaded_irq to support that.
--- drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +3 -2
@@ -824,8 +824,9 @@ __tegra241_cmdqv_probe(struct arm_smmu_device *smmu, struct resource *res, cmdqv->dev = smmu->impl_dev;if (cmdqv->irq >0 ) { - ret =request_irq (irq, tegra241_cmdqv_isr,0 ,"tegra241-cmdqv" , - cmdqv); + ret =request_threaded_irq (irq,NULL , tegra241_cmdqv_isr, +IRQF_ONESHOT ,"tegra241-cmdqv" , + cmdqv);if (ret) {dev_err (cmdqv->dev,"failed to request irq (%d): %d\n" , cmdqv->irq, ret);
Signature · 1 line
2.43.0
PATCH v9 26/29 iommu/tegra241-cmdqv: Simplify deinit flow in tegra241_cmdqv_remove_vintf()
In reply to Nicolin ChenThe current flow of tegra241_cmdqv_remove_vintf() is:
1. For each LVCMDQ, tegra241_vintf_remove_lvcmdq():
a. Disable the LVCMDQ HW
b. Release the LVCMDQ SW resource
2. For current VINTF, tegra241_vintf_hw_deinit():
c. Disable all LVCMDQ HWs
d. Disable VINTF HW
Obviously, the step 1.a and the step 2.c are redundant.
Since tegra241_vintf_hw_deinit() disables all of its LVCMDQ HWs, it could simplify the flow in tegra241_cmdqv_remove_vintf() by calling that first:
1. For current VINTF, tegra241_vintf_hw_deinit():
a. Disable all LVCMDQ HWs
b. Disable VINTF HW
2. Release all LVCMDQ SW resources
Drop tegra241_vintf_remove_lvcmdq(), and move tegra241_vintf_free_lvcmdq() as the new step 2.
--- drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-)
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +3 -10
@@ -628,24 +628,17 @@ static int tegra241_cmdqv_init_vintf(struct tegra241_cmdqv *cmdqv, u16 max_idx,/* Remove Helpers */ -static void tegra241_vintf_remove_lvcmdq (struct tegra241_vintf *vintf,u16 lidx) -{ -tegra241_vcmdq_hw_deinit (vintf->lvcmdqs[lidx]); -tegra241_vintf_free_lvcmdq (vintf, lidx); -} -static void tegra241_cmdqv_remove_vintf (struct tegra241_cmdqv *cmdqv,u16 idx) {struct tegra241_vintf *vintf = cmdqv->vintfs[idx];u16 lidx; +tegra241_vintf_hw_deinit (vintf); +/* Remove LVCMDQ resources */ for (lidx =0 ; lidx < vintf->cmdqv->num_lvcmdqs_per_vintf; lidx++)if (vintf->lvcmdqs[lidx]) -tegra241_vintf_remove_lvcmdq (vintf, lidx); - -/* Remove VINTF resources */ -tegra241_vintf_hw_deinit (vintf); +tegra241_vintf_free_lvcmdq (vintf, lidx);dev_dbg (cmdqv->dev,"VINTF%u: deallocated\n" , vintf->idx);tegra241_cmdqv_deinit_vintf (cmdqv, idx);
Signature · 1 line
2.43.0
PATCH v9 27/29 iommu/tegra241-cmdqv: Do not statically map LVCMDQs
In reply to Nicolin ChenTo simplify the mappings from global VCMDQs to VINTFs' LVCMDQs, the design chose to do static allocations and mappings in the global reset function.
However, with the user-owned VINTF support, it exposes a security concern:
if user space VM only wants one LVCMDQ for a VINTF, statically mapping two or more LVCMDQs creates a hidden VCMDQ that user space could DoS attack by writing random stuff to overwhelm the kernel with unhandleable IRQs.
Thus, to support the user-owned VINTF feature, a LVCMDQ mapping has to be done dynamically.
HW allows pre-assigning global VCMDQs in the CMDQ_ALLOC registers, without finalizing the mappings by keeping CMDQV_CMDQ_ALLOCATED=0. So, add a pair of map/unmap helper that simply sets/clears that bit.
For kernel-owned VINTF0, move LVCMDQ mappings to tegra241_vintf_hw_init(), and the unmappings to tegra241_vintf_hw_deinit().
For user-owned VINTFs that will be added, the mappings/unmappings will be on demand upon an LVCMDQ allocation from the user space.
However, the dynamic LVCMDQ mapping/unmapping can complicate the timing of calling tegra241_vcmdq_hw_init/deinit(), which write LVCMDQ address space, i.e. requiring LVCMDQ to be mapped. Highlight that with a note to the top of either of them.
--- .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 37 +++++++++++++++++-- 1 file changed, 33 insertions(+), 4 deletions(-)
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +33 -4
@@ -351,6 +351,7 @@ tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu,/* HW Reset Functions */ +/* This function is for LVCMDQ, so @vcmdq must not be unmapped yet */ static void tegra241_vcmdq_hw_deinit (struct tegra241_vcmdq *vcmdq) {char header[64 ], *h =lvcmdq_error_header (vcmdq, header,64 );
@@ -379,6 +380,7 @@ static void tegra241_vcmdq_hw_deinit(struct tegra241_vcmdq *vcmdq)dev_dbg (vcmdq->cmdqv->dev,"%sdeinited\n" , h); } +/* This function is for LVCMDQ, so @vcmdq must be mapped prior */ static int tegra241_vcmdq_hw_init (struct tegra241_vcmdq *vcmdq) {char header[64 ], *h =lvcmdq_error_header (vcmdq, header,64 );
@@ -404,16 +406,42 @@ static int tegra241_vcmdq_hw_init(struct tegra241_vcmdq *vcmdq)return 0 ; } +/* Unmap a global VCMDQ from the pre-assigned LVCMDQ */ +static void tegra241_vcmdq_unmap_lvcmdq (struct tegra241_vcmdq *vcmdq) +{ +u32 regval =readl (REG_CMDQV (vcmdq->cmdqv,CMDQ_ALLOC (vcmdq->idx))); +char header[64 ], *h =lvcmdq_error_header (vcmdq, header,64 ); + +writel (regval & ~CMDQV_CMDQ_ALLOCATED , +REG_CMDQV (vcmdq->cmdqv,CMDQ_ALLOC (vcmdq->idx))); +dev_dbg (vcmdq->cmdqv->dev,"%sunmapped\n" , h); +} +static void tegra241_vintf_hw_deinit (struct tegra241_vintf *vintf) { -u16 lidx; +u16 lidx = vintf->cmdqv->num_lvcmdqs_per_vintf; -for (lidx =0 ; lidx < vintf->cmdqv->num_lvcmdqs_per_vintf; lidx++) -if (vintf->lvcmdqs && vintf->lvcmdqs[lidx]) +/* HW requires to unmap LVCMDQs in descending order */ +while (lidx--) { +if (vintf->lvcmdqs && vintf->lvcmdqs[lidx]) {tegra241_vcmdq_hw_deinit (vintf->lvcmdqs[lidx]); +tegra241_vcmdq_unmap_lvcmdq (vintf->lvcmdqs[lidx]); + } + }vintf_write_config (vintf,0 ); } +/* Map a global VCMDQ to the pre-assigned LVCMDQ */ +static void tegra241_vcmdq_map_lvcmdq (struct tegra241_vcmdq *vcmdq) +{ +u32 regval =readl (REG_CMDQV (vcmdq->cmdqv,CMDQ_ALLOC (vcmdq->idx))); +char header[64 ], *h =lvcmdq_error_header (vcmdq, header,64 ); + +writel (regval |CMDQV_CMDQ_ALLOCATED , +REG_CMDQV (vcmdq->cmdqv,CMDQ_ALLOC (vcmdq->idx))); +dev_dbg (vcmdq->cmdqv->dev,"%smapped\n" , h); +} +static int tegra241_vintf_hw_init (struct tegra241_vintf *vintf,bool hyp_own) {u32 regval;
@@ -441,8 +469,10 @@ static int tegra241_vintf_hw_init(struct tegra241_vintf *vintf, bool hyp_own)*/ vintf->hyp_own = !!(VINTF_HYP_OWN &readl (REG_VINTF (vintf,CONFIG ))); +/* HW requires to map LVCMDQs in ascending order */ for (lidx =0 ; lidx < vintf->cmdqv->num_lvcmdqs_per_vintf; lidx++) {if (vintf->lvcmdqs && vintf->lvcmdqs[lidx]) { +tegra241_vcmdq_map_lvcmdq (vintf->lvcmdqs[lidx]); ret =tegra241_vcmdq_hw_init (vintf->lvcmdqs[lidx]);if (ret) {tegra241_vintf_hw_deinit (vintf);
@@ -476,7 +506,6 @@ static int tegra241_cmdqv_hw_reset(struct arm_smmu_device *smmu)for (lidx =0 ; lidx < cmdqv->num_lvcmdqs_per_vintf; lidx++) { regval =FIELD_PREP (CMDQV_CMDQ_ALLOC_VINTF , idx); regval |=FIELD_PREP (CMDQV_CMDQ_ALLOC_LVCMDQ , lidx); - regval |=CMDQV_CMDQ_ALLOCATED ;writel_relaxed (regval,REG_CMDQV (cmdqv,CMDQ_ALLOC (qidx++))); }
Signature · 1 line
2.43.0
PATCH v9 28/29 iommu/tegra241-cmdqv: Add user-space use support
In reply to Nicolin ChenThe CMDQV HW supports a user-space use for virtualization cases. It allows the VM to issue guest-level TLBI or ATC_INV commands directly to the queue and executes them without a VMEXIT, as HW will replace the VMID field in a TLBI command and the SID field in an ATC_INV command with the preset VMID and SID.
This is built upon the vIOMMU infrastructure by allowing VMM to allocate a VINTF (as a vIOMMU object) and assign VCMDQs (HW QUEUE objs) to the VINTF.
So firstly, replace the standard vSMMU model with the VINTF implementation but reuse the standard cache_invalidate op (for unsupported commands) and the standard alloc_domain_nested op (for standard nested STE).
Each VINTF has two 64KB MMIO pages (128B per logical VCMDQ):
- Page0 (directly accessed by guest) has all the control and status bits.
- Page1 (trapped by VMM) has guest-owned queue memory location/size info.
VMM should trap the emulated VINTF0's page1 of the guest VM for the guest- level VCMDQ location/size info and forward that to the kernel to translate to a physical memory location to program the VCMDQ HW during an allocation call. Then, it should mmap the assigned VINTF's page0 to the VINTF0 page0 of the guest VM. This allows the guest OS to read and write the guest-own VINTF's page0 for direct control of the VCMDQ HW.
For ATC invalidation commands that hold an SID, it requires all devices to register their virtual SIDs to the SID_MATCH registers and their physical SIDs to the pairing SID_REPLACE registers, so that HW can use those as a lookup table to replace those virtual SIDs with the correct physical SIDs. Thus, implement the driver-allocated vDEVICE op with a tegra241_vintf_sid structure to allocate SID_REPLACE and to program the SIDs accordingly.
This enables the HW accelerated feature for NVIDIA Grace CPU. Compared to the standard SMMUv3 operating in the nested translation mode trapping CMDQ for TLBI and ATC_INV commands, this gives a huge performance improvement:
70% to 90% reductions of invalidation time were measured by various DMA unmap tests running in a guest OS.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 7 + include/uapi/linux/iommufd.h | 58 +++ .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 6 +- .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 400 +++++++++++++++++- 4 files changed, 465 insertions(+), 6 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +7 -0
@@ -1057,10 +1057,17 @@ int arm_smmu_attach_prepare_vmaster(struct arm_smmu_attach_state *state,void arm_smmu_attach_commit_vmaster (struct arm_smmu_attach_state *state);void arm_smmu_master_clear_vmaster (struct arm_smmu_master *master);int arm_vmaster_report_event (struct arm_smmu_vmaster *vmaster,u64 *evt); +struct iommu_domain * +arm_vsmmu_alloc_domain_nested (struct iommufd_viommu *viommu,u32 flags, +const struct iommu_user_data *user_data); +int arm_vsmmu_cache_invalidate (struct iommufd_viommu *viommu, +struct iommu_user_data_array *array);#else #define arm_smmu_get_viommu_size NULL#define arm_smmu_hw_info NULL#define arm_vsmmu_init NULL +#define arm_vsmmu_alloc_domain_nested NULL +#define arm_vsmmu_cache_invalidate NULL static inline int arm_smmu_attach_prepare_vmaster(struct arm_smmu_attach_state *state,
include/uapi/linux/iommufd.h +58 -0
95 lines not on this page · raw message
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +3 -3
@@ -225,7 +225,7 @@ static int arm_smmu_validate_vste(struct iommu_hwpt_arm_smmuv3 *arg,return 0 ; } -static struct iommu_domain * +struct iommu_domain *arm_vsmmu_alloc_domain_nested (struct iommufd_viommu *viommu,u32 flags,const struct iommu_user_data *user_data) {
@@ -336,8 +336,8 @@ static int arm_vsmmu_convert_user_cmd(struct arm_vsmmu *vsmmu,return 0 ; } -static int arm_vsmmu_cache_invalidate (struct iommufd_viommu *viommu, -struct iommu_user_data_array *array) +int arm_vsmmu_cache_invalidate (struct iommufd_viommu *viommu, +struct iommu_user_data_array *array) { struct arm_vsmmu *vsmmu = container_of(viommu, struct arm_vsmmu, core);struct arm_smmu_device *smmu = vsmmu->smmu;
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +397 -3
557 lines not on this page · raw message
Signature · 1 line
2.43.0
PATCH v9 29/29 iommu/tegra241-cmdqv: Add IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV support
In reply to Nicolin ChenAdd a new vEVENTQ type for VINTFs that are assigned to the user space. Simply report the two 64-bit LVCMDQ_ERR_MAPs register values.
--- include/uapi/linux/iommufd.h | 15 +++++++++++++ .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 22 +++++++++++++++++++ 2 files changed, 37 insertions(+)
include/uapi/linux/iommufd.h +15 -0
@@ -1145,10 +1145,12 @@ struct iommufd_vevent_header {* enum iommu_veventq_type - Virtual Event Queue Type * @IOMMU_VEVENTQ_TYPE_DEFAULT: Reserved for future use * @IOMMU_VEVENTQ_TYPE_ARM_SMMUV3: ARM SMMUv3 Virtual Event Queue +* @IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV: NVIDIA Tegra241 CMDQV Extension IRQ */ enum iommu_veventq_type {IOMMU_VEVENTQ_TYPE_DEFAULT =0 ,IOMMU_VEVENTQ_TYPE_ARM_SMMUV3 =1 , +IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV =2 , };/**
@@ -1172,6 +1174,19 @@ struct iommu_vevent_arm_smmuv3 {__aligned_le64 evt[4 ]; }; +/** +* struct iommu_vevent_tegra241_cmdqv - Tegra241 CMDQV IRQ +* (IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV) +* @lvcmdq_err_map: 128-bit logical vcmdq error map, little-endian. +* (Refer to register LVCMDQ_ERR_MAPs per VINTF ) +* +* The 128-bit register value from HW exclusively reflect the error bits for a +* Virtual Interface represented by a vIOMMU object. Read and report directly. +*/ +struct iommu_vevent_tegra241_cmdqv { +__aligned_le64 lvcmdq_err_map[2 ]; +}; +/** * struct iommu_veventq_alloc - ioctl(IOMMU_VEVENTQ_ALLOC) * @size: sizeof(struct iommu_veventq_alloc)
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +22 -0
@@ -295,6 +295,20 @@ static inline int vcmdq_write_config(struct tegra241_vcmdq *vcmdq, u32 regval)/* ISR Functions */ +static void tegra241_vintf_user_handle_error (struct tegra241_vintf *vintf) +{ +struct iommufd_viommu *viommu = &vintf->vsmmu.core; +struct iommu_vevent_tegra241_cmdqv vevent_data; +int i; + +for (i =0 ; i <LVCMDQ_ERR_MAP_NUM_64 ; i++) + vevent_data.lvcmdq_err_map[i] = +readq_relaxed (REG_VINTF (vintf,LVCMDQ_ERR_MAP_64 (i))); + +iommufd_viommu_report_event (viommu,IOMMU_VEVENTQ_TYPE_TEGRA241_CMDQV , + &vevent_data,sizeof (vevent_data)); +} +static void tegra241_vintf0_handle_error (struct tegra241_vintf *vintf) {int i;
@@ -340,6 +354,14 @@ static irqreturn_t tegra241_cmdqv_isr(int irq, void *devid) vintf_map &= ~BIT_ULL (0 ); } +/* Handle other user VINTFs and their LVCMDQs */ +while (vintf_map) { +unsigned long idx =__ffs64 (vintf_map); + +tegra241_vintf_user_handle_error (cmdqv->vintfs[idx]); + vintf_map &= ~BIT_ULL (idx); + } +return IRQ_HANDLED ; }
Signature · 1 line
2.43.0