RE: [PATCH 3/6] iommu/vt-d: Make helpers support modifying present pasid entry

Tian, Kevin
2024-07-15T07:53:46Z

> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Friday, June 28, 2024 4:56 PM
>
> diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> index b18eebb479de..5d3a12b081a2 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -314,6 +314,9 @@ int intel_pasid_setup_first_level(struct intel_iommu
> *iommu,
> return -EINVAL;
> }
>
> + /* Clear the old configuration if it already exists */
> + intel_pasid_tear_down_entry(iommu, dev, pasid, false, true);
> +
> spin_lock(&iommu->lock);
> pte = intel_pasid_get_entry(dev, pasid);
> if (!pte) {

with this change there will be two invocations on
intel_pasid_tear_down_entry() in the call stack of RID attach:

intel_iommu_attach_device()
device_block_translation()
intel_pasid_tear_down_entry()
dmar_domain_attach_device()
domain_setup_first_level()
intel_pasid_tear_down_entry()

it's not being a real problem as intel_pasid_tear_down_entry()
exits early if the pasid entry is non-present, but it will likely cause
confusion when reading the code.

What about moving it into intel_iommu_set_dev_pasid() to
better show the purpose?