Messages in this thread Patch in this message | | | Date | Mon, 30 Jan 2023 02:22:17 -0800 | From | Nicolin Chen <> | Subject | Re: [PATCH v2 3/3] iommufd/device: Change iommufd_hw_pagetable_has_group to device centric |
| |
On Sun, Jan 29, 2023 at 02:38:55AM -0800, Nicolin Chen wrote:
> > > @@ -385,10 +372,8 @@ 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); > > > refcount_dec(hwpt->devices_users); > > > - list_del(&idev->devices_item); > > > - if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) { > > > + if (iommufd_hw_pagetable_has_device(hwpt, idev->dev)) { > > > if (refcount_read(hwpt->devices_users) == 1) { > > > iopt_table_remove_domain(&hwpt->ioas->iopt, > > > hwpt->domain); > > > @@ -397,7 +382,6 @@ void iommufd_device_detach(struct iommufd_device > > > *idev) > > > iommu_detach_group(hwpt->domain, idev->group); > > > } > > > > emmm how do we track last device detach in a group? Here the first > > device detach already leads to group detach... > > Oh no. That's a bug. Thanks for catching it. > > We need an additional refcount somewhere to track the number of > attached devices in the iommu_group.
Wondering if we can let iommu_attach/detach_device handle this:
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index d0d7c2177ad6..b38f71e92e2a 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -57,6 +57,7 @@ struct iommu_group { struct iommu_domain *domain; struct list_head entry; unsigned int owner_cnt; + unsigned int attached_cnt; void *owner; }; @@ -64,6 +65,7 @@ struct group_device { struct list_head list; struct device *dev; char *name; + bool attached; }; struct iommu_group_attribute { @@ -2035,6 +2037,7 @@ static int __iommu_attach_device(struct iommu_domain *domain, */ int iommu_attach_device(struct iommu_domain *domain, struct device *dev) { + struct group_device *grp_dev; struct iommu_group *group; int ret; @@ -2042,16 +2045,22 @@ int iommu_attach_device(struct iommu_domain *domain, struct device *dev) if (!group) return -ENODEV; - /* - * Lock the group to make sure the device-count doesn't - * change while we are attaching - */ mutex_lock(&group->mutex); - ret = -EINVAL; - if (iommu_group_device_count(group) != 1) + + list_for_each_entry(grp_dev, &group->devices, list) + if (grp_dev->dev == dev) + break; + if (grp_dev->attached) goto out_unlock; - ret = __iommu_attach_group(domain, group); + /* Attach the group when attaching the first device in the group */ + if (group->attached_cnt == 0) { + ret = __iommu_attach_group(domain, group); + if (ret) + goto out_unlock; + } + grp_dev->attached = true; + group->attached_cnt++; out_unlock: mutex_unlock(&group->mutex); @@ -2071,6 +2080,7 @@ int iommu_deferred_attach(struct device *dev, struct iommu_domain *domain) void iommu_detach_device(struct iommu_domain *domain, struct device *dev) { + struct group_device *grp_dev; struct iommu_group *group; group = iommu_group_get(dev); @@ -2078,10 +2088,20 @@ void iommu_detach_device(struct iommu_domain *domain, struct device *dev) return; mutex_lock(&group->mutex); - if (WARN_ON(domain != group->domain) || - WARN_ON(iommu_group_device_count(group) != 1)) + if (WARN_ON(domain != group->domain)) goto out_unlock; - __iommu_group_set_core_domain(group); + + list_for_each_entry(grp_dev, &group->devices, list) + if (grp_dev->dev == dev) + break; + if (!grp_dev->attached) + goto out_unlock; + + grp_dev->attached = false; + group->attached_cnt--; + /* Detach the group when detaching the last device in the group */ + if (group->attached_cnt == 0) + __iommu_group_set_core_domain(group); out_unlock: mutex_unlock(&group->mutex);
| |