On 2024/6/28 17:52, Baolu Lu wrote:
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@xxxxxxxxx>
---
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);
+
[1]
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.
actually, this does not break the assumption. In [1]. it already clears the
pasid entry.
Is this a requirement for the replace functionality?
Replace would surely need to modify a present pasid entry. But this patch
only moves adds the pasid entry tear down and prq drain into the helpers.
--
Regards,
Yi Liu