[v5,5/6] iommu/arm-smmu-v3: Free pasid domains on iommu release
Commit Message
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
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?
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
@@ -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;
@@ -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 */