On Fri, May 16, 2025 at 10:28:45AM -0300, Jason Gunthorpe wrote:
On Thu, May 15, 2025 at 07:05:45PM -0700, Nicolin Chen wrote:
Otherwise, they would be something like: // in my_viommu_alloc() my_viommu = iommufd_viommu_alloc(ucmd, ...); iommufd_viommu_alloc_mmap(ucmd->ictx, my_viommu, ...); iommufd_viommu_destroy_mmap(ucmd->ictx, my_viommu, ...); // in my_viommu_destory() iommufd_viommu_destroy_mmap(my_viommu->ictx, my_viommu ...);
Yes, this is how the other objects work..
That being said, I ended up doing something more in this patch for the ucmd rework and Kevin's comments:
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 98fe5a49c9606..ddd144031af5d 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -136,14 +136,6 @@ int iopt_pin_pages(struct io_pagetable *iopt, unsigned long iova, void iopt_unpin_pages(struct io_pagetable *iopt, unsigned long iova, unsigned long length); -struct iommufd_ucmd {
- struct iommufd_ctx *ictx;
- void __user *ubuffer;
- u32 user_size;
- void *cmd;
- struct iommufd_object *alloced_obj;
-};
You don't need to move this unless you are using inlines. Just use a forward declaration.
Since we forward ucmd now, ictx is in the ucmd so we need this structure for:
- if (!IS_ERR(ret)) \ + if (!IS_ERR(ret)) { \ ret->member.ops = viommu_ops; \ + ret->member.ictx = ucmd->ictx; \ + } \
Or, we could have a smaller:
#define ucmd_to_ictx(ucmd) (struct iommufd_ctx *)(ucmd)
and add a line of comments in struct iommufd_ucmd:
struct iommufd_ucmd { - struct iommufd_ctx *ictx; + struct iommufd_ctx *ictx; /* Do not move; must be the first member */ void __user *ubuffer; u32 user_size; void *cmd; struct iommufd_object *alloced_obj; };
Thanks Nicolin