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

Baolu Lu
2024-06-28T09:52:12Z

On 2024/6/28 16:55, Yi Liu wrote:
> To handle domain replacement, set_dev_pasid op needs to modify a present
> pasid entry. One way is sharing the most logics of remove_dev_pasid() in
> the beginning of set_dev_pasid() to remove the old config. But this means
> the set_dev_pasid path needs to rollback to the old config if it fails to
> set up the new pasid entry. This needs to invoke the set_dev_pasid op of
> the old domain. It breaks the iommu layering a bit. Another way is
> implementing the set_dev_pasid() without rollback to old hardware config.
> This can be achieved by implementing it in the order of preparing the
> dev_pasid info for the new domain, modify the pasid entry, then undo the
> dev_pasid info of the old domain, and if failed, undo the dev_pasid info
> of the new domain. This would keep the old domain unchanged.
>
> Following the second way, needs to make the pasid entry set up helpers
> support modifying present pasid entry.
>
> Signed-off-by: Yi Liu<yi.l.liu@intel.com>
> ---
> drivers/iommu/intel/pasid.c | 37 ++++++++++++-------------------------
> 1 file changed, 12 insertions(+), 25 deletions(-)
>
> 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) {
> @@ -321,13 +324,6 @@ int intel_pasid_setup_first_level(struct intel_iommu *iommu,
> return -ENODEV;
> }
>
> - if (pasid_pte_is_present(pte)) {
> - spin_unlock(&iommu->lock);
> - return -EBUSY;
> - }
> -
> - pasid_clear_entry(pte);
> -
> /* Setup the first level page table pointer: */
> pasid_set_flptr(pte, (u64)__pa(pgd));

The above changes the previous assumption that when a new page table is
about to be set up on a PASID, there should be no existing one still in
place.

Is this a requirement for the replace functionality?

Best regards,
baolu