diff mbox series

[2/6] iommu/vt-d: Move intel_drain_pasid_prq() into intel_pasid_tear_down_entry()

Message ID 20240628085538.47049-3-yi.l.liu@intel.com (mailing list archive)
State New
Headers show
Series Make set_dev_pasid op supportting domain replacement | expand

Commit Message

Yi Liu June 28, 2024, 8:55 a.m. UTC 
Draining PRQ is needed before repurposing a PASID. It makes sense to invoke
it in the intel_pasid_tear_down_entry().

Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/iommu/intel/iommu.c | 5 ++---
 drivers/iommu/intel/pasid.c | 9 ++++++++-
 drivers/iommu/intel/pasid.h | 5 ++---
 drivers/iommu/intel/svm.c   | 6 +++++-
 4 files changed, 17 insertions(+), 8 deletions(-)

Comments

Baolu Lu June 28, 2024, 9:42 a.m. UTC | #1 
On 2024/6/28 16:55, Yi Liu wrote:
> Draining PRQ is needed before repurposing a PASID. It makes sense to invoke
> it in the intel_pasid_tear_down_entry().

Can you please elaborate on the value of this merge?

Draining the PRQ is necessary when PRI is enabled on the device, and a
page table is about to be removed from the PASID. This might occur in
conjunction with tearing down a PASID entry, but it seems they are two
distinct actions.

Best regards,
baolu
Yi Liu June 28, 2024, 10:51 a.m. UTC | #2 
On 2024/6/28 17:42, Baolu Lu wrote:
> On 2024/6/28 16:55, Yi Liu wrote:
>> Draining PRQ is needed before repurposing a PASID. It makes sense to invoke
>> it in the intel_pasid_tear_down_entry().
> 
> Can you please elaborate on the value of this merge?
> 

The major reason is that the next patch would have multiple places that
need to destroy pasid entry and do prq drain. Wrap them would make life
easier I suppose.

> Draining the PRQ is necessary when PRI is enabled on the device, and a
> page table is about to be removed from the PASID. This might occur in
> conjunction with tearing down a PASID entry, but it seems they are two
> distinct actions.

Seems like mostly they have conjunction, while there is indeed one
exception in the intel_mm_release(). Given the above reason, do you have
any suggestion for it?
diff mbox series

Patch

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 288c929b3d15..dd3de95c7122 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3241,7 +3241,7 @@  void device_block_translation(struct device *dev)
 	if (!dev_is_real_dma_subdevice(dev)) {
 		if (sm_supported(iommu))
 			intel_pasid_tear_down_entry(iommu, dev,
-						    IOMMU_NO_PASID, false);
+						    IOMMU_NO_PASID, false, false);
 		else
 			domain_context_clear(info);
 	}
@@ -4060,8 +4060,7 @@  static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid,
 	intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
 	kfree(dev_pasid);
 out_tear_down:
-	intel_pasid_tear_down_entry(iommu, dev, pasid, false);
-	intel_drain_pasid_prq(dev, pasid);
+	intel_pasid_tear_down_entry(iommu, dev, pasid, false, true);
 }
 
 static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
index 573e1b8e3cfb..b18eebb479de 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -233,8 +233,12 @@  devtlb_invalidation_with_pasid(struct intel_iommu *iommu,
 		qi_flush_dev_iotlb_pasid(iommu, sid, pfsid, pasid, qdep, 0, 64 - VTD_PAGE_SHIFT);
 }
 
+/*
+ * Not all PASID entry destroy requires PRQ drain as it can be handled in
+ * the remove_dev_pasid path. Caller should be clear on it.
+ */
 void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev,
-				 u32 pasid, bool fault_ignore)
+				 u32 pasid, bool fault_ignore, bool drain_prq)
 {
 	struct pasid_entry *pte;
 	u16 did, pgtt;
@@ -264,6 +268,9 @@  void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev,
 	/* Device IOTLB doesn't need to be flushed in caching mode. */
 	if (!cap_caching_mode(iommu->cap))
 		devtlb_invalidation_with_pasid(iommu, dev, pasid);
+
+	if (drain_prq)
+		intel_drain_pasid_prq(dev, pasid);
 }
 
 /*
diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index da9978fef7ac..8b77b0d21c6e 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -313,9 +313,8 @@  int intel_pasid_setup_pass_through(struct intel_iommu *iommu,
 				   struct device *dev, u32 pasid);
 int intel_pasid_setup_nested(struct intel_iommu *iommu, struct device *dev,
 			     u32 pasid, struct dmar_domain *domain);
-void intel_pasid_tear_down_entry(struct intel_iommu *iommu,
-				 struct device *dev, u32 pasid,
-				 bool fault_ignore);
+void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev,
+				 u32 pasid, bool fault_ignore, bool drain_prq);
 void intel_pasid_setup_page_snoop_control(struct intel_iommu *iommu,
 					  struct device *dev, u32 pasid);
 int intel_pasid_setup_sm_context(struct device *dev);
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index a5845ca94867..679e094d9f52 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -175,8 +175,12 @@  static void intel_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
 	spin_lock_irqsave(&domain->lock, flags);
 	list_for_each_entry(dev_pasid, &domain->dev_pasids, link_domain) {
 		info = dev_iommu_priv_get(dev_pasid->dev);
+		/*
+		 * PRQ drain would happen in the remove_dev_pasid() path,
+		 * no need to do it here.
+		 */
 		intel_pasid_tear_down_entry(info->iommu, dev_pasid->dev,
-					    dev_pasid->pasid, true);
+					    dev_pasid->pasid, true, false);
 	}
 	spin_unlock_irqrestore(&domain->lock, flags);