From: Nicolin Chen nicolinc@nvidia.com
A hw_pagetable is allocated with an IOAS id, so it was supposed to link to the IOAS upon its allocation. But, previously with ARM SMMUv3 driver IOMMUFD fails to add a newly allocated hwpt to the IOAS, because SMMUv3 driver "finalises" an iommu_domain (hwpt->domain) after it attaches to a device. This was because the existing domain_alloc op doesn't pass in a dev pointer, so the driver could not know which SMMU device to look for. Now, IOMMUFD allocates the hwpt->domain using domain_alloc_user op that passes in the dev pointer. So, there's no need to wait for a device attachment anymore.
Move iopt_table_add_domain() call, along with the list_add_tail call on the hwpt_item, to the iommufd_hw_pagetable_alloc() right after a domain allocation. Accordingly, move iopt_table_remove_domain() and list_del to the iommufd_hw_pagetable_destroy() routine.
This simplifies the logic in the do_attach/detach(), by reducing the dependency on the device list and potential locking conundrum with the coming nesting feature.
Similarly, drop the iopt_table_add/remove_domain() calls, for selftest, from the iommufd_device_selftest_attach/detach(). Also, allocate hwpts outside the iommufd_device_selftest_attach() to make it symmetric with the iommufd_device_selftest_detach().
Signed-off-by: Nicolin Chen nicolinc@nvidia.com Signed-off-by: Yi Liu yi.l.liu@intel.com --- drivers/iommu/iommufd/device.c | 54 ++++--------------------- drivers/iommu/iommufd/hw_pagetable.c | 13 ++++++ drivers/iommu/iommufd/iommufd_private.h | 6 +-- drivers/iommu/iommufd/selftest.c | 16 ++++++-- 4 files changed, 35 insertions(+), 54 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index 0e5d2bde7b3c..71a8c4f1c4a9 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -311,18 +311,11 @@ static void __iommmufd_device_detach(struct iommufd_device *idev,
mutex_lock(&hwpt->devices_lock); list_del(&idev->devices_item); - if (hwpt->ioas != new_ioas) - mutex_lock(&hwpt->ioas->mutex); - if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) { - if (list_empty(&hwpt->devices)) { - iopt_table_remove_domain(&hwpt->ioas->iopt, - hwpt->domain); - list_del(&hwpt->hwpt_item); - } - if (detach_group) - iommu_detach_group(hwpt->domain, idev->group); - } + if (detach_group && !iommufd_hw_pagetable_has_group(hwpt, idev->group)) + iommu_detach_group(hwpt->domain, idev->group); + if (hwpt->ioas != new_ioas) { + mutex_lock(&hwpt->ioas->mutex); iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev); mutex_unlock(&hwpt->ioas->mutex); } @@ -384,14 +377,6 @@ static int iommufd_device_do_attach(struct iommufd_device *idev, rc = iommu_group_replace_domain(idev->group, hwpt->domain); if (rc) goto out_iova; - - if (list_empty(&hwpt->devices)) { - 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); - } }
/* Replace the cur_hwpt without iommu_detach_group() */ @@ -404,11 +389,6 @@ static int iommufd_device_do_attach(struct iommufd_device *idev, mutex_unlock(&hwpt->devices_lock); return 0;
-out_detach: - if (cur_hwpt) - iommu_group_replace_domain(idev->group, cur_hwpt->domain); - else - iommu_detach_group(hwpt->domain, idev->group); out_iova: iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev); out_unlock: @@ -940,35 +920,17 @@ EXPORT_SYMBOL_NS_GPL(iommufd_access_rw, IOMMUFD); * Creating a real iommufd_device is too hard, bypass creating a iommufd_device * and go directly to attaching a domain. */ -struct iommufd_hw_pagetable * -iommufd_device_selftest_attach(struct iommufd_ctx *ictx, - struct iommufd_ioas *ioas, - struct device *mock_dev) -{ - struct iommufd_hw_pagetable *hwpt; - int rc; - - hwpt = iommufd_hw_pagetable_alloc(ictx, ioas, mock_dev); - if (IS_ERR(hwpt)) - return hwpt; - - rc = iopt_table_add_domain(&hwpt->ioas->iopt, hwpt->domain); - if (rc) - goto out_hwpt;
+int iommufd_device_selftest_attach(struct iommufd_ctx *ictx, + struct iommufd_hw_pagetable *hwpt) +{ refcount_inc(&hwpt->obj.users); - iommufd_object_finalize(ictx, &hwpt->obj); - return hwpt; - -out_hwpt: - iommufd_object_abort_and_destroy(ictx, &hwpt->obj); - return ERR_PTR(rc); + return 0; }
void iommufd_device_selftest_detach(struct iommufd_ctx *ictx, struct iommufd_hw_pagetable *hwpt) { - iopt_table_remove_domain(&hwpt->ioas->iopt, hwpt->domain); refcount_dec(&hwpt->obj.users); } #endif diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c index 08d963ee38c7..bda21ec737cf 100644 --- a/drivers/iommu/iommufd/hw_pagetable.c +++ b/drivers/iommu/iommufd/hw_pagetable.c @@ -11,8 +11,12 @@ void iommufd_hw_pagetable_destroy(struct iommufd_object *obj) struct iommufd_hw_pagetable *hwpt = container_of(obj, struct iommufd_hw_pagetable, obj);
+ lockdep_assert_held(&hwpt->ioas->mutex); + WARN_ON(!list_empty(&hwpt->devices));
+ iopt_table_remove_domain(&hwpt->ioas->iopt, hwpt->domain); + list_del(&hwpt->hwpt_item); iommu_domain_free(hwpt->domain); refcount_dec(&hwpt->ioas->obj.users); mutex_destroy(&hwpt->devices_lock); @@ -34,6 +38,8 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas, struct iommufd_hw_pagetable *hwpt; int rc;
+ lockdep_assert_held(&ioas->mutex); + hwpt = iommufd_object_alloc(ictx, hwpt, IOMMUFD_OBJ_HW_PAGETABLE); if (IS_ERR(hwpt)) return hwpt; @@ -53,11 +59,18 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas, INIT_LIST_HEAD(&hwpt->devices); INIT_LIST_HEAD(&hwpt->hwpt_item); mutex_init(&hwpt->devices_lock); + rc = iopt_table_add_domain(&ioas->iopt, hwpt->domain); + if (rc) + goto out_free_domain; + list_add_tail(&hwpt->hwpt_item, &ioas->hwpt_list); + /* Pairs with iommufd_hw_pagetable_destroy() */ refcount_inc(&ioas->obj.users); hwpt->ioas = ioas; return hwpt;
+out_free_domain: + iommu_domain_free(hwpt->domain); out_abort: iommufd_object_abort(ictx, &hwpt->obj); return ERR_PTR(rc); diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 7748117e36f9..604ad29f87b8 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -278,10 +278,8 @@ void iopt_remove_access(struct io_pagetable *iopt, void iommufd_access_destroy_object(struct iommufd_object *obj);
#ifdef CONFIG_IOMMUFD_TEST -struct iommufd_hw_pagetable * -iommufd_device_selftest_attach(struct iommufd_ctx *ictx, - struct iommufd_ioas *ioas, - struct device *mock_dev); +int iommufd_device_selftest_attach(struct iommufd_ctx *ictx, + struct iommufd_hw_pagetable *hwpt); void iommufd_device_selftest_detach(struct iommufd_ctx *ictx, struct iommufd_hw_pagetable *hwpt); struct device *iommufd_selftest_obj_to_dev(struct iommufd_object *obj); diff --git a/drivers/iommu/iommufd/selftest.c b/drivers/iommu/iommufd/selftest.c index 5013c8757f4b..5f841d1d9e96 100644 --- a/drivers/iommu/iommufd/selftest.c +++ b/drivers/iommu/iommufd/selftest.c @@ -311,22 +311,30 @@ static int iommufd_test_mock_domain(struct iommufd_ucmd *ucmd, sobj->idev.mock_dev.bus = &mock_bus; sobj->idev.mock_dev.iommu = &iommu;
- hwpt = iommufd_device_selftest_attach(ucmd->ictx, ioas, - &sobj->idev.mock_dev); + hwpt = iommufd_hw_pagetable_alloc(ucmd->ictx, ioas, + &sobj->idev.mock_dev); if (IS_ERR(hwpt)) { rc = PTR_ERR(hwpt); - goto out_sobj; + goto out_unlock; } sobj->idev.hwpt = hwpt;
+ rc = iommufd_device_selftest_attach(ucmd->ictx, hwpt); + if (rc) + goto out_free_hwpt; + /* Userspace must destroy both of these IDs to destroy the object */ cmd->mock_domain.out_hwpt_id = hwpt->obj.id; cmd->mock_domain.out_device_id = sobj->obj.id; iommufd_object_finalize(ucmd->ictx, &sobj->obj); + iommufd_object_finalize(ucmd->ictx, &hwpt->obj); iommufd_put_object(&ioas->obj); return iommufd_ucmd_respond(ucmd, sizeof(*cmd));
-out_sobj: +out_free_hwpt: + iommufd_object_abort_and_destroy(ucmd->ictx, &hwpt->obj); +out_unlock: + mutex_unlock(&ioas->mutex); iommufd_object_abort(ucmd->ictx, &sobj->obj); out_ioas: iommufd_put_object(&ioas->obj);