diff mbox series

[v2,3/7] iommufd: Move ioas related HWPT destruction into iommufd_hw_pagetable_destroy()

Message ID 3-v2-406f7ac07936+6a-iommufd_hwpt_jgg@nvidia.com (mailing list archive)
State New
Headers show
Series Revise the hwpt lifetime model | expand

Commit Message

Jason Gunthorpe Feb. 22, 2023, 9:02 p.m. UTC
A HWPT is permanently associated with an IOAS when it is created, remove
the strange situation where a refcount != 0 HWPT can have been
disconnected from the IOAS by putting all the IOAS related destruction in
the object destroy function.

Initializing a HWPT is two stages, we have to allocate it, attach it to a
device and then populate the domain. Once the domain is populated it is
fully linked to the IOAS.

Arrange things so that all the error unwinds flow through the
iommufd_hw_pagetable_destroy() and allow it to handle all cases.

Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/iommufd/device.c       | 20 +++++++-------------
 drivers/iommu/iommufd/hw_pagetable.c | 27 +++++++++++++++++++--------
 2 files changed, 26 insertions(+), 21 deletions(-)

Comments

Tian, Kevin Feb. 24, 2023, 6:25 a.m. UTC | #1
> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Thursday, February 23, 2023 5:03 AM
> @@ -753,6 +744,10 @@ iommufd_device_selftest_attach(struct
> iommufd_ctx *ictx,
>  	if (rc)
>  		goto out_hwpt;
> 
> +	mutex_lock(&ioas->mutex);
> +	list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
> +	mutex_unlock(&ioas->mutex);
> +
>  	refcount_inc(&hwpt->obj.users);
>  	iommufd_object_finalize(ictx, &hwpt->obj);
>  	return hwpt;
> @@ -765,7 +760,6 @@ iommufd_device_selftest_attach(struct iommufd_ctx
> *ictx,
>  void iommufd_device_selftest_detach(struct iommufd_ctx *ictx,
>  				    struct iommufd_hw_pagetable *hwpt)
>  {
> -	iopt_table_remove_domain(&hwpt->ioas->iopt, hwpt->domain);
>  	refcount_dec(&hwpt->obj.users);
>  }
>  #endif

It will read clearer if first having a patch to add list_add_tail() to selftest
attach and list_del() in selftest detach and then having this patch to
move list_del()+iopt_table_remove_domain() to HWPT destruction.
diff mbox series

Patch

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 0a80ff7b2e0d79..6787a0d8d6e9f0 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -243,6 +243,7 @@  static int iommufd_device_do_attach(struct iommufd_device *idev,
 						   hwpt->domain);
 			if (rc)
 				goto out_detach;
+			list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
 		}
 	}
 
@@ -307,7 +308,6 @@  static int iommufd_device_auto_get_domain(struct iommufd_device *idev,
 	rc = iommufd_device_do_attach(idev, hwpt);
 	if (rc)
 		goto out_abort;
-	list_add_tail(&hwpt->hwpt_item, &ioas->hwpt_list);
 
 	mutex_unlock(&ioas->mutex);
 	iommufd_object_finalize(idev->ictx, &hwpt->obj);
@@ -386,28 +386,19 @@  void iommufd_device_detach(struct iommufd_device *idev)
 {
 	struct iommufd_hw_pagetable *hwpt = idev->hwpt;
 
-	mutex_lock(&hwpt->ioas->mutex);
 	mutex_lock(&hwpt->devices_lock);
 	list_del(&idev->devices_item);
-	if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
-		if (list_empty(&hwpt->devices)) {
-			iopt_table_remove_domain(&hwpt->ioas->iopt,
-						 hwpt->domain);
-			list_del(&hwpt->hwpt_item);
-		}
+	if (!iommufd_hw_pagetable_has_group(hwpt, idev->group))
 		iommu_detach_group(hwpt->domain, idev->group);
-	}
 	iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev);
+	idev->hwpt = NULL;
 	mutex_unlock(&hwpt->devices_lock);
-	mutex_unlock(&hwpt->ioas->mutex);
 
 	if (hwpt->auto_domain)
 		iommufd_object_destroy_user(idev->ictx, &hwpt->obj);
 	else
 		refcount_dec(&hwpt->obj.users);
 
-	idev->hwpt = NULL;
-
 	refcount_dec(&idev->obj.users);
 }
 EXPORT_SYMBOL_NS_GPL(iommufd_device_detach, IOMMUFD);
@@ -753,6 +744,10 @@  iommufd_device_selftest_attach(struct iommufd_ctx *ictx,
 	if (rc)
 		goto out_hwpt;
 
+	mutex_lock(&ioas->mutex);
+	list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
+	mutex_unlock(&ioas->mutex);
+
 	refcount_inc(&hwpt->obj.users);
 	iommufd_object_finalize(ictx, &hwpt->obj);
 	return hwpt;
@@ -765,7 +760,6 @@  iommufd_device_selftest_attach(struct iommufd_ctx *ictx,
 void iommufd_device_selftest_detach(struct iommufd_ctx *ictx,
 				    struct iommufd_hw_pagetable *hwpt)
 {
-	iopt_table_remove_domain(&hwpt->ioas->iopt, hwpt->domain);
 	refcount_dec(&hwpt->obj.users);
 }
 #endif
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 43d473989a0667..10db1359c067c1 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -13,7 +13,17 @@  void iommufd_hw_pagetable_destroy(struct iommufd_object *obj)
 
 	WARN_ON(!list_empty(&hwpt->devices));
 
-	iommu_domain_free(hwpt->domain);
+	if (!list_empty(&hwpt->hwpt_item)) {
+		mutex_lock(&hwpt->ioas->mutex);
+		list_del(&hwpt->hwpt_item);
+		mutex_unlock(&hwpt->ioas->mutex);
+
+		iopt_table_remove_domain(&hwpt->ioas->iopt, hwpt->domain);
+	}
+
+	if (hwpt->domain)
+		iommu_domain_free(hwpt->domain);
+
 	refcount_dec(&hwpt->ioas->obj.users);
 	mutex_destroy(&hwpt->devices_lock);
 }
@@ -37,21 +47,22 @@  iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
 	if (IS_ERR(hwpt))
 		return hwpt;
 
-	hwpt->domain = iommu_domain_alloc(dev->bus);
-	if (!hwpt->domain) {
-		rc = -ENOMEM;
-		goto out_abort;
-	}
-
 	INIT_LIST_HEAD(&hwpt->devices);
 	INIT_LIST_HEAD(&hwpt->hwpt_item);
 	mutex_init(&hwpt->devices_lock);
 	/* Pairs with iommufd_hw_pagetable_destroy() */
 	refcount_inc(&ioas->obj.users);
 	hwpt->ioas = ioas;
+
+	hwpt->domain = iommu_domain_alloc(dev->bus);
+	if (!hwpt->domain) {
+		rc = -ENOMEM;
+		goto out_abort;
+	}
+
 	return hwpt;
 
 out_abort:
-	iommufd_object_abort(ictx, &hwpt->obj);
+	iommufd_object_abort_and_destroy(ictx, &hwpt->obj);
 	return ERR_PTR(rc);
 }