Re: [PATCH v6 07/25] iommufd/access: Add internal APIs for HW queue to use

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

Now, access->ops can be NULL, to support an internal use case for the new HW queue object. Since an access object in this case will be allocated by an inernal iommufd object, the refcount on the ictx should be skipped, so as not to deadlock the release of the ictx as it would otherwise wait for the release of the access first during the release of the internal object that could wait for the release of ictx: ictx --releases--> hw_queue --releases--> access ^ | |_________________releases________________v

Add a set of lightweight internal APIs to unlink access and ictx: ictx --releases--> hw_queue --releases--> access

Signed-off-by: Nicolin Chen nicolinc@nvidia.com

drivers/iommu/iommufd/iommufd_private.h | 8 ++++ drivers/iommu/iommufd/device.c | 59 +++++++++++++++++++++---- 2 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 4a375a8c9216..468717d5e5bc 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -484,6 +484,14 @@ void iopt_remove_access(struct io_pagetable *iopt, struct iommufd_access *access, u32 iopt_access_list_id); void iommufd_access_destroy_object(struct iommufd_object *obj); +/* iommufd_access for internal use */ +struct iommufd_access *iommufd_access_create_internal(struct iommufd_ctx *ictx); +#define iommufd_access_destroy_internal(ictx, access) \

• iommufd_object_destroy_user(ictx, &(access)->obj)

+int iommufd_access_attach_internal(struct iommufd_access *access,

• 		   struct iommufd_ioas *ioas);
  

+#define iommufd_access_detach_internal(access) iommufd_access_detach(access)

struct iommufd_eventq { struct iommufd_object obj; struct iommufd_ctx *ictx; diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index 9293722b9cff..ad33f1e41a24 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -1084,7 +1084,39 @@ void iommufd_access_destroy_object(struct iommufd_object *obj) if (access->ioas) WARN_ON(iommufd_access_change_ioas(access, NULL)); mutex_unlock(&access->ioas_lock);

• iommufd_ctx_put(access->ictx);

• if (access->ops)

• iommufd_ctx_put(access->ictx);
  

Purely relying on access->ops being NULL feels a bit hacky to me..

+}

+static struct iommufd_access *__iommufd_access_create(struct iommufd_ctx *ictx) +{

• struct iommufd_access *access;
• /*

• * There is no uAPI for the access object, but to keep things symmetric
  

• * use the object infrastructure anyhow.
  

• */
  

• access = iommufd_object_alloc(ictx, access, IOMMUFD_OBJ_ACCESS);
• if (IS_ERR(access))

• return access;
  

• /* The calling driver is a user until iommufd_access_destroy() */
• refcount_inc(&access->obj.users);
• mutex_init(&access->ioas_lock);
• return access;

+}

+struct iommufd_access *iommufd_access_create_internal(struct iommufd_ctx *ictx) +{

• struct iommufd_access *access;
• access = __iommufd_access_create(ictx);
• if (IS_ERR(access))

• return access;
  

• access->iova_alignment = PAGE_SIZE;

Maybe setting acces->ictx = NULL; explicitly here would be a clear demarcation between the new API for "internal" v/s the original one. Else, I definitely believe we should have a comment mentioning that access->ictx is NULL for internal.

• iommufd_object_finalize(ictx, &access->obj);
• return access;

} /** @@ -1106,11 +1138,7 @@ iommufd_access_create(struct iommufd_ctx *ictx, { struct iommufd_access *access;

• /*

• * There is no uAPI for the access object, but to keep things symmetric
  

• * use the object infrastructure anyhow.
  

• */
  

• access = iommufd_object_alloc(ictx, access, IOMMUFD_OBJ_ACCESS);

• access = __iommufd_access_create(ictx); if (IS_ERR(access)) return access;

@@ -1122,13 +1150,10 @@ iommufd_access_create(struct iommufd_ctx *ictx, else access->iova_alignment = 1;

• /* The calling driver is a user until iommufd_access_destroy() */
• refcount_inc(&access->obj.users); access->ictx = ictx; iommufd_ctx_get(ictx); iommufd_object_finalize(ictx, &access->obj); *id = access->obj.id;
• mutex_init(&access->ioas_lock); return access;

} EXPORT_SYMBOL_NS_GPL(iommufd_access_create, "IOMMUFD"); @@ -1173,6 +1198,22 @@ int iommufd_access_attach(struct iommufd_access *access, u32 ioas_id) } EXPORT_SYMBOL_NS_GPL(iommufd_access_attach, "IOMMUFD"); +int iommufd_access_attach_internal(struct iommufd_access *access,

• 		   struct iommufd_ioas *ioas)
  

+{

• int rc;
• mutex_lock(&access->ioas_lock);
• if (WARN_ON(access->ioas)) {

• mutex_unlock(&access->ioas_lock);
  

• return -EINVAL;
  

• }
• rc = iommufd_access_change_ioas(access, ioas);
• mutex_unlock(&access->ioas_lock);
• return rc;

+}

int iommufd_access_replace(struct iommufd_access *access, u32 ioas_id) { int rc; -- 2.43.0