Re: [PATCH v6 08/25] iommufd/viommu: Add driver-defined vDEVICE support

On Sat, Jun 14, 2025 at 12:14:33AM -0700, Nicolin Chen wrote:

NVIDIA VCMDQ driver will have a driver-defined vDEVICE structure and do some HW configurations with that.

To allow IOMMU drivers to define their own vDEVICE structures, move the struct iommufd_vdevice to the public header and provide a pair of viommu ops, similar to get_viommu_size and viommu_init.

Doing this, however, creates a new window between the vDEVICE allocation and its driver-level initialization, during which an abort could happen but it can't invoke a driver destroy function from the struct viommu_ops since the driver structure isn't initialized yet. vIOMMU object doesn't have this problem, since its destroy op is set via the viommu_ops by the driver viommu_init function. Thus, vDEVICE should do something similar: add a destroy function pointer inside the struct iommufd_vdevice instead of the struct iommufd_viommu_ops.

Note that there is unlikely a use case for a type dependent vDEVICE, so a static vdevice_size is probably enough for the near term instead of a get_vdevice_size function op.

Reviewed-by: Jason Gunthorpe jgg@nvidia.com Reviewed-by: Kevin Tian kevin.tian@intel.com Signed-off-by: Nicolin Chen nicolinc@nvidia.com

---

drivers/iommu/iommufd/iommufd_private.h | 7 ------- include/linux/iommufd.h | 26 +++++++++++++++++++++++++ drivers/iommu/iommufd/viommu.c | 26 ++++++++++++++++++++++++- 3 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 468717d5e5bc..e6b1eb2ab375 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -638,13 +638,6 @@ void iommufd_viommu_destroy(struct iommufd_object *obj); int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd); void iommufd_vdevice_destroy(struct iommufd_object *obj); -struct iommufd_vdevice {

• struct iommufd_object obj;
• struct iommufd_viommu *viommu;
• struct device *dev;
• 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); diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h index 2d1bf2f97ee3..f3b5cfdb6d53 100644 --- a/include/linux/iommufd.h +++ b/include/linux/iommufd.h @@ -104,6 +104,16 @@ struct iommufd_viommu { enum iommu_viommu_type type; }; +struct iommufd_vdevice {

• struct iommufd_object obj;
• struct iommufd_viommu *viommu;
• struct device *dev;
• u64 id; /* per-vIOMMU virtual ID */

Nit: Why not call this viommu_id?

• /* Clean up all driver-specific parts of an iommufd_vdevice */
• void (*destroy)(struct iommufd_vdevice *vdev);

+};

/**

• struct iommufd_viommu_ops - vIOMMU specific operations
• @destroy: Clean up all driver-specific parts of an iommufd_viommu. The memory

@@ -120,6 +130,14 @@ struct iommufd_viommu {

•                array->entry_num to report the number of handled requests.
  

•                The data structure of the array entry must be defined in
  

•                include/uapi/linux/iommufd.h
  

• • @vdevice_size: Size of the driver-defined vDEVICE structure per this vIOMMU
• • @vdevice_init: Initialize the driver-level structure of a vDEVICE object, or

• •            related HW procedure. @vdev is already initialized by iommufd
    

• •            core: vdev->dev and vdev->viommu pointers; vdev->id carries a
    

• •            per-vIOMMU virtual ID (refer to struct iommu_vdevice_alloc in
    

• •            include/uapi/linux/iommufd.h)
    

• •            If driver has a deinit function to revert what vdevice_init op
    

• •            does, it should set it to the @vdev->destroy function pointer
    

  */

struct iommufd_viommu_ops { void (*destroy)(struct iommufd_viommu *viommu); @@ -128,6 +146,8 @@ struct iommufd_viommu_ops { const struct iommu_user_data *user_data); int (*cache_invalidate)(struct iommufd_viommu *viommu, struct iommu_user_data_array *array);

• const size_t vdevice_size;
• int (*vdevice_init)(struct iommufd_vdevice *vdev);

}; #if IS_ENABLED(CONFIG_IOMMUFD) @@ -224,4 +244,10 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, BUILD_BUG_ON_ZERO(offsetof(drv_struct, member)) + \ BUILD_BUG_ON_ZERO(!__same_type(struct iommufd_viommu, \ ((drv_struct *)NULL)->member)))

+#define VDEVICE_STRUCT_SIZE(drv_struct, member) \

• (sizeof(drv_struct) + \

• BUILD_BUG_ON_ZERO(offsetof(drv_struct, member)) +                     \
  

• BUILD_BUG_ON_ZERO(!__same_type(struct iommufd_vdevice,                \
  

• 			((drv_struct *)NULL)->member)))
  

#endif diff --git a/drivers/iommu/iommufd/viommu.c b/drivers/iommu/iommufd/viommu.c index c5eea9900c54..28ea5d026222 100644 --- a/drivers/iommu/iommufd/viommu.c +++ b/drivers/iommu/iommufd/viommu.c @@ -116,6 +116,8 @@ void iommufd_vdevice_destroy(struct iommufd_object *obj) container_of(obj, struct iommufd_vdevice, obj); struct iommufd_viommu *viommu = vdev->viommu;

• if (vdev->destroy)

• vdev->destroy(vdev);
  

  /* xa_cmpxchg is okay to fail if alloc failed xa_cmpxchg previously */ xa_cmpxchg(&viommu->vdevs, vdev->id, vdev, NULL, GFP_KERNEL); refcount_dec(&viommu->obj.users);

@@ -126,6 +128,7 @@ int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd) { struct iommu_vdevice_alloc *cmd = ucmd->cmd; struct iommufd_vdevice *vdev, *curr;

• size_t vdev_size = sizeof(*vdev); struct iommufd_viommu *viommu; struct iommufd_device *idev; u64 virt_id = cmd->virt_id;

@@ -150,7 +153,22 @@ int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_idev; }

• vdev = iommufd_object_alloc_ucmd(ucmd, vdev, IOMMUFD_OBJ_VDEVICE);

• if (viommu->ops && viommu->ops->vdevice_size) {

• /*
  

•  * It is a driver bug for:
  

•  * - ops->vdevice_size smaller than the core structure size
  

•  * - not implementing a pairing ops->vdevice_init op
  

•  */
  

• if (WARN_ON_ONCE(viommu->ops->vdevice_size < vdev_size ||
  

• 		 !viommu->ops->vdevice_init)) {
  

• 	rc = -EOPNOTSUPP;
  

• 	goto out_put_idev;
  

• }
  

• vdev_size = viommu->ops->vdevice_size;
  

• }
• vdev = (struct iommufd_vdevice *)_iommufd_object_alloc_ucmd(

• ucmd, vdev_size, IOMMUFD_OBJ_VDEVICE);
  

  if (IS_ERR(vdev)) { rc = PTR_ERR(vdev); goto out_put_idev;

@@ -168,6 +186,12 @@ int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd) goto out_put_idev; }

• if (viommu->ops && viommu->ops->vdevice_init) {

• rc = viommu->ops->vdevice_init(vdev);
  

• if (rc)
  

• 	goto out_put_idev;
  

• }
• cmd->out_vdevice_id = vdev->obj.id; rc = iommufd_ucmd_respond(ucmd, sizeof(*cmd));

Reviewed-by: Pranjal Shrivastava praan@google.com

-- 2.43.0