iommufd: Add vIOMMU infrastructure (Part-4 vQUEUE)
Subsystems (7)
v3 cover iommufd: Add vIOMMU infrastructure (Part-4 vQUEUE)
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.
A vQUEUE 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 is an IOMMU HW feature to directly access the virtual queue in the guest address space, to avoid VM Exits to improve the performance.
As an initial use case, it adds support for guest-owned HW virtual queues that VMM can allocate per request from a guest OS writing the VM register. Introduce IOMMUFD_OBJ_VQUEUE and its allocator IOMMUFD_CMD_VQUEUE_ALLOC, allowing VMM to forward the IOMMU-specific queue info, such as queue base address, size, and etc.
Meanwhile, a guest-owned virtual queue needs the kernel (a virtual queue driver) to control the queue by reading/writing its consumer and producer indexes, which means the virtual queue HW allows the guest kernel to get a direct R/W access to those registers. Introduce an mmap infrastructure to the iommufd core so as to support pass through a piece of MMIO region from the host physical address space to the guest physical address space. The VMA info (vm_pgoff/size) used by an mmap must be pre-allocated during the IOMMUFD_CMD_VQUEUE_ALLOC and returned to the user space as an output driver-data carried via the IOMMUFD_CMD_VQUEUE_ALLOC. So, this requires a driver-specific user data support in the vIOMMU allocation flow.
As a real-world use case, this series implements a vQUEUE support to 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_vqueue-v3
Paring QEMU branch for testing:
https://github.com/nicolinc/qemu/commits/wip/for_iommufd_vqueue-v3
Changelog
v3
* 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 (23):
iommufd/viommu: Add driver-allocated vDEVICE support
iommu: Pass in a driver-level user data structure to viommu_alloc op iommufd/viommu: Allow driver-specific user data for a vIOMMU object
iommu: Add iommu_copy_struct_to_user helper iommufd/driver: Let iommufd_viommu_alloc helper save ictx to
viommu->ictx
iommufd/driver: Add iommufd_struct_destroy to revert
iommufd_viommu_alloc
iommufd/selftest: Support user_data in mock_viommu_alloc
iommufd/selftest: Add covearge for viommu data
iommufd: Abstract iopt_pin_pages and iopt_unpin_pages helpers
iommufd/viommu: Introduce IOMMUFD_OBJ_VQUEUE and its related struct
iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
iommufd/driver: Add iommufd_vqueue_depend/undepend() helpers
iommufd/selftest: Add coverage for IOMMUFD_CMD_VQUEUE_ALLOC
iommufd: Add mmap interface
iommufd/selftest: Add coverage for the new mmap interface
Documentation: userspace-api: iommufd: Update vQUEUE
iommu/arm-smmu-v3-iommufd: Add vsmmu_alloc impl op
iommu/arm-smmu-v3-iommufd: Support implementation-defined hw_info
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 | 8 + drivers/iommu/iommufd/iommufd_private.h | 28 +- drivers/iommu/iommufd/iommufd_test.h | 20 + include/linux/iommu.h | 43 +- include/linux/iommufd.h | 184 ++++++- include/uapi/linux/iommufd.h | 117 ++++- tools/testing/selftests/iommu/iommufd_utils.h | 52 +- .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 42 +- .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 481 +++++++++++++++++- drivers/iommu/iommufd/device.c | 117 +---- drivers/iommu/iommufd/driver.c | 88 ++++ drivers/iommu/iommufd/io_pagetable.c | 95 ++++ drivers/iommu/iommufd/main.c | 84 ++- drivers/iommu/iommufd/selftest.c | 126 ++++- drivers/iommu/iommufd/viommu.c | 116 ++++- tools/testing/selftests/iommu/iommufd.c | 96 +++- .../selftests/iommu/iommufd_fail_nth.c | 11 +- Documentation/userspace-api/iommufd.rst | 15 + 19 files changed, 1555 insertions(+), 193 deletions(-)
Signature · 1 line
2.43.0
PATCH v3 1/23 iommufd/viommu: Add driver-allocated vDEVICE support
To allow IOMMU drivers to allocate own vDEVICE structures, move the struct iommufd_vdevice to the public header and provide a pair of viommu ops.
The iommufd_vdevice_alloc_ioctl will prioritize the callback function from the viommu ops, i.e. a driver-allocated vDEVICE.
--- drivers/iommu/iommufd/iommufd_private.h | 8 ------ include/linux/iommufd.h | 38 +++++++++++++++++++++++++ drivers/iommu/iommufd/viommu.c | 9 +++++- 3 files changed, 46 insertions(+), 9 deletions(-)
drivers/iommu/iommufd/iommufd_private.h +0 -8
@@ -607,14 +607,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_ctx *ictx; -struct iommufd_viommu *viommu; -struct device *dev; -u64 id;/* per-vIOMMU virtual ID */ -}; -#ifdef CONFIG_IOMMUFD_TEST int iommufd_test (struct iommufd_ucmd *ucmd);void iommufd_selftest_destroy (struct iommufd_object *obj);
include/linux/iommufd.h +38 -0
@@ -104,6 +104,14 @@ struct iommufd_viommu {unsigned int type; }; +struct iommufd_vdevice { +struct iommufd_object obj; +struct iommufd_ctx *ictx; +struct iommufd_viommu *viommu; +struct device *dev; +u64 id;/* per-vIOMMU virtual ID */ +}; +/** * struct iommufd_viommu_ops - vIOMMU specific operations * @destroy: Clean up all driver-specific parts of an iommufd_viommu. The memory
@@ -120,6 +128,15 @@ 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_alloc: Allocate a vDEVICE object and init its driver-level structure +* or HW procedure. Note that the core-level structure is filled +* by the iommufd core after calling this op. @virt_id carries a +* per-vIOMMU virtual ID (refer to struct iommu_vdevice_alloc in +* include/uapi/linux/iommufd.h) for the driver to initialize HW +* for an attached physical device. +* @vdevice_destroy: Clean up all driver-specific parts of an iommufd_vdevice. +* The memory of the vDEVICE will be free-ed by iommufd core +* after calling this op */ struct iommufd_viommu_ops {void (*destroy)(struct iommufd_viommu *viommu);
@@ -128,6 +145,10 @@ struct iommufd_viommu_ops { const struct iommu_user_data *user_data);int (*cache_invalidate)(struct iommufd_viommu *viommu,struct iommu_user_data_array *array); +struct iommufd_vdevice *(*vdevice_alloc)(struct iommufd_viommu *viommu, +struct device *dev, +u64 virt_id); +void (*vdevice_destroy)(struct iommufd_vdevice *vdev); };#if IS_ENABLED (CONFIG_IOMMUFD )
@@ -245,4 +266,21 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, ret->member.ops = viommu_ops; \ ret; \ }) + +#define iommufd_vdevice_alloc (viommu, drv_struct, member) \ + ({ \ + drv_struct *ret; \ + \ + static_assert(__same_type(struct iommufd_viommu, *viommu)); \ + static_assert(__same_type(struct iommufd_vdevice, \ + ((drv_struct *)NULL)->member)); \ + static_assert(offsetof(drv_struct, member.obj) == 0); \ + ret = (drv_struct *)_iommufd_object_alloc( \ + viommu->ictx, sizeof(drv_struct), IOMMUFD_OBJ_VDEVICE);\ + if (!IS_ERR(ret)) { \ + ret->member.viommu = viommu; \ + ret->member.ictx = viommu->ictx; \ + } \ + ret; \ + })#endif
drivers/iommu/iommufd/viommu.c +8 -1
@@ -90,6 +90,9 @@ void iommufd_vdevice_destroy(struct iommufd_object *obj) container_of(obj, struct iommufd_vdevice, obj);struct iommufd_viommu *viommu = vdev->viommu; +if (viommu->ops && viommu->ops->vdevice_destroy) + viommu->ops->vdevice_destroy (vdev); +/* xa_cmpxchg is okay to fail if alloc failed xa_cmpxchg previously */ xa_cmpxchg (&viommu->vdevs, vdev->id, vdev,NULL ,GFP_KERNEL );refcount_dec (&viommu->obj.users);
@@ -124,7 +127,11 @@ int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_idev; } - vdev =iommufd_object_alloc (ucmd->ictx, vdev,IOMMUFD_OBJ_VDEVICE ); +if (viommu->ops && viommu->ops->vdevice_alloc) + vdev = viommu->ops->vdevice_alloc (viommu, idev->dev, virt_id); +else + vdev =iommufd_object_alloc (ucmd->ictx, vdev, +IOMMUFD_OBJ_VDEVICE );if (IS_ERR (vdev)) { rc =PTR_ERR (vdev); goto out_put_idev;
Signature · 1 line
2.43.0
PATCH v3 2/23 iommu: Pass in a driver-level user data structure to viommu_alloc 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 (vm_pgoff and size), iommufd should add a driver specific data structure to the IOMMUFD_CMD_VIOMMU_ALLOC ioctl, for driver to output the info (during the allocation) back to user space.
Similar to the existing ioctls and their IOMMU handlers, add a user_data to viommu_alloc 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 | 8 ++++---- drivers/iommu/iommufd/viommu.c | 2 +- 5 files changed, 11 insertions(+), 8 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +2 -1
@@ -1062,7 +1062,8 @@ void *arm_smmu_hw_info(struct device *dev, u32 *length, u32 *type);struct iommufd_viommu *arm_vsmmu_alloc (struct device *dev,struct iommu_domain *parent,struct iommufd_ctx *ictx, -unsigned int viommu_type); +unsigned int viommu_type, +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
@@ -662,7 +662,8 @@ struct iommu_ops {struct iommufd_viommu *(*viommu_alloc)(struct device *dev,struct iommu_domain *parent_domain, -struct iommufd_ctx *ictx,unsigned int viommu_type); +struct iommufd_ctx *ictx,unsigned int viommu_type, +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
@@ -385,7 +385,8 @@ static const struct iommufd_viommu_ops arm_vsmmu_ops = {struct iommufd_viommu *arm_vsmmu_alloc (struct device *dev,struct iommu_domain *parent,struct iommufd_ctx *ictx, -unsigned int viommu_type) +unsigned int viommu_type, +const struct iommu_user_data *user_data) {struct arm_smmu_device *smmu = iommu_get_iommu_dev(dev, struct arm_smmu_device, iommu);
drivers/iommu/iommufd/selftest.c +4 -4
@@ -733,10 +733,10 @@ static struct iommufd_viommu_ops mock_viommu_ops = { .cache_invalidate = mock_viommu_cache_invalidate, }; -static struct iommufd_viommu *mock_viommu_alloc (struct device *dev, -struct iommu_domain *domain, -struct iommufd_ctx *ictx, -unsigned int viommu_type) +static struct iommufd_viommu * +mock_viommu_alloc (struct device *dev,struct iommu_domain *domain, +struct iommufd_ctx *ictx,unsigned int viommu_type, +const struct iommu_user_data *user_data) {struct mock_iommu_device *mock_iommu = iommu_get_iommu_dev(dev, struct mock_iommu_device, iommu_dev);
drivers/iommu/iommufd/viommu.c +1 -1
@@ -48,7 +48,7 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) } viommu = ops->viommu_alloc (idev->dev, hwpt_paging->common.domain, - ucmd->ictx, cmd->type); + ucmd->ictx, cmd->type,NULL );if (IS_ERR (viommu)) { rc =PTR_ERR (viommu); goto out_put_hwpt;
Signature · 1 line
2.43.0
Re: [PATCH v3 02/23] iommu: Pass in a driver-level user data structure to viommu_alloc op
Quoted 8 lines · Nicolin Chen
The 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 (
vm_pgoffand size), iommufd should add a driver specific data structure to theIOMMUFD_CMD_VIOMMU_ALLOCioctl, for driver to output the info (during the allocation) back to user space.
Thanks! AMD vIOMMU has similar requirement. 3rd 4K of IOMMU MMIO space is accelerated. We need to map VF MMIO BAR to guest. So this change is useful for AMD driver as well.
-Vasant
Quoted 90 lines
Similar to the existing ioctls and their IOMMU handlers, add a
user_datatoviommu_allocop to bridge between iommufd and drivers.Reviewed-by Jason Gunthorpe <[email protected]>Reviewed-by Lu Baolu <[email protected]>Reviewed-by Pranjal Shrivastava <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- 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 | 8 ++++---- drivers/iommu/iommufd/viommu.c | 2 +- 5 files changed, 11 insertions(+), 8 deletions(-)drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +2
-1@@ -1062,7 +1062,8 @@ void *arm_smmu_hw_info(struct device *dev, u32 *length, u32 *type);struct iommufd_viommu *arm_vsmmu_alloc (struct device *dev,struct iommu_domain *parent,struct iommufd_ctx *ictx, -unsigned int viommu_type); +unsigned int viommu_type, +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@@ -662,7 +662,8 @@ struct iommu_ops {struct iommufd_viommu *(*viommu_alloc)(struct device *dev,struct iommu_domain *parent_domain, -struct iommufd_ctx *ictx,unsigned int viommu_type); +struct iommufd_ctx *ictx,unsigned int viommu_type, +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@@ -385,7 +385,8 @@ static const struct iommufd_viommu_ops arm_vsmmu_ops = {struct iommufd_viommu *arm_vsmmu_alloc (struct device *dev,struct iommu_domain *parent,struct iommufd_ctx *ictx, -unsigned int viommu_type) +unsigned int viommu_type, +const struct iommu_user_data *user_data) {struct arm_smmu_device *smmu = iommu_get_iommu_dev(dev, struct arm_smmu_device, iommu);drivers/iommu/iommufd/selftest.c +4
-4@@ -733,10 +733,10 @@ static struct iommufd_viommu_ops mock_viommu_ops = { .cache_invalidate = mock_viommu_cache_invalidate, }; -static struct iommufd_viommu *mock_viommu_alloc (struct device *dev, -struct iommu_domain *domain, -struct iommufd_ctx *ictx, -unsigned int viommu_type) +static struct iommufd_viommu * +mock_viommu_alloc (struct device *dev,struct iommu_domain *domain, +struct iommufd_ctx *ictx,unsigned int viommu_type, +const struct iommu_user_data *user_data) {struct mock_iommu_device *mock_iommu = iommu_get_iommu_dev(dev, struct mock_iommu_device, iommu_dev);drivers/iommu/iommufd/viommu.c +1
-1@@ -48,7 +48,7 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) } viommu = ops->viommu_alloc (idev->dev, hwpt_paging->common.domain, - ucmd->ictx, cmd->type); + ucmd->ictx, cmd->type,NULL );if (IS_ERR (viommu)) { rc =PTR_ERR (viommu); goto out_put_hwpt;
PATCH v3 3/23 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_alloc 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 an array of 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;
@@ -48,7 +53,8 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) } viommu = ops->viommu_alloc (idev->dev, hwpt_paging->common.domain, - ucmd->ictx, cmd->type,NULL ); + ucmd->ictx, cmd->type, + user_data.len ? &user_data :NULL );if (IS_ERR (viommu)) { rc =PTR_ERR (viommu); goto out_put_hwpt;
Signature · 1 line
2.43.0
Re: [PATCH v3 03/23] iommufd/viommu: Allow driver-specific user data for a vIOMMU object
Quoted 3 lines · Nicolin Chen
The new type of vIOMMU for tegra241-cmdqv driver needs a driver-specific user data. So, add data_len/uptr to the
iommu_viommu_allocuAPI and pass it in via theviommu_allociommu op.
-Vasant
Quoted 61 lines
Reviewed-by Jason Gunthorpe <[email protected]>Reviewed-by Lu Baolu <[email protected]>Acked-by Alok Tiwari <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- 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 an array of 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;@@ -48,7 +53,8 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd) } viommu = ops->viommu_alloc (idev->dev, hwpt_paging->common.domain, - ucmd->ictx, cmd->type,NULL ); + ucmd->ictx, cmd->type, + user_data.len ? &user_data :NULL );if (IS_ERR (viommu)) { rc =PTR_ERR (viommu); goto out_put_hwpt;
PATCH v3 4/23 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
@@ -562,6 +562,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 v3 5/23 iommufd/driver: Let iommufd_viommu_alloc helper save ictx to viommu->ictx
In reply to Nicolin ChenWhen an IOMMU driver calls iommufd_viommu_alloc(), it must pass in an ictx pointer as the underlying _iommufd_object_alloc() helper function requires that to allocate a new object. However, neither the iommufd_viommu_alloc() nor its underlying _iommufd_object_alloc() saves the ictx in the allocated viommu object, although viommu could hold an ictx pointer.
When the IOMMU driver wants to use another iommufd function passing in the allocated viommu, it could have avoided passing in the ictx pointer again, if viommu->ictx is valid.
Save ictx to viommu->ictx in the iommufd_viommu_alloc(), in order to ease a new vIOMMU-based helper that would then get the ictx from viommu->ictx.
--- include/linux/iommufd.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
include/linux/iommufd.h +3 -1
@@ -262,8 +262,10 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, static_assert(offsetof(drv_struct, member.obj) == 0); \ ret = (drv_struct *)_iommufd_object_alloc ( \ ictx,sizeof (drv_struct),IOMMUFD_OBJ_VIOMMU ); \ -if (!IS_ERR (ret)) \ +if (!IS_ERR (ret)) { \ ret->member.ops = viommu_ops; \ + ret->member.ictx = ictx; \ + } \ ret; \ })
Signature · 1 line
2.43.0
PATCH v3 6/23 iommufd/driver: Add iommufd_struct_destroy to revert iommufd_viommu_alloc
In reply to Nicolin ChenAn IOMMU driver that allocated a vIOMMU may want to revert the allocation, if it encounters an internal error after the allocation. So, there needs a destroy helper for drivers to use. For instance:
static my_viommu_alloc()
{
...
my_viommu = iommufd_viommu_alloc(viomm, struct my_viommu, core);
...
ret = init_my_viommu();
if (ret) {
/* Need to destroy the my_viommu->core */
return ERR_PTR(ret);
}
return &my_viommu->core;
}Move iommufd_object_abort() to the driver.c file and the public header, to introduce common iommufd_struct_destroy() helper that will abort all kinds of driver structures, not confined to iommufd_viommu but also the new ones being added in the future.
--- drivers/iommu/iommufd/iommufd_private.h | 1 - include/linux/iommufd.h | 16 ++++++++++++++++ drivers/iommu/iommufd/driver.c | 14 ++++++++++++++ drivers/iommu/iommufd/main.c | 13 ------------- 4 files changed, 30 insertions(+), 14 deletions(-)
drivers/iommu/iommufd/iommufd_private.h +0 -1
@@ -180,7 +180,6 @@ static inline void iommufd_put_object(struct iommufd_ctx *ictx,wake_up_interruptible_all (&ictx->destroy_wait); } -void iommufd_object_abort (struct iommufd_ctx *ictx,struct iommufd_object *obj);void iommufd_object_abort_and_destroy (struct iommufd_ctx *ictx,struct iommufd_object *obj); void iommufd_object_finalize(struct iommufd_ctx *ictx,
include/linux/iommufd.h +16 -0
@@ -213,6 +213,7 @@ static inline int iommufd_vfio_compat_set_no_iommu(struct iommufd_ctx *ictx)struct iommufd_object *_iommufd_object_alloc (struct iommufd_ctx *ictx,size_t size,enum iommufd_object_type type); +void iommufd_object_abort (struct iommufd_ctx *ictx,struct iommufd_object *obj);struct device *iommufd_viommu_find_dev (struct iommufd_viommu *viommu,unsigned long vdev_id); int iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
@@ -228,6 +229,11 @@ _iommufd_object_alloc(struct iommufd_ctx *ictx, size_t size,return ERR_PTR (-EOPNOTSUPP ); } +static inline void iommufd_object_abort (struct iommufd_ctx *ictx, +struct iommufd_object *obj) +{ +} +static inline struct device *iommufd_viommu_find_dev (struct iommufd_viommu *viommu,unsigned long vdev_id) {
@@ -285,4 +291,14 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, } \ ret; \ }) + +/* Helper for IOMMU driver to destroy structures created by allocators above */ +#define iommufd_struct_destroy (drv_struct, member) \ + ({ \ + static_assert(__same_type(struct iommufd_object, \ + drv_struct->member.obj)); \ + static_assert(offsetof(typeof(*drv_struct), member.obj) == 0); \ + iommufd_object_abort(drv_struct->member.ictx, \ + &drv_struct->member.obj); \ + })#endif
drivers/iommu/iommufd/driver.c +14 -0
@@ -36,6 +36,20 @@ struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx, }EXPORT_SYMBOL_NS_GPL (_iommufd_object_alloc,"IOMMUFD" ); +/* Undo _iommufd_object_alloc() if iommufd_object_finalize() was not called */ +void iommufd_object_abort (struct iommufd_ctx *ictx,struct iommufd_object *obj) +{ +XA_STATE (xas, &ictx->objects, obj->id); +void *old; + +xa_lock (&ictx->objects); + old =xas_store (&xas,NULL ); +xa_unlock (&ictx->objects); +WARN_ON (old !=XA_ZERO_ENTRY ); +kfree (obj); +} +EXPORT_SYMBOL_NS_GPL (iommufd_object_abort,"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)
drivers/iommu/iommufd/main.c +0 -13
@@ -51,19 +51,6 @@ void iommufd_object_finalize(struct iommufd_ctx *ictx,WARN_ON (old !=XA_ZERO_ENTRY ); } -/* Undo _iommufd_object_alloc() if iommufd_object_finalize() was not called */ -void iommufd_object_abort (struct iommufd_ctx *ictx,struct iommufd_object *obj) -{ -XA_STATE (xas, &ictx->objects, obj->id); -void *old; - -xa_lock (&ictx->objects); - old =xas_store (&xas,NULL ); -xa_unlock (&ictx->objects); -WARN_ON (old !=XA_ZERO_ENTRY ); -kfree (obj); -} -/* * Abort an object that has been fully initialized and needs destroy, but has * not been finalized.
Signature · 1 line
2.43.0
PATCH v3 7/23 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 | 19 +++++++++++++++++++ 2 files changed, 32 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 +19 -0
@@ -740,16 +740,35 @@ mock_viommu_alloc(struct device *dev, struct iommu_domain *domain, {struct mock_iommu_device *mock_iommu = iommu_get_iommu_dev(dev, struct mock_iommu_device, iommu_dev); +struct iommu_viommu_selftest data;struct mock_viommu *mock_viommu; +int rc;if (viommu_type !=IOMMU_VIOMMU_TYPE_SELFTEST )return ERR_PTR (-EOPNOTSUPP ); +if (user_data) { + rc =iommu_copy_struct_from_user ( + &data, user_data,IOMMU_VIOMMU_TYPE_SELFTEST , out_data); +if (rc) +return ERR_PTR (rc); + } + mock_viommu = iommufd_viommu_alloc(ictx, struct mock_viommu, core, &mock_viommu_ops);if (IS_ERR (mock_viommu))return ERR_CAST (mock_viommu); +if (user_data) { + data.out_data = data.in_data; + rc =iommu_copy_struct_to_user ( + user_data, &data,IOMMU_VIOMMU_TYPE_SELFTEST , out_data); +if (rc) { +iommufd_struct_destroy (mock_viommu, core); +return ERR_PTR (rc); + } + } +refcount_inc (&mock_iommu->users);return &mock_viommu->core; }
Signature · 1 line
2.43.0
PATCH v3 8/23 iommufd/selftest: Add covearge 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 | 30 ++++++++++++++----- .../selftests/iommu/iommufd_fail_nth.c | 5 ++-- 3 files changed, 39 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 +23 -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,20 @@ 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) { +test_cmd_viommu_alloc (self->device_id, self->hwpt_id, +IOMMU_VIOMMU_TYPE_SELFTEST , &data, +sizeof (data), &self->viommu_id); +assert (data.out_data == data.in_data); + } +} + TEST_F(iommufd_viommu, vdevice_alloc) {uint32_t viommu_id = self->viommu_id;
@@ -3105,8 +3122,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 v3 9/23 iommufd: Abstract iopt_pin_pages and iopt_unpin_pages helpers
In reply to Nicolin ChenThe new vQUEUE object will be added for HW to access the guest memory, for HW-accelerated virtualization feature. It needs to ensure the guest memory pages are pinned when HW accesses them and they are contiguous in physical address space.
This is very like the existing iommufd_access_pin_pages() that outputs the pinned page list for the caller to test its contiguity.
Move those code from iommufd_access_pin/unpin_pages() and related function for a pair of iopt helpers that can be shared with the vQUEUE allocator.
Since the vQUEUE allocator will be a user-space triggered ioctl function, WARN_ON would not be a good fit in the new iopt_unpin_pages(), thus change them to use WARN_ON_ONCE instead.
Rename check_area_prot() to align with the existing iopt_area helpers, and inline it to the header since iommufd_access_rw() still uses it.
--- drivers/iommu/iommufd/io_pagetable.h | 8 ++ drivers/iommu/iommufd/iommufd_private.h | 6 ++ drivers/iommu/iommufd/device.c | 117 ++---------------------- drivers/iommu/iommufd/io_pagetable.c | 95 +++++++++++++++++++ 4 files changed, 117 insertions(+), 109 deletions(-)
drivers/iommu/iommufd/io_pagetable.h +8 -0
@@ -114,6 +114,14 @@ static inline unsigned long iopt_area_iova_to_index(struct iopt_area *area,return iopt_area_start_byte (area, iova) /PAGE_SIZE ; } +static inline bool iopt_area_check_prot (struct iopt_area *area, +unsigned int flags) +{ +if (flags &IOMMUFD_ACCESS_RW_WRITE ) +return area->iommu_prot &IOMMU_WRITE ; +return area->iommu_prot &IOMMU_READ ; +} +#define __make_iopt_iter (name) \ static inline struct iopt_##name *iopt_##name##_iter_first( \ struct io_pagetable *iopt, unsigned long start, \
drivers/iommu/iommufd/iommufd_private.h +6 -0
@@ -130,6 +130,12 @@ int iopt_cut_iova(struct io_pagetable *iopt, unsigned long *iovas,void iopt_enable_large_pages (struct io_pagetable *iopt);int iopt_disable_large_pages (struct io_pagetable *iopt); +int iopt_pin_pages (struct io_pagetable *iopt,unsigned long iova, +unsigned long length,struct page **out_pages, +unsigned int flags); +void iopt_unpin_pages (struct io_pagetable *iopt,unsigned long iova, +unsigned long length); +struct iommufd_ucmd {struct iommufd_ctx *ictx;void __user *ubuffer;
drivers/iommu/iommufd/device.c +8 -109
149 lines not on this page · raw message
drivers/iommu/iommufd/io_pagetable.c +95 -0
98 lines not on this page · raw message
Signature · 1 line
2.43.0
PATCH v3 10/23 iommufd/viommu: Introduce IOMMUFD_OBJ_VQUEUE and its related struct
In reply to Nicolin ChenAdd a new IOMMUFD_OBJ_VQUEUE with an iommufd_vqueue structure representing a HW-accelerated queue type of physical IOMMU's passed to a user space VM. This vQUEUE object, is a subset of vIOMMU resources of a physical IOMMU's, such as:
- NVIDIA's Virtual Command Queue
- AMD vIOMMU's Command Buffer, Event Log Buffer, and PPR Log Buffer
Introduce a struct iommufd_vqueue and an allocator iommufd_vqueue_alloc(). Also, add a pair of viommu ops for iommufd to forward user space ioctls to IOMMU drivers.
--- include/linux/iommufd.h | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+)
include/linux/iommufd.h +38 -0
@@ -37,6 +37,7 @@ enum iommufd_object_type {IOMMUFD_OBJ_VIOMMU ,IOMMUFD_OBJ_VDEVICE ,IOMMUFD_OBJ_VEVENTQ , +IOMMUFD_OBJ_VQUEUE , #ifdef CONFIG_IOMMUFD_TEST IOMMUFD_OBJ_SELFTEST, #endif
@@ -112,6 +113,14 @@ struct iommufd_vdevice {u64 id;/* per-vIOMMU virtual ID */ }; +struct iommufd_vqueue { +struct iommufd_object obj; +struct iommufd_ctx *ictx; +struct iommufd_viommu *viommu; +dma_addr_t addr;/* in guest physical address space */ +size_t length; +}; +/** * struct iommufd_viommu_ops - vIOMMU specific operations * @destroy: Clean up all driver-specific parts of an iommufd_viommu. The memory
@@ -137,6 +146,14 @@ struct iommufd_vdevice {* @vdevice_destroy: Clean up all driver-specific parts of an iommufd_vdevice. * The memory of the vDEVICE will be free-ed by iommufd core * after calling this op +* @vqueue_alloc: Allocate a vQUEUE object for a HW-accelerated queue given the +* @type (must be defined in include/uapi/linux/iommufd.h) for a +* @viommu. @index carries the logical vQUEUE ID per @viommu in a +* guest VM; @addr carries the guest physical base address of the +* queue memory; @length carries the size of the guest queue +* @vqueue_destroy: Clean up all driver-specific parts of an iommufd_vqueue. The +* memory of the iommufd_vqueue will be free-ed by iommufd core +* after calling this op */ struct iommufd_viommu_ops {void (*destroy)(struct iommufd_viommu *viommu);
@@ -149,6 +166,10 @@ struct iommufd_viommu_ops {struct device *dev, u64 virt_id);void (*vdevice_destroy)(struct iommufd_vdevice *vdev); +struct iommufd_vqueue *(*vqueue_alloc)(struct iommufd_viommu *viommu, +unsigned int type,u32 index, +dma_addr_t addr,size_t length); +void (*vqueue_destroy)(struct iommufd_vqueue *vqueue); };#if IS_ENABLED (CONFIG_IOMMUFD )
@@ -292,6 +313,23 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, ret; \ }) +#define iommufd_vqueue_alloc (viommu, drv_struct, member) \ + ({ \ + drv_struct *ret; \ + \ + static_assert(__same_type(struct iommufd_viommu, *viommu)); \ + static_assert(__same_type(struct iommufd_vqueue, \ + ((drv_struct *)NULL)->member)); \ + static_assert(offsetof(drv_struct, member.obj) == 0); \ + ret = (drv_struct *)_iommufd_object_alloc( \ + viommu->ictx, sizeof(drv_struct), IOMMUFD_OBJ_VQUEUE); \ + if (!IS_ERR(ret)) { \ + ret->member.viommu = viommu; \ + ret->member.ictx = viommu->ictx; \ + } \ + ret; \ + }) +/* Helper for IOMMU driver to destroy structures created by allocators above */ #define iommufd_struct_destroy (drv_struct, member) \ ({ \
Signature · 1 line
2.43.0
Re: [PATCH v3 10/23] iommufd/viommu: Introduce IOMMUFD_OBJ_VQUEUE and its related struct
Quoted 7 lines · Nicolin Chen
Add a new
IOMMUFD_OBJ_VQUEUEwith aniommufd_vqueuestructure representing a HW-accelerated queue type of physical IOMMU's passed to a user space VM. This vQUEUE object, is a subset of vIOMMU resources of a physical IOMMU's, such as:
- NVIDIA's Virtual Command Queue
- AMD vIOMMU's Command Buffer, Event Log Buffer, and PPR Log Buffer
Quoted 7 lines
Introduce a struct
iommufd_vqueueand an allocatoriommufd_vqueue_alloc(). Also, add a pair of viommu ops for iommufd to forward user space ioctls to IOMMU drivers.Reviewed-by Lu Baolu <[email protected]>Reviewed-by Pranjal Shrivastava <[email protected]>Signed-off-by Nicolin Chen <[email protected]>
-Vasant
PATCH v3 11/23 iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
In reply to Nicolin ChenIntroduce a new IOMMUFD_CMD_VQUEUE_ALLOC ioctl for user space to allocate a vQUEUE object for a vIOMMU specific HW-accelerated virtual queue, e.g.:
- NVIDIA's Virtual Command Queue
- AMD vIOMMU's Command Buffer, Event Log Buffer, and PPR Log Buffer
This is a vIOMMU based ioctl. Simply increase the refcount of the vIOMMU.
--- drivers/iommu/iommufd/iommufd_private.h | 2 + include/uapi/linux/iommufd.h | 44 +++++++++++ drivers/iommu/iommufd/main.c | 6 ++ drivers/iommu/iommufd/viommu.c | 99 +++++++++++++++++++++++++ 4 files changed, 151 insertions(+)
drivers/iommu/iommufd/iommufd_private.h +2 -0
@@ -611,6 +611,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_vqueue_alloc_ioctl (struct iommufd_ucmd *ucmd); +void iommufd_vqueue_destroy (struct iommufd_object *obj);#ifdef CONFIG_IOMMUFD_TEST int iommufd_test (struct iommufd_ucmd *ucmd);
include/uapi/linux/iommufd.h +44 -0
54 lines not on this page · raw message
drivers/iommu/iommufd/main.c +6 -0
@@ -307,6 +307,7 @@ union ucmd_buffer {struct iommu_veventq_alloc veventq;struct iommu_vfio_ioas vfio_ioas;struct iommu_viommu_alloc viommu; +struct iommu_vqueue_alloc vqueue;#ifdef CONFIG_IOMMUFD_TEST struct iommu_test_cmd test;#endif
@@ -366,6 +367,8 @@ static const struct iommufd_ioctl_op iommufd_ioctl_ops[] = { __reserved),IOCTL_OP (IOMMU_VIOMMU_ALLOC ,iommufd_viommu_alloc_ioctl ,struct iommu_viommu_alloc ,out_viommu_id ), +IOCTL_OP (IOMMU_VQUEUE_ALLOC ,iommufd_vqueue_alloc_ioctl , +struct iommu_vqueue_alloc ,length ), #ifdef CONFIG_IOMMUFD_TEST IOCTL_OP(IOMMU_TEST_CMD, iommufd_test, struct iommu_test_cmd, last), #endif
@@ -511,6 +514,9 @@ static const struct iommufd_object_ops iommufd_object_ops[] = { [IOMMUFD_OBJ_VIOMMU ] = { .destroy = iommufd_viommu_destroy, }, + [IOMMUFD_OBJ_VQUEUE ] = { + .destroy = iommufd_vqueue_destroy, + }, #ifdef CONFIG_IOMMUFD_TEST [IOMMUFD_OBJ_SELFTEST ] = { .destroy = iommufd_selftest_destroy,
drivers/iommu/iommufd/viommu.c +99 -0
102 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
Hi Nicolin,
Quoted 7 lines · Nicolin Chen
Introduce a new
IOMMUFD_CMD_VQUEUE_ALLOCioctl for user space to allocate a vQUEUE object for a vIOMMU specific HW-accelerated virtual queue, e.g.:
- NVIDIA's Virtual Command Queue
- AMD vIOMMU's Command Buffer, Event Log Buffer, and PPR Log BufferThis is a vIOMMU based ioctl. Simply increase the refcount of the vIOMMU.
Quoted 9 lines
Reviewed-by Pranjal Shrivastava <[email protected]>Signed-off-by Nicolin Chen <[email protected]>--- drivers/iommu/iommufd/iommufd_private.h | 2 + include/uapi/linux/iommufd.h | 44 +++++++++++ drivers/iommu/iommufd/main.c | 6 ++ drivers/iommu/iommufd/viommu.c | 99 +++++++++++++++++++++++++ 4 files changed, 151 insertions(+)
.../...
Quoted 13 lines
include/uapi/linux/iommufd.h +13
-0+ +/** +* struct iommu_vqueue_alloc - ioctl(IOMMU_VQUEUE_ALLOC) +* @size: sizeof(struct iommu_vqueue_alloc) +* @flags: Must be 0 +* @viommu_id: Virtual IOMMU ID to associate the virtual queue with +* @type: One of enum iommu_vqueue_type +* @index: The logical index to the virtual queue per virtual IOMMU, for a multi +* queue model +* @out_vqueue_id: The ID of the new virtual queue +* @addr: Base address of the queue memory in the guest physical address space +* @length: Length of the queue memory in the guest physical address space +*
include/uapi/linux/iommufd.h struct iommu_veventq_alloc { · line 1181
Quoted 10 lines
include/uapi/linux/iommufd.h +10
-0+* Allocate a virtual queue object for a vIOMMU-specific HW-acceleration feature +* that allows HW to access a guest queue memory described by @addr and @length. +* It's suggested for VMM to back the queue memory using a single huge page with +* a proper alignment for its contiguity in the host physical address space. The +* call will fail, if the queue memory is not contiguous in the physical address +* space. Upon success, its underlying physical pages will be pinned to prevent +* VMM from unmapping them in the IOAS, until the virtual queue gets destroyed. +* +* A vIOMMU can allocate multiple queues, but it must use a different @index to +* separate each allocation, e.g. VCMDQ0, VCMDQ1, ...
This will handle multiple queues. But AMD vIOMMU needs to comunicate certain control bit setting which is not related to buffers like "Completion wait interrupt".
How do we handle that? extend iommu_queue_alloc() or have different interface?
-Vasant
Jason Gunthorpe
maintainerRe: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
Quoted 23 lines · Vasant Hegde
Quoted 14 lines
+/**
+ * structiommu_vqueue_alloc-ioctl(IOMMU_VQUEUE_ALLOC)
+ * @size:sizeof(struct iommu_vqueue_alloc)
+ * @flags: Must be 0
+ * @viommu_id: Virtual IOMMU ID to associate the virtual queue with
+ * @type: One of enumiommu_vqueue_type
+ * @index: The logical index to the virtual queue per virtual IOMMU, for a multi
+ * queue model
+ * @out_vqueue_id: The ID of the new virtual queue
+ * @addr: Base address of the queue memory in the guest physical address space
+ * @length: Length of the queue memory in the guest physical address space
+ *
+ * Allocate a virtual queue object for a vIOMMU-specific HW-acceleration feature
+ * that allows HW to access a guest queue memory described by @addr and @length.Quoted 8 lines
+ * It's suggested for VMM to back the queue memory using a single huge page with
+ * a proper alignment for its contiguity in the host physical address space. The
+ * call will fail, if the queue memory is not contiguous in the physical address
+ * space. Upon success, its underlying physical pages will be pinned to prevent
+ * VMM from unmapping them in the IOAS, until the virtual queue gets destroyed.
+ *
+ * A vIOMMU can allocate multiple queues, but it must use a different @index to
+ * separate each allocation, e.g. VCMDQ0, VCMDQ1, ...
Quoted 5 lines
This will handle multiple queues. But AMD vIOMMU needs to comunicate certain control bit setting which is not related to buffers like "Completion wait interrupt".
How do we handle that? extend
iommu_queue_alloc()or have different interface?
Do you need a modify queue operation?
Jason
Re: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
Hi Jason,
Quoted 24 lines · Jason Gunthorpe
Quoted 23 lines · Vasant Hegde
Quoted 22 lines
+/**
+ * structiommu_vqueue_alloc-ioctl(IOMMU_VQUEUE_ALLOC)
+ * @size:sizeof(struct iommu_vqueue_alloc)
+ * @flags: Must be 0
+ * @viommu_id: Virtual IOMMU ID to associate the virtual queue with
+ * @type: One of enumiommu_vqueue_type
+ * @index: The logical index to the virtual queue per virtual IOMMU, for a multi
+ * queue model
+ * @out_vqueue_id: The ID of the new virtual queue
+ * @addr: Base address of the queue memory in the guest physical address space
+ * @length: Length of the queue memory in the guest physical address space
+ *
+ * Allocate a virtual queue object for a vIOMMU-specific HW-acceleration feature
+ * that allows HW to access a guest queue memory described by @addr and @length.
+ * It's suggested for VMM to back the queue memory using a single huge page with
+ * a proper alignment for its contiguity in the host physical address space. The
+ * call will fail, if the queue memory is not contiguous in the physical address
+ * space. Upon success, its underlying physical pages will be pinned to prevent
+ * VMM from unmapping them in the IOAS, until the virtual queue gets destroyed.
+ *
+ * A vIOMMU can allocate multiple queues, but it must use a different @index to
+ * separate each allocation, e.g. VCMDQ0, VCMDQ1, ...
Quoted 7 lines
Quoted 5 lines
This will handle multiple queues. But AMD vIOMMU needs to comunicate certain control bit setting which is not related to buffers like "Completion wait interrupt".
How do we handle that? extend
iommu_queue_alloc()or have different interface?Do you need a modify queue operation?
We have two types of operations. One that impacts the queue, other set of bits which doesn't operate on qeueue.
ex: Event log buffer
- We configure "MMIO Offset 0010h Event Log Base Address Register" with Base
address and size
- MMIO Offset 0018h IOMMU Control Register
EventLogEn: Event log enable
* When guest sets this bit, qemu will trap and will send queue_alloc
* When guest disables this bit, qemu will trap and send vqueue_destroy
This part is fine.
EventIntEn: Event log interrupt enable
* When guest sets this bit, qemu will trap
* this needs to be communicated to Host so that we can program VF Control
BAR and enable the interrupt
- There is other bit "Completion wait interrupt enable" This doesn't related to any buffer. Instead if we configure this for
completion wait command it will generate interrupt.
I am asking how do we handle above two steps? Should it be part of queue IOCTL or may be some other IOCTL which just passes these info to HW driver?
-Vasant
Tian, Kevin
maintainerRE: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
Quoted 31 lines
From: Vasant Hegde <[email protected]>
Sent: Wednesday, May 7, 2025 3:42 PMHi Jason,
Quoted 1 line · Jason Gunthorpe
Do you need a modify queue operation?
We have two types of operations. One that impacts the queue, other set of bits
which doesn't operate on qeueue.ex: Event log buffer
- We configure "MMIO Offset 0010h Event Log Base Address Register" with
Base
address and size- MMIO Offset 0018h IOMMU Control Register
EventLogEn: Event log enable
* When guest sets this bit, qemu will trap and will sendqueue_alloc
* When guest disables this bit, qemu will trap and sendvqueue_destroyThis part is fine.
EventIntEn: Event log interrupt enable
* When guest sets this bit, qemu will trap
* this needs to be communicated to Host so that we can program VF
Control
BAR and enable the interrupt
Quoted 8 lines
- There is other bit "Completion wait interrupt enable" This doesn't related to any buffer. Instead if we configure this for
completion wait command it will generate interrupt.I am asking how do we handle above two steps? Should it be part of queue IOCTL
or may be some other IOCTL which just passes these info to HW driver?
Probably IOMMUFD_CMD_OPTION can server the purpose?
Jason Gunthorpe
maintainerRe: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
In reply to Vasant HegdeQuoted 6 lines · Vasant Hegde
- MMIO Offset 0018h IOMMU Control Register
EventLogEn: Event log enable
* When guest sets this bit, qemu will trap and will sendqueue_alloc
* When guest disables this bit, qemu will trap and sendvqueue_destroyThis part is fine.
Ok
Quoted 4 lines
EventIntEn: Event log interrupt enable
* When guest sets this bit, qemu will trap
* this needs to be communicated to Host so that we can program VF Control
BAR and enable the interrupt
This sounds like modifying the vqueue? Or maybe on the viommu?
Quoted 3 lines
- There is other bit "Completion wait interrupt enable" This doesn't related to any buffer. Instead if we configure this for
completion wait command it will generate interrupt.
This sounds like a modify on the VIOMMU object?
Jason
Re: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
Jason,
Quoted 8 lines · Jason Gunthorpe
Quoted 6 lines · Vasant Hegde
- MMIO Offset 0018h IOMMU Control Register
EventLogEn: Event log enable
* When guest sets this bit, qemu will trap and will sendqueue_alloc
* When guest disables this bit, qemu will trap and sendvqueue_destroyThis part is fine.
Quoted 8 lines
Ok
Quoted 4 lines
EventIntEn: Event log interrupt enable
* When guest sets this bit, qemu will trap
* this needs to be communicated to Host so that we can program VF Control
BAR and enable the interruptThis sounds like modifying the vqueue? Or maybe on the viommu?
IMO its VIOMMU as it informs HW to trigger interrupt or not.
Quoted 6 lines
Quoted 4 lines
- There is other bit "Completion wait interrupt enable" This doesn't related to any buffer. Instead if we configure this for
completion wait command it will generate interrupt.This sounds like a modify on the VIOMMU object?
Again in my view its VIOMMU object as it tells HW what to do when it finishes completion wait command.
-Vasant
Re: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
Quoted 8 lines · Vasant Hegde
Quoted 5 lines
Quoted 3 lines
- There is other bit "Completion wait interrupt enable" This doesn't related to any buffer. Instead if we configure this for
completion wait command it will generate interrupt.This sounds like a modify on the VIOMMU object?
Again in my view its VIOMMU object as it tells HW what to do when it finishes completion wait command.
According to the spec:
https://www.amd.com/content/dam/amd/en/documents/processor-tech-docs/specifications/48882_IOMMU.pdf
This is for an interrupt from a COMPLETION_WAIT command:
"The COMPLETION_WAIT command allows software to serialize itself
with IOMMU command processing. The COMPLETION_WAIT command does not finish until all older commands issued since a prior COMPLETION_WAIT have completely executed."
So, basically it's like the IRQ for CMD_SYNC on ARM. IMHO, this is very specific to Command Buffer (i.e. a vQUEUE object, and now HW QUEUE object), though the bit is located in a global IOMMU control register.
Looking at this paragraph:
"
To restart the IOMMU command processing after the IOMMU halts it, use the following procedure.
• Wait until CmdBufRun=0b in the IOMMU Status Register [MMIO Offset 2020h] so that all commands complete processing as the circumstances allow. CmdBufRun must be 0b to modify the command buffer registers properly.
• Set CmdBufEn=0b in the IOMMU Control Register [MMIO Offset 0018h].
• As necessary, change the following registers (e.g., to relocate the command buffer):
• the Command Buffer Base Address Register [MMIO Offset 0008h],
• the Command Buffer Head Pointer Register [MMIO Offset 2000h],
• the Command Buffer Tail Pointer Register [MMIO Offset 2008h].
• Any or all command buffer entries may be copied from the old command buffer to the new and software must set the head and tail pointers appropriately.
• Write the IOMMU Control Register [MMIO Offset 0018h] with CmdBufEn=1b and ComWaitIntEn as desired
",
the ComWaitIntEn bit is suggested to be set along with the CmdBufEn bit, i.e. it can be a part of the IOMMU_HW_QUEUE_ALLOC ioctl.
What I am not sure is if the HW allows setting the ComWaitIntEn bit after CmdBufEn=1, which seems to be unlikely but the spec does not highlight. If so, this would be an modification to the HW QUEUE, in which case we could either do an relocation of the HW QUEUE (where we can set the flag in the 2nd allocation) or add an new option via IOMMUFD_CMD_OPTION (as Kevin suggested), and I think it should be a per-HW_QUEUE option since it doesn't affect other type of queues like Event/PRR Log Buffers.
Similarly, an Event Log Buffer can have an EventIntEn flag; and a PPR Log Buffer can have an PprIntEn flag too, right?
Thanks
Nicolin
Jason Gunthorpe
maintainerRe: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
Quoted 8 lines · Nicolin Chen
What I am not sure is if the HW allows setting the ComWaitIntEn bit after CmdBufEn=1, which seems to be unlikely but the spec does not highlight. If so, this would be an modification to the HW QUEUE, in which case we could either do an relocation of the HW QUEUE (where we can set the flag in the 2nd allocation) or add an new option via
IOMMUFD_CMD_OPTION(as Kevin suggested), and I think it should be a per-HW_QUEUE option since it doesn't affect other type of queues like Event/PRR Log Buffers.
The main question is if the control is global to the entire VIOMMU and all its HW QUEUE's or local to a single HW QUEUE.
If it is global then some "modify viommu" operation should be used to change it.
If it is local then some "modify hw queu" operation.
IOMMUFD_CMD_OPTION could be used with an object_id == VIOMMU as a kind of modify..
Jason
Re: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
Quoted 11 lines · Jason Gunthorpe
Quoted 8 lines · Nicolin Chen
What I am not sure is if the HW allows setting the ComWaitIntEn bit after CmdBufEn=1, which seems to be unlikely but the spec does not highlight. If so, this would be an modification to the HW QUEUE, in which case we could either do an relocation of the HW QUEUE (where we can set the flag in the 2nd allocation) or add an new option via
IOMMUFD_CMD_OPTION(as Kevin suggested), and I think it should be a per-HW_QUEUE option since it doesn't affect other type of queues like Event/PRR Log Buffers.
Quoted 2 lines
The main question is if the control is global to the entire VIOMMU and all its HW QUEUE's or local to a single HW QUEUE.
Oh, that's right.. I recall AMD only has one Command Buffer, but can have dual Event Log Buffers and dual PPR Log Buffers.
And the EventIntEn or PprIntEn bit seem to be global for the dual buffers..
Quoted 7 lines
If it is global then some "modify viommu" operation should be used to change it.
If it is local then some "modify hw queu" operation.
IOMMUFD_CMD_OPTIONcould be used with anobject_id== VIOMMU as a kind of modify..
Vasant can confirm. But looks like it should be a vIOMMU option.
Thanks
Nicolin
Re: [PATCH v3 11/23] iommufd/viommu: Add IOMMUFD_CMD_VQUEUE_ALLOC ioctl
Hi Nicolin, Jason,
Quoted 12 lines · Nicolin Chen
Quoted 11 lines · Jason Gunthorpe
Quoted 8 lines · Nicolin Chen
What I am not sure is if the HW allows setting the ComWaitIntEn bit after CmdBufEn=1, which seems to be unlikely but the spec does not highlight. If so, this would be an modification to the HW QUEUE, in which case we could either do an relocation of the HW QUEUE (where we can set the flag in the 2nd allocation) or add an new option via
IOMMUFD_CMD_OPTION(as Kevin suggested), and I think it should be a per-HW_QUEUE option since it doesn't affect other type of queues like Event/PRR Log Buffers.
Quoted 5 lines
Quoted 2 lines
The main question is if the control is global to the entire VIOMMU and all its HW QUEUE's or local to a single HW QUEUE.
Oh, that's right.. I recall AMD only has one Command Buffer, but can have dual Event Log Buffers and dual PPR Log Buffers.
Right.
Quoted 3 lines
And the EventIntEn or PprIntEn bit seem to be global for the dual buffers..
Yes. But there are other bit to configure dual buffers etc. (like DualEventLogEn).
Quoted 11 lines
Quoted 8 lines
If it is global then some "modify viommu" operation should be used to change it.
If it is local then some "modify hw queu" operation.
IOMMUFD_CMD_OPTIONcould be used with anobject_id== VIOMMU as a kind of modify..Vasant can confirm. But looks like it should be a vIOMMU option.
I think CMD_OPTION will work. So something like below?
if (cmd_option_id == IOMMU_OPTION_VIOMMU && cmd->object_id == viommu_id)
iommufd_viommu->ops->viommu_options() ?
-Vasant
PATCH v3 12/23 iommufd/driver: Add iommufd_vqueue_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_vqueue_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 vQUEUE objects only.
--- include/linux/iommufd.h | 48 ++++++++++++++++++++++++++++++++++ drivers/iommu/iommufd/driver.c | 28 ++++++++++++++++++++ 2 files changed, 76 insertions(+)
include/linux/iommufd.h +48 -0
@@ -235,6 +235,10 @@ struct iommufd_object *_iommufd_object_alloc(struct iommufd_ctx *ictx,size_t size, enum iommufd_object_type type);void iommufd_object_abort (struct iommufd_ctx *ictx,struct iommufd_object *obj); +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,
@@ -255,6 +259,18 @@ static inline void iommufd_object_abort(struct iommufd_ctx *ictx, { } +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) {
@@ -339,4 +355,36 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu,iommufd_object_abort (drv_struct->member.ictx, \ &drv_struct->member.obj); \ }) + +/* +* Helpers for IOMMU driver to build/destroy a dependency between two sibling +* structures created by one of the allocators above +*/ +#define iommufd_vqueue_depend (vqueue_dependent, vqueue_depended, member) \ + ({ \ + static_assert(__same_type(struct iommufd_vqueue, \ + vqueue_dependent->member)); \ + static_assert(offsetof(typeof(*vqueue_dependent), \ + member.obj) == 0); \ + static_assert(__same_type(struct iommufd_vqueue, \ + vqueue_depended->member)); \ + static_assert(offsetof(typeof(*vqueue_depended), \ + member.obj) == 0); \ + _iommufd_object_depend(&vqueue_dependent->member.obj, \ + &vqueue_depended->member.obj); \ + }) + +#define iommufd_vqueue_undepend (vqueue_dependent, vqueue_depended, member) \ + ({ \ + static_assert(__same_type(struct iommufd_vqueue, \ + vqueue_dependent->member)); \ + static_assert(offsetof(typeof(*vqueue_dependent), \ + member.obj) == 0); \ + static_assert(__same_type(struct iommufd_vqueue, \ + vqueue_depended->member)); \ + static_assert(offsetof(typeof(*vqueue_depended), \ + member.obj) == 0); \ + _iommufd_object_undepend(&vqueue_dependent->member.obj, \ + &vqueue_depended->member.obj); \ + })#endif
drivers/iommu/iommufd/driver.c +28 -0
@@ -50,6 +50,34 @@ void iommufd_object_abort(struct iommufd_ctx *ictx, struct iommufd_object *obj) }EXPORT_SYMBOL_NS_GPL (iommufd_object_abort,"IOMMUFD" ); +/* 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 v3 13/23 iommufd/selftest: Add coverage for IOMMUFD_CMD_VQUEUE_ALLOC
In reply to Nicolin ChenSome simple tests for IOMMUFD_CMD_VQUEUE_ALLOC infrastructure covering the new iommufd_vqueue_depend/undepend() helpers.
--- drivers/iommu/iommufd/iommufd_test.h | 3 + tools/testing/selftests/iommu/iommufd_utils.h | 31 +++++++++ drivers/iommu/iommufd/selftest.c | 67 +++++++++++++++++++ tools/testing/selftests/iommu/iommufd.c | 59 ++++++++++++++++ .../selftests/iommu/iommufd_fail_nth.c | 6 ++ 5 files changed, 166 insertions(+)
drivers/iommu/iommufd/iommufd_test.h +3 -0
@@ -265,4 +265,7 @@ struct iommu_viommu_event_selftest { __u32 virt_id; }; +#define IOMMU_VQUEUE_TYPE_SELFTEST 0xdeadbeef +#define IOMMU_TEST_VQUEUE_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_vqueue_alloc (int fd,__u32 viommu_id,__u32 type, +__u32 idx,__u64 addr,__u64 length, +__u32 *vqueue_id) +{ +struct iommu_vqueue_alloc cmd = { + .size =sizeof (cmd), + .viommu_id = viommu_id, + .type = type, + .index = idx, + .addr = addr, + .length = length, + }; +int ret; + + ret =ioctl (fd,IOMMU_VQUEUE_ALLOC , &cmd); +if (ret) +return ret; +if (vqueue_id) + *vqueue_id = cmd.out_vqueue_id; +return 0 ; +} + +#define test_cmd_vqueue_alloc (viommu_id, type, idx, addr, len, vqueue_id) \ + ASSERT_EQ(0, _test_cmd_vqueue_alloc(self->fd, viommu_id, type, idx, \ + addr, len, vqueue_id)) +#define test_err_vqueue_alloc (_errno, viommu_id, type, idx, addr, len, \ + vqueue_id) \ + EXPECT_ERRNO(_errno, \ + _test_cmd_vqueue_alloc(self->fd, viommu_id, type, idx, \ + addr, len, vqueue_id)) +static int _test_cmd_veventq_alloc (int fd,__u32 viommu_id,__u32 type,__u32 *veventq_id,__u32 *veventq_fd) {
drivers/iommu/iommufd/selftest.c +67 -0
89 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 vqueue_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_vqueue_alloc (self->fd, viommu_id, +IOMMU_VQUEUE_TYPE_SELFTEST ,0 , iova, +PAGE_SIZE , &vqueue_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 v3 14/23 iommufd: Add mmap interface
In reply to Nicolin ChenFor vIOMMU passing through HW resources to user space (VMs), allowing a guest VM to control the passed through the HW directly by R/W accessing the hardware MMIO registers, add an mmap infrastructure to map a region of hardware MMIO pages to user space.
Maintain an mt_mmap per ictx to store iommufd_mmap structure pointers, for pfn and owner validations. To allow IOMMU drivers to add and delete mmappable regions onto/from the mt_mmap, add a pair of vIOMMU helpers:iommufd_viommu_alloc_mmap() and iommufd_viommu_destroy_mmap().
--- drivers/iommu/iommufd/iommufd_private.h | 11 +++++ include/linux/iommufd.h | 40 +++++++++++++++ drivers/iommu/iommufd/driver.c | 46 +++++++++++++++++ drivers/iommu/iommufd/main.c | 65 +++++++++++++++++++++++++ 4 files changed, 162 insertions(+)
drivers/iommu/iommufd/iommufd_private.h +11 -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,15 @@ struct iommufd_ctx {struct iommufd_ioas *vfio_ioas; }; +/* Entry for iommufd_ctx::mt_mmap */ +struct iommufd_mmap { +struct iommufd_object *owner; + +/* inclusive pfn range for mmap() */ +unsigned long pfn_start; +unsigned long pfn_end; +}; +/* * 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 +40 -0
@@ -239,6 +239,11 @@ 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); +int _iommufd_alloc_mmap (struct iommufd_ctx *ictx,struct iommufd_object *owner, +phys_addr_t base,size_t size,unsigned long *immap_id); +void _iommufd_destroy_mmap (struct iommufd_ctx *ictx, +struct iommufd_object *owner, +unsigned long immap_id);struct device *iommufd_viommu_find_dev (struct iommufd_viommu *viommu,unsigned long vdev_id); int iommufd_viommu_get_vdev_id(struct iommufd_viommu *viommu,
@@ -271,6 +276,20 @@ _iommufd_object_undepend(struct iommufd_object *obj_dependent, { } +static inline int _iommufd_alloc_mmap (struct iommufd_ctx *ictx, +struct iommufd_object *owner, +phys_addr_t base,size_t size, +unsigned long *immap_id) +{ +return -EOPNOTSUPP ; +} + +static inline void _iommufd_destroy_mmap (struct iommufd_ctx *ictx, +struct iommufd_object *owner, +unsigned long immap_id) +{ +} +static inline struct device *iommufd_viommu_find_dev (struct iommufd_viommu *viommu,unsigned long vdev_id) {
@@ -387,4 +406,25 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu,_iommufd_object_undepend (&vqueue_dependent->member.obj, \ &vqueue_depended->member.obj); \ }) + +/* +* Helpers for IOMMU driver to alloc/destroy an mmapable area for a structure. +* Driver should report the @out_immap_id to user space for an mmap() syscall +*/ +#define iommufd_viommu_alloc_mmap (viommu, member, base, size, out_immap_id) \ + ({ \ + static_assert(__same_type(struct iommufd_viommu, \ + viommu->member)); \ + static_assert(offsetof(typeof(*viommu), member.obj) == 0); \ + _iommufd_alloc_mmap(viommu->member.ictx, &viommu->member.obj, \ + base, size, out_immap_id); \ + }) +#define iommufd_viommu_destroy_mmap (viommu, member, immap_id) \ + ({ \ + static_assert(__same_type(struct iommufd_viommu, \ + viommu->member)); \ + static_assert(offsetof(typeof(*viommu), member.obj) == 0); \ + _iommufd_destroy_mmap(viommu->member.ictx, &viommu->member.obj,\ + immap_id); \ + })#endif
drivers/iommu/iommufd/driver.c +46 -0
52 lines not on this page · raw message
drivers/iommu/iommufd/main.c +65 -0
82 lines not on this page · raw message
Signature · 1 line
2.43.0
PATCH v3 15/23 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 +++ drivers/iommu/iommufd/selftest.c | 40 ++++++++++++++++++++++--- tools/testing/selftests/iommu/iommufd.c | 7 +++++ 3 files changed, 47 insertions(+), 4 deletions(-)
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_pgoff: The offset argument for mmap syscall +* @out_mmap_pgsz: Maximum page size 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_pgoff; +__aligned_u64 out_mmap_pgsz; };/* Should not be equal to any defined value in enum iommu_viommu_invalidate_data_type */
drivers/iommu/iommufd/selftest.c +36 -4
@@ -149,6 +149,9 @@ struct mock_viommu {struct iommufd_viommu core;struct mock_iommu_domain *s2_parent;struct mock_vqueue *mock_vqueue[IOMMU_TEST_VQUEUE_MAX ]; + +unsigned long mmap_pgoff; +u32 *page;/* Mmap page to test u32 type of in_data */ };static inline struct mock_viommu *to_mock_viommu (struct iommufd_viommu *viommu)
@@ -645,9 +648,14 @@ 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); +if (mock_viommu->mmap_pgoff) +iommufd_viommu_destroy_mmap (mock_viommu, core, + mock_viommu->mmap_pgoff); +free_page ((unsigned long )mock_viommu->page);/* iommufd core frees mock_viommu and viommu */ }
@@ -827,17 +835,41 @@ mock_viommu_alloc(struct device *dev, struct iommu_domain *domain,return ERR_CAST (mock_viommu);if (user_data) { + mock_viommu->page = + (u32 *)__get_free_page (GFP_KERNEL | __GFP_ZERO); +if (!mock_viommu->page) { + rc = -ENOMEM ; + goto err_destroy_struct; + } + + rc =iommufd_viommu_alloc_mmap (mock_viommu, core, +__pa (mock_viommu->page), +PAGE_SIZE , + &mock_viommu->mmap_pgoff); +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_pgsz =PAGE_SIZE ; + data.out_mmap_pgoff = mock_viommu->mmap_pgoff; rc =iommu_copy_struct_to_user ( user_data, &data,IOMMU_VIOMMU_TYPE_SELFTEST , out_data); -if (rc) { -iommufd_struct_destroy (mock_viommu, core); -return ERR_PTR (rc); - } +if (rc) + goto err_destroy_mmap; }refcount_inc (&mock_iommu->users);return &mock_viommu->core; + +err_destroy_mmap: +iommufd_viommu_destroy_mmap (mock_viommu, core, mock_viommu->mmap_pgoff); +err_free_page: +free_page ((unsigned long )mock_viommu->page); +err_destroy_struct: +iommufd_struct_destroy (mock_viommu, core); +return ERR_PTR (rc); }static const struct iommu_ops mock_ops = {
tools/testing/selftests/iommu/iommufd.c +7 -0
@@ -2799,12 +2799,19 @@ TEST_F(iommufd_viommu, viommu_alloc_with_data)struct iommu_viommu_selftest data = { .in_data =0xbeef , }; +uint32_t *test;if (self->device_id) {test_cmd_viommu_alloc (self->device_id, self->hwpt_id,IOMMU_VIOMMU_TYPE_SELFTEST , &data,sizeof (data), &self->viommu_id);assert (data.out_data == data.in_data); + test =mmap (NULL , data.out_mmap_pgsz,PROT_READ |PROT_WRITE , +MAP_SHARED , self->fd, data.out_mmap_pgoff); +assert (test && *test == data.in_data); +EXPECT_ERRNO (EBUSY , +_test_ioctl_destroy (self->fd, self->viommu_id)); +munmap (test, data.out_mmap_pgsz); } }
Signature · 1 line
2.43.0
PATCH v3 16/23 Documentation: userspace-api: iommufd: Update vQUEUE
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 | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
Documentation/userspace-api/iommufd.rst +15 -0
@@ -124,6 +124,20 @@ 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_VQUEUE, representing a hardware accelerated virtual 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 + allows VM to work with the IOMMU HW directly without a VM Exit, i.e. reducing + overhead from the hypercalls. Along with this vQUEUE 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 control the allocated vQUEUE HW. Thus, when allocating a vQUEUE, + the VMM must request a pair of VMA info (vm_pgoff/size) for an mmap syscall. + The length argument of an mmap syscall can be smaller than the given size for + a partial mmap, but the addr argument of the mmap syscall should never offset + from the returned vm_pgoff, which implies that an mmap will always start from + the beginning of the physical MMIO region. + All user-visible objects are destroyed via the IOMMU_DESTROY uAPI. The diagrams below show relationships between user-visible objects and kernel
@@ -270,6 +284,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_vqueue for IOMMUFD_OBJ_VQUEUE. Several terminologies when looking at these datastructures:
Signature · 1 line
2.43.0
Re: [PATCH v3 16/23] Documentation: userspace-api: iommufd: Update vQUEUE
Documentation/userspace-api/iommufd.rst Following IOMMUFD objects are exposed to userspace: · line 138
Quoted 12 lines · Nicolin Chen
+-
IOMMUFD_OBJ_VQUEUE, representing a hardware accelerated virtual queue, as aDocumentation/userspace-api/iommufd.rst +11
-0+ 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 + allows VM to work with the IOMMU HW directly without a VM Exit, i.e. reducing + overhead from the hypercalls. Along with this vQUEUE 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 control the allocated vQUEUE HW. Thus, when allocating a vQUEUE, + the VMM must request a pair of VMA info (vm_pgoff/size) for an mmap syscall. + The length argument of an mmap syscall can be smaller than the given size for + a partial mmap, but the addr argument of the mmap syscall should never offset + from the returned vm_pgoff, which implies that an mmap will always start from
Did you mean never be offset from returned vm_pgoff?
Documentation/userspace-api/iommufd.rst Following IOMMUFD objects are exposed to userspace: · line 140
Quoted 2 lines
Documentation/userspace-api/iommufd.rst +2
-0+ the beginning of the physical MMIO region. +
Confused...
Signature · 1 line
An old man doll... just what I always wanted! - Clara
Re: [PATCH v3 16/23] Documentation: userspace-api: iommufd: Update vQUEUE
Quoted 14 lines · Bagas Sanjaya
Documentation/userspace-api/iommufd.rst Following IOMMUFD objects are exposed to userspace: · line 138
Quoted 12 lines · Nicolin Chen
+-
IOMMUFD_OBJ_VQUEUE, representing a hardware accelerated virtual queue, as aDocumentation/userspace-api/iommufd.rst +11
-0+ 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 + allows VM to work with the IOMMU HW directly without a VM Exit, i.e. reducing + overhead from the hypercalls. Along with this vQUEUE 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 control the allocated vQUEUE HW. Thus, when allocating a vQUEUE, + the VMM must request a pair of VMA info (vm_pgoff/size) for an mmap syscall. + The length argument of an mmap syscall can be smaller than the given size for + a partial mmap, but the addr argument of the mmap syscall should never offset + from the returned vm_pgoff, which implies that an mmap will always start from
Quoted 1 line
Did you mean never be offset from returned
vm_pgoff?
Yes. Will fix this.
Quoted 4 lines
Documentation/userspace-api/iommufd.rst Following IOMMUFD objects are exposed to userspace: · line 140
Quoted 2 lines
Documentation/userspace-api/iommufd.rst +2
-0+ the beginning of the physical MMIO region. +Confused...
Meaning that VMM should just use the given vm_pgoff as is, without adding any offset to the vm_pgoff.
Thanks
Nicolin
Re: [PATCH v3 16/23] Documentation: userspace-api: iommufd: Update vQUEUE
Quoted 19 lines · Nicolin Chen
Quoted 14 lines · Bagas Sanjaya
Documentation/userspace-api/iommufd.rst Following IOMMUFD objects are exposed to userspace: · line 138
Quoted 12 lines · Nicolin Chen
+-
IOMMUFD_OBJ_VQUEUE, representing a hardware accelerated virtual queue, as aDocumentation/userspace-api/iommufd.rst +11
-0+ 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 + allows VM to work with the IOMMU HW directly without a VM Exit, i.e. reducing + overhead from the hypercalls. Along with this vQUEUE 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 control the allocated vQUEUE HW. Thus, when allocating a vQUEUE, + the VMM must request a pair of VMA info (vm_pgoff/size) for an mmap syscall. + The length argument of an mmap syscall can be smaller than the given size for + a partial mmap, but the addr argument of the mmap syscall should never offset + from the returned vm_pgoff, which implies that an mmap will always start fromQuoted 1 line
Did you mean never be offset from returned
vm_pgoff?Yes. Will fix this.
Quoted 7 lines
Quoted 4 lines
Documentation/userspace-api/iommufd.rst Following IOMMUFD objects are exposed to userspace: · line 140
Quoted 2 lines
Documentation/userspace-api/iommufd.rst +2
-0+ the beginning of the physical MMIO region. +Confused...
Meaning that VMM should just use the given
vm_pgoffas is, without adding any offset to thevm_pgoff.
Understood, thanks!
Signature · 1 line
An old man doll... just what I always wanted! - Clara
PATCH v3 17/23 iommu/arm-smmu-v3-iommufd: Add vsmmu_alloc impl op
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 a vsmmu_alloc op and prioritize it in arm_vsmmu_alloc().
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 6 ++++++ .../iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 17 +++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +6 -0
@@ -16,6 +16,7 @@#include <linux/sizes.h> struct arm_smmu_device ; +struct arm_smmu_domain ;/* MMIO registers */ #define ARM_SMMU_IDR0 0x0
@@ -720,6 +721,11 @@ 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); +struct arm_vsmmu *(*vsmmu_alloc)( +struct arm_smmu_device *smmu, +struct arm_smmu_domain *smmu_domain,struct iommufd_ctx *ictx, +unsigned int viommu_type, +const struct iommu_user_data *user_data); };/* An SMMUv3 instance */
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +11 -6
@@ -392,10 +392,7 @@ struct iommufd_viommu *arm_vsmmu_alloc(struct device *dev, iommu_get_iommu_dev(dev, struct arm_smmu_device, iommu);struct arm_smmu_master *master =dev_iommu_priv_get (dev);struct arm_smmu_domain *s2_parent =to_smmu_domain (parent); -struct arm_vsmmu *vsmmu; - -if (viommu_type !=IOMMU_VIOMMU_TYPE_ARM_SMMUV3 ) -return ERR_PTR (-EOPNOTSUPP ); +struct arm_vsmmu *vsmmu =ERR_PTR (-EOPNOTSUPP );if (!(smmu->features &ARM_SMMU_FEAT_NESTING ))return ERR_PTR (-EOPNOTSUPP );
@@ -423,8 +420,16 @@ struct iommufd_viommu *arm_vsmmu_alloc(struct device *dev, !(smmu->features & ARM_SMMU_FEAT_S2FWB))return ERR_PTR (-EOPNOTSUPP ); - vsmmu = iommufd_viommu_alloc(ictx, struct arm_vsmmu, core, - &arm_vsmmu_ops); +/* Prioritize the impl that may support IOMMU_VIOMMU_TYPE_ARM_SMMUV3 */ +if (master->smmu->impl_ops && master->smmu->impl_ops->vsmmu_alloc) + vsmmu = master->smmu->impl_ops->vsmmu_alloc ( + master->smmu, s2_parent, ictx, viommu_type, user_data); +if (PTR_ERR (vsmmu) == -EOPNOTSUPP ) { +/* Otherwise, allocate an IOMMU_VIOMMU_TYPE_ARM_SMMUV3 here */ +if (viommu_type ==IOMMU_VIOMMU_TYPE_ARM_SMMUV3 ) + vsmmu = iommufd_viommu_alloc(ictx, struct arm_vsmmu, + core, &arm_vsmmu_ops); + }if (IS_ERR (vsmmu))return ERR_CAST (vsmmu);
Signature · 1 line
2.43.0
PATCH v3 18/23 iommu/arm-smmu-v3-iommufd: Support implementation-defined hw_info
In reply to Nicolin ChenRepurpose the @__reserved field in the struct iommu_hw_info_arm_smmuv3, to an HW implementation-defined field @impl.
This will be used by Tegra241 CMDQV implementation on top of a standard ARM SMMUv3 IOMMU. The @impl will be only valid if @flags is set with an implementation-defined flag.
Thus in the driver-level, add an hw_info impl op that will return such a flag and fill the impl field.
--- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 + include/uapi/linux/iommufd.h | 4 ++-- .../iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 16 +++++++++++++--- 3 files changed, 16 insertions(+), 5 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +1 -0
@@ -726,6 +726,7 @@ struct arm_smmu_impl_ops { struct arm_smmu_domain *smmu_domain, struct iommufd_ctx *ictx, unsigned int viommu_type, const struct iommu_user_data *user_data); +u32 (*hw_info)(struct arm_smmu_device *smmu,u32 *impl); };/* An SMMUv3 instance */
include/uapi/linux/iommufd.h +2 -2
@@ -554,7 +554,7 @@ struct iommu_hw_info_vtd {* (IOMMU_HW_INFO_TYPE_ARM_SMMUV3) * * @flags: Must be set to 0 -* @__reserved: Must be 0 +* @impl: Must be 0 * @idr: Implemented features for ARM SMMU Non-secure programming interface * @iidr: Information about the implementation and implementer of ARM SMMU, * and architecture version supported
@@ -585,7 +585,7 @@ struct iommu_hw_info_vtd {*/ struct iommu_hw_info_arm_smmuv3 {__u32 flags; -__u32 __reserved; +__u32 impl;__u32 idr[6 ];__u32 iidr;__u32 aidr;
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +13 -3
@@ -10,7 +10,9 @@void *arm_smmu_hw_info (struct device *dev,u32 *length,u32 *type) {struct arm_smmu_master *master =dev_iommu_priv_get (dev); +struct arm_smmu_device *smmu = master->smmu;struct iommu_hw_info_arm_smmuv3 *info; +u32 flags =0 , impl =0 ;u32 __iomem *base_idr;unsigned int i;
@@ -18,15 +20,23 @@ void *arm_smmu_hw_info(struct device *dev, u32 *length, u32 *type)if (!info)return ERR_PTR (-ENOMEM ); - base_idr = master->smmu->base +ARM_SMMU_IDR0 ; + base_idr = smmu->base +ARM_SMMU_IDR0 ;for (i =0 ; i <=5 ; i++) info->idr[i] =readl_relaxed (base_idr + i); - info->iidr =readl_relaxed (master->smmu->base +ARM_SMMU_IIDR ); - info->aidr =readl_relaxed (master->smmu->base +ARM_SMMU_AIDR ); + info->iidr =readl_relaxed (smmu->base +ARM_SMMU_IIDR ); + info->aidr =readl_relaxed (smmu->base +ARM_SMMU_AIDR ); *length =sizeof (*info); *type =IOMMU_HW_INFO_TYPE_ARM_SMMUV3 ; +if (smmu->impl_ops && smmu->impl_ops->hw_info) { + flags = smmu->impl_ops->hw_info (smmu, &impl); +if (flags) { + info->impl = impl; + info->flags |= flags; + } + } +return info; }
Signature · 1 line
2.43.0
PATCH v3 19/23 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 v3 20/23 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 v3 21/23 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.
Delay the LVCMDQ mappings to tegra241_vintf_hw_init(), and the unmappings to tegra241_vintf_hw_deinit().
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 v3 22/23 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 (vQUEUE objects) 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 | 15 + include/uapi/linux/iommufd.h | 50 ++- .../arm/arm-smmu-v3/arm-smmu-v3-iommufd.c | 6 +- .../iommu/arm/arm-smmu-v3/tegra241-cmdqv.c | 404 +++++++++++++++++- 4 files changed, 466 insertions(+), 9 deletions(-)
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h +15 -0
@@ -1000,6 +1000,14 @@ int arm_smmu_cmdq_issue_cmdlist(struct arm_smmu_device *smmu, struct arm_smmu_cmdq *cmdq, u64 *cmds, int n, bool sync); +static inline phys_addr_t +arm_smmu_domain_ipa_to_pa (struct arm_smmu_domain *smmu_domain,u64 ipa) +{ +if (WARN_ON_ONCE (smmu_domain->stage !=ARM_SMMU_DOMAIN_S2 )) +return 0 ; +return iommu_iova_to_phys (&smmu_domain->domain, ipa); +} +#ifdef CONFIG_ARM_SMMU_V3_SVA bool arm_smmu_sva_supported (struct arm_smmu_device *smmu);bool arm_smmu_master_sva_supported (struct arm_smmu_master *master);
@@ -1076,9 +1084,16 @@ 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_hw_info NULL#define arm_vsmmu_alloc 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 +48 -2
79 lines not on this page · raw message
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-iommufd.c +3 -3
@@ -226,7 +226,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) {
@@ -337,8 +337,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 +400 -4
570 lines not on this page · raw message
Signature · 1 line
2.43.0
Re: [PATCH v3 22/23] iommu/tegra241-cmdqv: Add user-space use support
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c tegra241_cmdqv_get_cmdq(struct arm_smmu_device *smmu, · line 411
Quoted 10 lines · Nicolin Chen
drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c +10
-0+static void tegra241_vcmdq_hw_flush_timeout (struct tegra241_vcmdq *vcmdq) +{ +struct arm_smmu_device *smmu = &vcmdq->cmdqv->smmu; +u64 cmd_sync[CMDQ_ENT_DWORDS ] = {}; + + cmd_sync[0 ] =FIELD_PREP (CMDQ_0_OP ,CMDQ_OP_CMD_SYNC ) | +FIELD_PREP (CMDQ_SYNC_0_CS ,CMDQ_SYNC_0_CS_NONE ); + +/* +* It does not hurt to insert another CMD_SYNC, taking adavantage of the
Oh, ignore the "adavantage". Will fix in the next version.
Nicolin
PATCH v3 23/23 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
@@ -1114,10 +1114,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 , };/**
@@ -1141,6 +1143,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
@@ -292,6 +292,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;
@@ -337,6 +351,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