From: Jason Gunthorpe <jgg@nvidia.com>
Date: Mon, 17 Jul 2023 15:11:59 -0300
Subject: iommufd: Replace the hwpt->devices list with iommufd_group
Git-commit: 91a2e17e243f70e0aec29facf44946439fce9195
Patch-mainline: v6.6-rc1
References: jsc#PED-7779 jsc#PED-7780
The devices list was used as a simple way to avoid having per-group
information. Now that this seems to be unavoidable, just commit to
per-group information fully and remove the devices list from the HWPT.
The iommufd_group stores the currently assigned HWPT for the entire group
and we can manage the per-device attach/detach with a list in the
iommufd_group.
For destruction the flow is organized to make the following patches
easier, the actual call to iommufd_object_destroy_user() is done at the
top of the call chain without holding any locks. The HWPT to be destroyed
is returned out from the locked region to make this possible. Later
patches create locking that requires this.
Link: https://lore.kernel.org/r/3-v8-6659224517ea+532-iommufd_alloc_jgg@nvidia.com
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Tested-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Acked-by: Joerg Roedel <jroedel@suse.de>
---
drivers/iommu/iommufd/device.c | 100 ++++++++++++++------------------
drivers/iommu/iommufd/hw_pagetable.c | 22 -------
drivers/iommu/iommufd/iommufd_private.h | 13 +---
3 files changed, 54 insertions(+), 81 deletions(-)
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -20,9 +20,12 @@ static void iommufd_group_release(struct
struct iommufd_group *igroup =
container_of(kref, struct iommufd_group, ref);
+ WARN_ON(igroup->hwpt || !list_empty(&igroup->device_list));
+
xa_cmpxchg(&igroup->ictx->groups, iommu_group_id(igroup->group), igroup,
NULL, GFP_KERNEL);
iommu_group_put(igroup->group);
+ mutex_destroy(&igroup->lock);
kfree(igroup);
}
@@ -83,6 +86,8 @@ static struct iommufd_group *iommufd_get
}
kref_init(&new_igroup->ref);
+ mutex_init(&new_igroup->lock);
+ INIT_LIST_HEAD(&new_igroup->device_list);
/* group reference moves into new_igroup */
new_igroup->group = group;
@@ -317,29 +322,18 @@ static int iommufd_device_setup_msi(stru
return 0;
}
-static bool iommufd_hw_pagetable_has_group(struct iommufd_hw_pagetable *hwpt,
- struct iommufd_group *igroup)
-{
- struct iommufd_device *cur_dev;
-
- lockdep_assert_held(&hwpt->devices_lock);
-
- list_for_each_entry(cur_dev, &hwpt->devices, devices_item)
- if (cur_dev->igroup->group == igroup->group)
- return true;
- return false;
-}
-
int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev)
{
phys_addr_t sw_msi_start = PHYS_ADDR_MAX;
int rc;
- lockdep_assert_held(&hwpt->devices_lock);
+ mutex_lock(&idev->igroup->lock);
- if (WARN_ON(idev->hwpt))
- return -EINVAL;
+ if (idev->igroup->hwpt != NULL && idev->igroup->hwpt != hwpt) {
+ rc = -EINVAL;
+ goto err_unlock;
+ }
/*
* Try to upgrade the domain we have, it is an iommu driver bug to
@@ -353,8 +347,9 @@ int iommufd_hw_pagetable_attach(struct i
hwpt->domain->ops->enforce_cache_coherency(
hwpt->domain);
if (!hwpt->enforce_cache_coherency) {
- WARN_ON(list_empty(&hwpt->devices));
- return -EINVAL;
+ WARN_ON(list_empty(&idev->igroup->device_list));
+ rc = -EINVAL;
+ goto err_unlock;
}
}
@@ -362,51 +357,52 @@ int iommufd_hw_pagetable_attach(struct i
idev->igroup->group,
&sw_msi_start);
if (rc)
- return rc;
+ goto err_unlock;
rc = iommufd_device_setup_msi(idev, hwpt, sw_msi_start);
if (rc)
goto err_unresv;
/*
- * FIXME: Hack around missing a device-centric iommu api, only attach to
- * the group once for the first device that is in the group.
+ * Only attach to the group once for the first device that is in the
+ * group. All the other devices will follow this attachment. The user
+ * should attach every device individually to the hwpt as the per-device
+ * reserved regions are only updated during individual device
+ * attachment.
*/
- if (!iommufd_hw_pagetable_has_group(hwpt, idev->igroup)) {
+ if (list_empty(&idev->igroup->device_list)) {
rc = iommu_attach_group(hwpt->domain, idev->igroup->group);
if (rc)
goto err_unresv;
+ idev->igroup->hwpt = hwpt;
}
+ refcount_inc(&hwpt->obj.users);
+ list_add_tail(&idev->group_item, &idev->igroup->device_list);
+ mutex_unlock(&idev->igroup->lock);
return 0;
err_unresv:
iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev);
+err_unlock:
+ mutex_unlock(&idev->igroup->lock);
return rc;
}
-void iommufd_hw_pagetable_detach(struct iommufd_hw_pagetable *hwpt,
- struct iommufd_device *idev)
+struct iommufd_hw_pagetable *
+iommufd_hw_pagetable_detach(struct iommufd_device *idev)
{
- if (!iommufd_hw_pagetable_has_group(hwpt, idev->igroup))
+ struct iommufd_hw_pagetable *hwpt = idev->igroup->hwpt;
+
+ mutex_lock(&idev->igroup->lock);
+ list_del(&idev->group_item);
+ if (list_empty(&idev->igroup->device_list)) {
iommu_detach_group(hwpt->domain, idev->igroup->group);
+ idev->igroup->hwpt = NULL;
+ }
iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev);
-}
-
-static int iommufd_device_do_attach(struct iommufd_device *idev,
- struct iommufd_hw_pagetable *hwpt)
-{
- int rc;
-
- mutex_lock(&hwpt->devices_lock);
- rc = iommufd_hw_pagetable_attach(hwpt, idev);
- if (rc)
- goto out_unlock;
+ mutex_unlock(&idev->igroup->lock);
- idev->hwpt = hwpt;
- refcount_inc(&hwpt->obj.users);
- list_add(&idev->devices_item, &hwpt->devices);
-out_unlock:
- mutex_unlock(&hwpt->devices_lock);
- return rc;
+ /* Caller must destroy hwpt */
+ return hwpt;
}
/*
@@ -415,7 +411,7 @@ out_unlock:
* Automatic domain selection will never pick a manually created domain.
*/
static int iommufd_device_auto_get_domain(struct iommufd_device *idev,
- struct iommufd_ioas *ioas)
+ struct iommufd_ioas *ioas, u32 *pt_id)
{
struct iommufd_hw_pagetable *hwpt;
int rc;
@@ -432,7 +428,7 @@ static int iommufd_device_auto_get_domai
if (!iommufd_lock_obj(&hwpt->obj))
continue;
- rc = iommufd_device_do_attach(idev, hwpt);
+ rc = iommufd_hw_pagetable_attach(hwpt, idev);
iommufd_put_object(&hwpt->obj);
/*
@@ -442,6 +438,7 @@ static int iommufd_device_auto_get_domai
*/
if (rc == -EINVAL)
continue;
+ *pt_id = hwpt->obj.id;
goto out_unlock;
}
@@ -451,6 +448,7 @@ static int iommufd_device_auto_get_domai
goto out_unlock;
}
hwpt->auto_domain = true;
+ *pt_id = hwpt->obj.id;
mutex_unlock(&ioas->mutex);
iommufd_object_finalize(idev->ictx, &hwpt->obj);
@@ -486,7 +484,7 @@ int iommufd_device_attach(struct iommufd
struct iommufd_hw_pagetable *hwpt =
container_of(pt_obj, struct iommufd_hw_pagetable, obj);
- rc = iommufd_device_do_attach(idev, hwpt);
+ rc = iommufd_hw_pagetable_attach(hwpt, idev);
if (rc)
goto out_put_pt_obj;
break;
@@ -495,7 +493,7 @@ int iommufd_device_attach(struct iommufd
struct iommufd_ioas *ioas =
container_of(pt_obj, struct iommufd_ioas, obj);
- rc = iommufd_device_auto_get_domain(idev, ioas);
+ rc = iommufd_device_auto_get_domain(idev, ioas, pt_id);
if (rc)
goto out_put_pt_obj;
break;
@@ -506,7 +504,6 @@ int iommufd_device_attach(struct iommufd
}
refcount_inc(&idev->obj.users);
- *pt_id = idev->hwpt->obj.id;
rc = 0;
out_put_pt_obj:
@@ -524,14 +521,9 @@ EXPORT_SYMBOL_NS_GPL(iommufd_device_atta
*/
void iommufd_device_detach(struct iommufd_device *idev)
{
- struct iommufd_hw_pagetable *hwpt = idev->hwpt;
-
- mutex_lock(&hwpt->devices_lock);
- list_del(&idev->devices_item);
- idev->hwpt = NULL;
- iommufd_hw_pagetable_detach(hwpt, idev);
- mutex_unlock(&hwpt->devices_lock);
+ struct iommufd_hw_pagetable *hwpt;
+ hwpt = iommufd_hw_pagetable_detach(idev);
if (hwpt->auto_domain)
iommufd_object_deref_user(idev->ictx, &hwpt->obj);
else
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -11,8 +11,6 @@ void iommufd_hw_pagetable_destroy(struct
struct iommufd_hw_pagetable *hwpt =
container_of(obj, struct iommufd_hw_pagetable, obj);
- WARN_ON(!list_empty(&hwpt->devices));
-
if (!list_empty(&hwpt->hwpt_item)) {
mutex_lock(&hwpt->ioas->mutex);
list_del(&hwpt->hwpt_item);
@@ -25,7 +23,6 @@ void iommufd_hw_pagetable_destroy(struct
iommu_domain_free(hwpt->domain);
refcount_dec(&hwpt->ioas->obj.users);
- mutex_destroy(&hwpt->devices_lock);
}
/**
@@ -52,9 +49,7 @@ iommufd_hw_pagetable_alloc(struct iommuf
if (IS_ERR(hwpt))
return hwpt;
- 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;
@@ -65,8 +60,6 @@ iommufd_hw_pagetable_alloc(struct iommuf
goto out_abort;
}
- mutex_lock(&hwpt->devices_lock);
-
/*
* immediate_attach exists only to accommodate iommu drivers that cannot
* directly allocate a domain. These drivers do not finish creating the
@@ -76,29 +69,18 @@ iommufd_hw_pagetable_alloc(struct iommuf
if (immediate_attach) {
rc = iommufd_hw_pagetable_attach(hwpt, idev);
if (rc)
- goto out_unlock;
+ goto out_abort;
}
rc = iopt_table_add_domain(&hwpt->ioas->iopt, hwpt->domain);
if (rc)
goto out_detach;
list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
-
- if (immediate_attach) {
- /* See iommufd_device_do_attach() */
- refcount_inc(&hwpt->obj.users);
- idev->hwpt = hwpt;
- list_add(&idev->devices_item, &hwpt->devices);
- }
-
- mutex_unlock(&hwpt->devices_lock);
return hwpt;
out_detach:
if (immediate_attach)
- iommufd_hw_pagetable_detach(hwpt, idev);
-out_unlock:
- mutex_unlock(&hwpt->devices_lock);
+ iommufd_hw_pagetable_detach(idev);
out_abort:
iommufd_object_abort_and_destroy(ictx, &hwpt->obj);
return ERR_PTR(rc);
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -261,8 +261,6 @@ struct iommufd_hw_pagetable {
bool msi_cookie : 1;
/* Head at iommufd_ioas::hwpt_list */
struct list_head hwpt_item;
- struct mutex devices_lock;
- struct list_head devices;
};
struct iommufd_hw_pagetable *
@@ -270,14 +268,17 @@ iommufd_hw_pagetable_alloc(struct iommuf
struct iommufd_device *idev, bool immediate_attach);
int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
struct iommufd_device *idev);
-void iommufd_hw_pagetable_detach(struct iommufd_hw_pagetable *hwpt,
- struct iommufd_device *idev);
+struct iommufd_hw_pagetable *
+iommufd_hw_pagetable_detach(struct iommufd_device *idev);
void iommufd_hw_pagetable_destroy(struct iommufd_object *obj);
struct iommufd_group {
struct kref ref;
+ struct mutex lock;
struct iommufd_ctx *ictx;
struct iommu_group *group;
+ struct iommufd_hw_pagetable *hwpt;
+ struct list_head device_list;
};
/*
@@ -289,9 +290,7 @@ struct iommufd_device {
struct iommufd_object obj;
struct iommufd_ctx *ictx;
struct iommufd_group *igroup;
- struct iommufd_hw_pagetable *hwpt;
- /* Head at iommufd_hw_pagetable::devices */
- struct list_head devices_item;
+ struct list_head group_item;
/* always the physical device */
struct device *dev;
bool enforce_cache_coherency;