On 2023/3/8 8:35, Jason Gunthorpe wrote:
Logically the HWPT should have the coherency set properly for the device that it is being created for when it is created.
This was happening implicitly if the immediate_attach was set because iommufd_hw_pagetable_attach() does it as the first thing.
Do it unconditionally so !immediate_attach works properly.
Reviewed-by: Kevin Tian kevin.tian@intel.com Signed-off-by: Jason Gunthorpe jgg@nvidia.com
drivers/iommu/iommufd/device.c | 20 +++++------------- drivers/iommu/iommufd/hw_pagetable.c | 27 +++++++++++++++++++++++++ drivers/iommu/iommufd/iommufd_private.h | 1 + 3 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index d6d5c2fcc33889..ddde14d6d1352c 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -293,21 +293,11 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt, if (idev->igroup->hwpt != NULL && idev->igroup->hwpt != hwpt) return -EINVAL;
- /*
* Try to upgrade the domain we have, it is an iommu driver bug to* report IOMMU_CAP_ENFORCE_CACHE_COHERENCY but fail* enforce_cache_coherency when there are no devices attached to the* domain.*/- if (idev->enforce_cache_coherency && !hwpt->enforce_cache_coherency) {
if (hwpt->domain->ops->enforce_cache_coherency)hwpt->enforce_cache_coherency =hwpt->domain->ops->enforce_cache_coherency(hwpt->domain);if (!hwpt->enforce_cache_coherency) {WARN_ON(list_empty(&idev->igroup->device_list));return -EINVAL;}
- /* Try to upgrade the domain we have */
- if (idev->enforce_cache_coherency) {
rc = iommufd_hw_pagetable_enforce_cc(hwpt);if (rc)return rc;
As "HWPT should have the coherency set properly for the device that it is being created for when it is created", is it an incompatible case if
idev->enforce_cache_coherency && !hwpt->enforce_cache_coherency
?
If so, why not,
if (idev->enforce_cache_coherency && !hwpt->enforce_cache_coherency) return -EINVAL;
?
Best regards, baolu
} rc = iopt_table_enforce_dev_resv_regions(&hwpt->ioas->iopt, idev->dev, diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c index 566eba0cd9b917..2584f9038b29a2 100644 --- a/drivers/iommu/iommufd/hw_pagetable.c +++ b/drivers/iommu/iommufd/hw_pagetable.c @@ -25,6 +25,20 @@ void iommufd_hw_pagetable_destroy(struct iommufd_object *obj) refcount_dec(&hwpt->ioas->obj.users); } +int iommufd_hw_pagetable_enforce_cc(struct iommufd_hw_pagetable *hwpt) +{
- if (hwpt->enforce_cache_coherency)
return 0;- if (hwpt->domain->ops->enforce_cache_coherency)
hwpt->enforce_cache_coherency =hwpt->domain->ops->enforce_cache_coherency(hwpt->domain);- if (!hwpt->enforce_cache_coherency)
return -EINVAL;- return 0;
+}
- /**
- iommufd_hw_pagetable_alloc() - Get an iommu_domain for a device
- @ictx: iommufd context
@@ -60,6 +74,19 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas, goto out_abort; }
- /*
* Set the coherency mode before we do iopt_table_add_domain() as some* iommus have a per-PTE bit that controls it and need to decide before* doing any maps. It is an iommu driver bug to report* IOMMU_CAP_ENFORCE_CACHE_COHERENCY but fail enforce_cache_coherency on* a new domain.*/- if (idev->enforce_cache_coherency) {
rc = iommufd_hw_pagetable_enforce_cc(hwpt);if (WARN_ON(rc))goto out_abort;- }
- mutex_lock(&idev->igroup->lock);
/* diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 8bf053f4d4a9ce..471a3fdff1e0b6 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -254,6 +254,7 @@ struct iommufd_hw_pagetable { struct iommufd_hw_pagetable * iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas, struct iommufd_device *idev, bool immediate_attach); +int iommufd_hw_pagetable_enforce_cc(struct iommufd_hw_pagetable *hwpt); int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt, struct iommufd_device *idev); struct iommufd_hw_pagetable *