[PATCH 3/3] iommufd/device: Change iommufd_hw_pagetable_has_group to device centric
From: Nicolin Chen
Date: Fri Jan 27 2023 - 21:30:16 EST
The iommufd_hw_pagetable_has_group() is a little hack to check whether
a hw_pagetable has an idev->group attached. This isn't device-centric
firstly, but also requires the hw_pagetable to maintain a device list
with a devices_lock, which gets overcomplicated among the routines for
different use cases, upcoming nested hwpts especially.
Since we can compare the iommu_group->domain and the hwpt->domain to
serve the same purpose while being device centric, change it and drop
the device list with the devices_lock accordingly.
Note that, in the detach routine, it previously checked !has_group as
there was a list_del on the device list. But now, the device is still
being attached to the hwpt, so the if logic gets inverted.
Signed-off-by: Nicolin Chen <nicolinc@xxxxxxxxxx>
---
drivers/iommu/iommufd/device.c | 40 ++++++++-----------------
drivers/iommu/iommufd/hw_pagetable.c | 5 ----
drivers/iommu/iommufd/iommufd_private.h | 2 --
3 files changed, 12 insertions(+), 35 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 0248073e3169..5a5dfc0c4ca0 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -24,8 +24,6 @@ struct iommufd_device {
struct iommufd_object obj;
struct iommufd_ctx *ictx;
struct iommufd_hw_pagetable *hwpt;
- /* Head at iommufd_hw_pagetable::devices */
- struct list_head devices_item;
/* always the physical device */
struct device *dev;
struct iommu_group *group;
@@ -181,15 +179,15 @@ static int iommufd_device_setup_msi(struct iommufd_device *idev,
return 0;
}
-static bool iommufd_hw_pagetable_has_group(struct iommufd_hw_pagetable *hwpt,
- struct iommu_group *group)
+static bool iommufd_hw_pagetable_has_device(struct iommufd_hw_pagetable *hwpt,
+ struct device *dev)
{
- struct iommufd_device *cur_dev;
-
- list_for_each_entry(cur_dev, &hwpt->devices, devices_item)
- if (cur_dev->group == group)
- return true;
- return false;
+ /*
+ * iommu_get_domain_for_dev() returns an iommu_group->domain ptr, if it
+ * is the same domain as the hwpt->domain, it means that this hwpt has
+ * the iommu_group/device.
+ */
+ return hwpt->domain == iommu_get_domain_for_dev(dev);
}
static int iommufd_device_do_attach(struct iommufd_device *idev,
@@ -200,8 +198,6 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
lockdep_assert_held(&hwpt->ioas->mutex);
- mutex_lock(&hwpt->devices_lock);
-
/*
* Try to upgrade the domain we have, it is an iommu driver bug to
* report IOMMU_CAP_ENFORCE_CACHE_COHERENCY but fail
@@ -215,25 +211,20 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
hwpt->domain);
if (!hwpt->enforce_cache_coherency) {
WARN_ON(refcount_read(hwpt->devices_users) == 1);
- rc = -EINVAL;
- goto out_unlock;
+ return -EINVAL;
}
}
rc = iopt_table_enforce_group_resv_regions(&hwpt->ioas->iopt, idev->dev,
idev->group, &sw_msi_start);
if (rc)
- goto out_unlock;
+ return rc;
rc = iommufd_device_setup_msi(idev, hwpt, sw_msi_start);
if (rc)
goto out_iova;
- /*
- * FIXME: Hack around missing a device-centric iommu api, only attach to
- * the group once for the first device that is in the group.
- */
- if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
+ if (!iommufd_hw_pagetable_has_device(hwpt, idev->dev)) {
rc = iommu_attach_group(hwpt->domain, idev->group);
if (rc)
goto out_iova;
@@ -250,16 +241,12 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
idev->hwpt = hwpt;
refcount_inc(&hwpt->obj.users);
refcount_inc(hwpt->devices_users);
- list_add(&idev->devices_item, &hwpt->devices);
- mutex_unlock(&hwpt->devices_lock);
return 0;
out_detach:
iommu_detach_group(hwpt->domain, idev->group);
out_iova:
iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev);
-out_unlock:
- mutex_unlock(&hwpt->devices_lock);
return rc;
}
@@ -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);
}
iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev);
- mutex_unlock(&hwpt->devices_lock);
mutex_unlock(&hwpt->ioas->mutex);
if (hwpt->auto_domain)
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 910e759ffeac..868a126ff37d 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -11,11 +11,8 @@ void iommufd_hw_pagetable_destroy(struct iommufd_object *obj)
struct iommufd_hw_pagetable *hwpt =
container_of(obj, struct iommufd_hw_pagetable, obj);
- WARN_ON(!list_empty(&hwpt->devices));
-
iommu_domain_free(hwpt->domain);
refcount_dec(&hwpt->ioas->obj.users);
- mutex_destroy(&hwpt->devices_lock);
WARN_ON(!refcount_dec_if_one(hwpt->devices_users));
kfree(hwpt->devices_users);
}
@@ -45,9 +42,7 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
goto out_abort;
}
- INIT_LIST_HEAD(&hwpt->devices);
INIT_LIST_HEAD(&hwpt->hwpt_item);
- mutex_init(&hwpt->devices_lock);
hwpt->devices_users = kzalloc(sizeof(*hwpt->devices_users), GFP_KERNEL);
if (!hwpt->devices_users) {
rc = -ENOMEM;
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index f128d77fb076..1c8e59b37f46 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -246,9 +246,7 @@ struct iommufd_hw_pagetable {
bool msi_cookie : 1;
/* Head at iommufd_ioas::hwpt_list */
struct list_head hwpt_item;
- struct mutex devices_lock;
refcount_t *devices_users;
- struct list_head devices;
};
struct iommufd_hw_pagetable *
--
2.39.1