Re: [PATCH RFCv2 06/13] iommufd: Make attach_handle generic

On 2025/1/11 11:32, Nicolin Chen wrote:

"attach_handle" was added exclusively for the iommufd_fault_iopf_handler() used by IOPF/PRI use cases, along with the "fault_data". Now, the iommufd version of sw_msi function will resue the attach_handle and fault_data for a non-fault case.

Move the attach_handle part out of the fault.c file to make it generic for all cases. Simplify the remaining fault specific routine to attach/detach.

I guess you can send it separately since both of our series need it. :)

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

drivers/iommu/iommufd/iommufd_private.h | 40 +------- drivers/iommu/iommufd/device.c | 105 +++++++++++++++++++++ drivers/iommu/iommufd/fault.c | 120 +++--------------------- 3 files changed, 122 insertions(+), 143 deletions(-)

diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index b6d706cf2c66..063c0a42f54f 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -472,42 +472,12 @@ void iommufd_fault_destroy(struct iommufd_object *obj); int iommufd_fault_iopf_handler(struct iopf_group *group); int iommufd_fault_domain_attach_dev(struct iommufd_hw_pagetable *hwpt,

• 		    struct iommufd_device *idev);
  

• 		    struct iommufd_device *idev,
  

• 		    bool enable_iopf);
  

  void iommufd_fault_domain_detach_dev(struct iommufd_hw_pagetable *hwpt,

• 		     struct iommufd_device *idev);
  

-int iommufd_fault_domain_replace_dev(struct iommufd_device *idev,

• 		     struct iommufd_hw_pagetable *hwpt,
  

• 		     struct iommufd_hw_pagetable *old);
  

-static inline int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,

• 			     struct iommufd_device *idev)
  

-{

• if (hwpt->fault)

• return iommufd_fault_domain_attach_dev(hwpt, idev);
  

• return iommu_attach_group(hwpt->domain, idev->igroup->group);

-}

-static inline void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,

• 			      struct iommufd_device *idev)
  

-{

• if (hwpt->fault) {

• iommufd_fault_domain_detach_dev(hwpt, idev);
  

• return;
  

• }
• iommu_detach_group(hwpt->domain, idev->igroup->group);

-}

-static inline int iommufd_hwpt_replace_device(struct iommufd_device *idev,

• 			      struct iommufd_hw_pagetable *hwpt,
  

• 			      struct iommufd_hw_pagetable *old)
  

-{

• if (old->fault || hwpt->fault)

• return iommufd_fault_domain_replace_dev(idev, hwpt, old);
  

• return iommu_group_replace_domain(idev->igroup->group, hwpt->domain);

-}

• 		     struct iommufd_device *idev,
  

• 		     struct iommufd_attach_handle *handle,
  

• 		     bool disable_iopf);
  

static inline struct iommufd_viommu * iommufd_get_viommu(struct iommufd_ucmd *ucmd, u32 id) diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index dfd0898fb6c1..38b31b652147 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -352,6 +352,111 @@ iommufd_device_attach_reserved_iova(struct iommufd_device *idev, return 0; } +/* The device attach/detach/replace helpers for attach_handle */

+static int iommufd_hwpt_attach_device(struct iommufd_hw_pagetable *hwpt,

• 		      struct iommufd_device *idev)
  

+{

• struct iommufd_attach_handle *handle;
• int rc;
• if (hwpt->fault) {

• rc = iommufd_fault_domain_attach_dev(hwpt, idev, true);
  

• if (rc)
  

• 	return rc;
  

• }
• handle = kzalloc(sizeof(*handle), GFP_KERNEL);
• if (!handle) {

• rc = -ENOMEM;
  

• goto out_fault_detach;
  

• }
• handle->idev = idev;
• rc = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,

• 		       &handle->handle);
  

• if (rc)

• goto out_free_handle;
  

• return 0;

+out_free_handle:

• kfree(handle);
• handle = NULL;

+out_fault_detach:

• if (hwpt->fault)

• iommufd_fault_domain_detach_dev(hwpt, idev, handle, true);
  

• return rc;

+}

+static struct iommufd_attach_handle * +iommufd_device_get_attach_handle(struct iommufd_device *idev) +{

• struct iommu_attach_handle *handle;
• handle =

• iommu_attach_handle_get(idev->igroup->group, IOMMU_NO_PASID, 0);
  

• if (IS_ERR(handle))

• return NULL;
  

• return to_iommufd_handle(handle);

+}

+static void iommufd_hwpt_detach_device(struct iommufd_hw_pagetable *hwpt,

• 		       struct iommufd_device *idev)
  

+{

• struct iommufd_attach_handle *handle;
• handle = iommufd_device_get_attach_handle(idev);
• iommu_detach_group_handle(hwpt->domain, idev->igroup->group);
• if (hwpt->fault)

• iommufd_fault_domain_detach_dev(hwpt, idev, handle, true);
  

• kfree(handle);

+}

+static int iommufd_hwpt_replace_device(struct iommufd_device *idev,

• 		       struct iommufd_hw_pagetable *hwpt,
  

• 		       struct iommufd_hw_pagetable *old)
  

+{

• struct iommufd_attach_handle *old_handle =

• iommufd_device_get_attach_handle(idev);
  

• struct iommufd_attach_handle *handle;
• int rc;
• if (hwpt->fault) {

• rc = iommufd_fault_domain_attach_dev(hwpt, idev, !old->fault);
  

• if (rc)
  

• 	return rc;
  

• }
• handle = kzalloc(sizeof(*handle), GFP_KERNEL);
• if (!handle) {

• rc = -ENOMEM;
  

• goto out_fault_detach;
  

• }
• handle->idev = idev;
• rc = iommu_replace_group_handle(idev->igroup->group, hwpt->domain,

• 			&handle->handle);
  

• if (rc)

• goto out_free_handle;
  

• if (old->fault)

• iommufd_fault_domain_detach_dev(old, idev, old_handle,
  

• 				!hwpt->fault);
  

• kfree(old_handle);
• return 0;

+out_free_handle:

• kfree(handle);
• handle = NULL;

+out_fault_detach:

• if (hwpt->fault)

• iommufd_fault_domain_detach_dev(hwpt, idev, handle,
  

• 				!old->fault);
  

• return rc;

+}

• int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt, struct iommufd_device *idev) {

diff --git a/drivers/iommu/iommufd/fault.c b/drivers/iommu/iommufd/fault.c index 06aa83a75e94..1d9bd3024b57 100644 --- a/drivers/iommu/iommufd/fault.c +++ b/drivers/iommu/iommufd/fault.c @@ -60,42 +60,17 @@ static void iommufd_fault_iopf_disable(struct iommufd_device *idev) mutex_unlock(&idev->iopf_lock); } -static int __fault_domain_attach_dev(struct iommufd_hw_pagetable *hwpt,

• 		     struct iommufd_device *idev)
  

-{

• struct iommufd_attach_handle *handle;
• int ret;
• handle = kzalloc(sizeof(*handle), GFP_KERNEL);
• if (!handle)

• return -ENOMEM;
  

• handle->idev = idev;
• ret = iommu_attach_group_handle(hwpt->domain, idev->igroup->group,

• 			&handle->handle);
  

• if (ret)

• kfree(handle);
  

• return ret;

-}

• int iommufd_fault_domain_attach_dev(struct iommufd_hw_pagetable *hwpt,

• 		    struct iommufd_device *idev)
  

• 		    struct iommufd_device *idev,
  

• 		    bool enable_iopf)
  

  {

• int ret;

• int rc = 0;

if (!hwpt->fault) return -EINVAL;

• ret = iommufd_fault_iopf_enable(idev);
• if (ret)

• return ret;
  

• ret = __fault_domain_attach_dev(hwpt, idev);
• if (ret)

• iommufd_fault_iopf_disable(idev);
  

• return ret;

• if (enable_iopf)

• rc = iommufd_fault_iopf_enable(idev);
  

• return rc; }

static void iommufd_auto_response_faults(struct iommufd_hw_pagetable *hwpt, @@ -127,86 +102,15 @@ static void iommufd_auto_response_faults(struct iommufd_hw_pagetable *hwpt, mutex_unlock(&fault->mutex); } -static struct iommufd_attach_handle * -iommufd_device_get_attach_handle(struct iommufd_device *idev) -{

• struct iommu_attach_handle *handle;
• handle = iommu_attach_handle_get(idev->igroup->group, IOMMU_NO_PASID, 0);
• if (IS_ERR(handle))

• return NULL;
  

• return to_iommufd_handle(handle);

-}

• void iommufd_fault_domain_detach_dev(struct iommufd_hw_pagetable *hwpt,

• 		     struct iommufd_device *idev)
  

• 		     struct iommufd_device *idev,
  

• 		     struct iommufd_attach_handle *handle,
  

• 		     bool disable_iopf)
  

  {

• struct iommufd_attach_handle *handle;
• handle = iommufd_device_get_attach_handle(idev);
• iommu_detach_group_handle(hwpt->domain, idev->igroup->group);
• iommufd_auto_response_faults(hwpt, handle);
• iommufd_fault_iopf_disable(idev);
• kfree(handle);

-}

-static int __fault_domain_replace_dev(struct iommufd_device *idev,

• 		      struct iommufd_hw_pagetable *hwpt,
  

• 		      struct iommufd_hw_pagetable *old)
  

-{

• struct iommufd_attach_handle *handle, *curr = NULL;
• int ret;
• if (old->fault)

• curr = iommufd_device_get_attach_handle(idev);
  

• if (hwpt->fault) {

• handle = kzalloc(sizeof(*handle), GFP_KERNEL);
  

• if (!handle)
  

• 	return -ENOMEM;
  

• handle->idev = idev;
  

• ret = iommu_replace_group_handle(idev->igroup->group,
  

• 				 hwpt->domain, &handle->handle);
  

• } else {

• ret = iommu_replace_group_handle(idev->igroup->group,
  

• 				 hwpt->domain, NULL);
  

• }
• if (!ret && curr) {

• iommufd_auto_response_faults(old, curr);
  

• kfree(curr);
  

• }
• return ret;

-}

-int iommufd_fault_domain_replace_dev(struct iommufd_device *idev,

• 		     struct iommufd_hw_pagetable *hwpt,
  

• 		     struct iommufd_hw_pagetable *old)
  

-{

• bool iopf_off = !hwpt->fault && old->fault;
• bool iopf_on = hwpt->fault && !old->fault;
• int ret;
• if (iopf_on) {

• ret = iommufd_fault_iopf_enable(idev);
  

• if (ret)
  

• 	return ret;
  

• }
• ret = __fault_domain_replace_dev(idev, hwpt, old);
• if (ret) {

• if (iopf_on)
  

• 	iommufd_fault_iopf_disable(idev);
  

• return ret;
  

• }
• if (iopf_off)

• if (handle)

• iommufd_auto_response_faults(hwpt, handle);
  

no need to check handle. After this patch, both the non-fault and the fault path will allocate handle. It cannot be used to isolate fault and non-fault path. Also, the callers of iommufd_fault_domain_detach_dev() will check hwpt->fault before calling it. So just call iommufd_auto_response_faults().

• if (disable_iopf) iommufd_fault_iopf_disable(idev);

• return 0; }

void iommufd_fault_destroy(struct iommufd_object *obj)