On Fri, Apr 25, 2025 at 10:58:06PM -0700, Nicolin Chen wrote:
NVIDIA 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 the destroy op returns void.
Add iommufd_vcmdq_depend/undepend() for-driver helpers, allowing LVCMDQ allocator to refcount_inc() a sibling LVCMDQ object and LVCMDQ destroyer to refcount_dec().
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.
Signed-off-by: Nicolin Chen nicolinc@nvidia.com
include/linux/iommufd.h | 47 ++++++++++++++++++++++++++++++++++ drivers/iommu/iommufd/driver.c | 28 ++++++++++++++++++++ 2 files changed, 75 insertions(+)
diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h index e91381aaec5a..5dff154e8ce1 100644 --- a/include/linux/iommufd.h +++ b/include/linux/iommufd.h @@ -232,6 +232,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, @@ -252,6 +256,17 @@ 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) { @@ -329,4 +344,36 @@ static inline int iommufd_viommu_report_event(struct iommufd_viommu *viommu, static_assert(offsetof(typeof(*drv_struct), member.obj) == 0); \ iommufd_object_abort(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_vcmdq_depend(vcmdq_dependent, vcmdq_depended, member) \
- ({ \
static_assert(__same_type(struct iommufd_object, \vcmdq_dependent->member.obj)); \static_assert(offsetof(typeof(*vcmdq_dependent), \member.obj) == 0); \static_assert(__same_type(struct iommufd_object, \vcmdq_depended->member.obj)); \static_assert(offsetof(typeof(*vcmdq_depended), \member.obj) == 0); \iommufd_object_depend(&vcmdq_dependent->member.obj, \&vcmdq_depended->member.obj); \- })
+#define iommufd_vcmdq_undepend(vcmdq_dependent, vcmdq_depended, member) \
- ({ \
static_assert(__same_type(struct iommufd_object, \vcmdq_dependent->member.obj)); \static_assert(offsetof(typeof(*vcmdq_dependent), \member.obj) == 0); \static_assert(__same_type(struct iommufd_object, \vcmdq_depended->member.obj)); \static_assert(offsetof(typeof(*vcmdq_depended), \member.obj) == 0); \iommufd_object_undepend(&vcmdq_dependent->member.obj, \&vcmdq_depended->member.obj); \- })
#endif diff --git a/drivers/iommu/iommufd/driver.c b/drivers/iommu/iommufd/driver.c index 7980a09761c2..fb7f8fe40f95 100644 --- a/drivers/iommu/iommufd/driver.c +++ b/drivers/iommu/iommufd/driver.c @@ -50,6 +50,34 @@ void iommufd_object_abort(struct iommufd_ctx *ictx, struct iommufd_object *obj) } EXPORT_SYMBOL_NS_GPL(iommufd_object_abort, "IOMMUFD"); +/* A per-structure helper is available 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");
+/* A per-structure helper is available 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)
If I'm getting this right, I think we are setting up dependencies like: vcmdq[2] -> vcmdq[1] -> vcmdq[0] based on refcounts of each object, which ensures that the unmaps happen in descending order..
If that's right, Is it fair to have iommufd_vcmdq_depend/undepend in the core code itself? Since it's a driver-level limitation, I think we should just have iommufd_object_depend/undepend in the core code and the iommufd_vcmdq_depend/undepend can move into the CMDQV driver?
-- 2.43.0
Thanks, Praan