diff mbox series

[v5,5/6] iommu/arm-smmu-v3: Free pasid domains on iommu release

Message ID 20230803181225.v5.5.I6f3fb0734ef5ef746ae7c9b27f632f506197eb30@changeid (mailing list archive)
State New, archived
Headers show
Series Add PASID support to SMMUv3 unmanaged domains | expand

Commit Message

Michael Shavit Aug. 3, 2023, 10:12 a.m. UTC
The iommu core doesn't guarantee that pasid domains will be detached
before the device is released.

Track the list of domains that a master is attached to with PASID, so
that they can be freed when the iommu is released.

Signed-off-by: Michael Shavit <mshavit@google.com>
---

Changes in v5:
- New commit: Free attached pasid domains on release_device() call

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 22 +++++++++++++++------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 10 +++++++++-
 2 files changed, 25 insertions(+), 7 deletions(-)

Comments

Michael Shavit Aug. 3, 2023, 10:17 a.m. UTC | #1
On Thu, Aug 3, 2023 at 6:14 PM Michael Shavit <mshavit@google.com> wrote:
>
> The iommu core doesn't guarantee that pasid domains will be detached
> before the device is released.

I'm not certain whether this is possible or not. Is this change really
necessary?
Jason Gunthorpe Aug. 3, 2023, 3:20 p.m. UTC | #2
On Thu, Aug 03, 2023 at 06:17:15PM +0800, Michael Shavit wrote:
> On Thu, Aug 3, 2023 at 6:14 PM Michael Shavit <mshavit@google.com> wrote:
> >
> > The iommu core doesn't guarantee that pasid domains will be detached
> > before the device is released.
> 
> I'm not certain whether this is possible or not. Is this change really
> necessary?

I think we should rely on the core code to detach all PASIDs prior to
calling release, drivers should not do this. I suppose that means we
have a missing bit in the core code.

FWIW, I'd like to get to a point where the core code can also
automatically attach an identity domain so that the driver's release
functions don't have to open code that either...

Thus I wouldn't do this patch..

Jason
diff mbox series

Patch

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 7b296458dafec..5fd6c4d4f0ae4 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2587,6 +2587,9 @@  static int arm_smmu_set_dev_pasid(struct iommu_domain *domain,
 	mutex_unlock(&arm_smmu_asid_lock);
 
 	master->nr_attached_pasid_domains += 1;
+	list_add(&attached_domain->list_in_master,
+		 &master->attached_domains);
+
 	return 0;
 }
 
@@ -2786,6 +2789,7 @@  static struct iommu_device *arm_smmu_probe_device(struct device *dev)
 	master->dev = dev;
 	master->smmu = smmu;
 	INIT_LIST_HEAD(&master->bonds);
+	INIT_LIST_HEAD(&master->attached_domains);
 	dev_iommu_priv_set(dev, master);
 
 	ret = arm_smmu_insert_master(smmu, master);
@@ -2825,16 +2829,21 @@  static struct iommu_device *arm_smmu_probe_device(struct device *dev)
 static void arm_smmu_release_device(struct device *dev)
 {
 	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
+	struct arm_smmu_attached_domain *attached_domain;
+	struct arm_smmu_domain *smmu_domain;
+	unsigned long flags;
 
 	if (WARN_ON(arm_smmu_master_sva_enabled(master)))
 		iopf_queue_remove_device(master->smmu->evtq.iopf, dev);
 	if (WARN_ON(master->nr_attached_pasid_domains != 0)) {
-		/*
-		 * TODO: Do we need to handle this case?
-		 * This requires a mechanism to obtain all the pasid domains
-		 * that this master is attached to so that we can clean up the
-		 * domain's attached_domain list.
-		 */
+		list_for_each_entry(attached_domain, &master->attached_domains, list_in_master) {
+			smmu_domain = attached_domain->domain;
+			spin_lock_irqsave(&smmu_domain->attached_ssids_lock, flags);
+			list_del(&attached_domain->list);
+			list_del(&attached_domain->list_in_master);
+			kfree(&attached_domain->list_in_master);
+			spin_unlock_irqrestore(&smmu_domain->attached_ssids_lock, flags);
+		}
 	}
 
 	arm_smmu_detach_dev(master);
@@ -2995,6 +3004,7 @@  static void arm_smmu_remove_dev_pasid(struct device *dev, ioasid_t pasid)
 		    attached_domain->ssid != pasid)
 			continue;
 		list_del(&attached_domain->list);
+		list_del(&attached_domain->list_in_master);
 		master->nr_attached_pasid_domains -= 1;
 		kfree(attached_domain);
 		break;
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 433f58bd99dd2..efa428629f4d9 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -689,9 +689,15 @@  struct arm_smmu_stream {
 	struct rb_node			node;
 };
 
-/* List of {masters, ssid} that a domain is attached to */
+/*
+ * List of {masters, ssid} that a domain is attached to, and conversely of
+ * domains that a master is attached to.
+ */
 struct arm_smmu_attached_domain {
+	/* List node arm_smmu_domain*/
 	struct list_head	list;
+	/* List node in arm_smmu_master*/
+	struct list_head	list_in_master;
 	struct arm_smmu_domain  *domain;
 	struct arm_smmu_master  *master;
 	int			ssid;
@@ -714,6 +720,8 @@  struct arm_smmu_master {
 	struct list_head		bonds;
 	unsigned int			ssid_bits;
 	unsigned int			nr_attached_pasid_domains;
+	/* Locked by the iommu core using the group mutex */
+	struct list_head		attached_domains;
 };
 
 /* SMMU private data for an IOMMU domain */