dmaengine.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] iommu/vt-d: Refactor code for non-PRI IOPF
@ 2023-03-24 12:02 Lu Baolu
  2023-03-24 12:02 ` [PATCH v3 1/6] dmaengine: idxd: Add enable/disable device IOPF feature Lu Baolu
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Lu Baolu @ 2023-03-24 12:02 UTC (permalink / raw)
  To: iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian, Fenghua Yu,
	Dave Jiang, Vinod Koul, Jacob Pan, linux-kernel, Lu Baolu

The existing SVA and IOPF implementation assumes that devices report I/O
page faults via PCI PRI. This is not always true as some emerging
devices are designed to handle the I/O page faults by themselves without
ever sending PCI page requests nor advertising PRI capability.

Refactor the SVA and IOPF code to allow SVA support with IOPF handled
either by IOMMU (PCI PRI) or device driver (device-specific IOPF).

This series is based on v6.3-rc1 and also available at github:
https://github.com/LuBaolu/intel-iommu/commits/intel-iommu-refactor-iopf-v3

Change log:
v3:
 - Split removing unnecessary checks in iopf disabling path into a
   separate patch.
 - Add some in-line comments.

v2:
 - https://lore.kernel.org/linux-iommu/20230309025639.26109-1-baolu.lu@linux.intel.com/
 - Separate a fix patch and merge it into v6. 3-rc1 [commit 60b1daa3b168
   ("iommu/vt-d: Fix error handling in sva enable/disable paths")]
 - Disallow device-specific IOPF is device advertising PRI capability;
 - Add some extra patches to move all IOPF related code to the IOPF
   enabling/disabling paths.

v1: Initial post
 - https://lore.kernel.org/linux-iommu/20230203084456.469641-1-baolu.lu@linux.intel.com/

Lu Baolu (6):
  dmaengine: idxd: Add enable/disable device IOPF feature
  iommu/vt-d: Allow SVA with device-specific IOPF
  iommu/vt-d: Move iopf code from SVA to IOPF enabling path
  iommu/vt-d: Move pfsid and ats_qdep calculation to device probe path
  iommu/vt-d: Move PRI handling to IOPF feature path
  iommu/vt-d: Remove unnecessary checks in iopf disabling path

 drivers/dma/idxd/init.c     |  31 +++++++--
 drivers/iommu/intel/iommu.c | 132 ++++++++++++++++++++++++------------
 2 files changed, 112 insertions(+), 51 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v3 1/6] dmaengine: idxd: Add enable/disable device IOPF feature
  2023-03-24 12:02 [PATCH v3 0/6] iommu/vt-d: Refactor code for non-PRI IOPF Lu Baolu
@ 2023-03-24 12:02 ` Lu Baolu
  2023-04-03  5:49   ` Baolu Lu
  2023-03-24 12:02 ` [PATCH v3 2/6] iommu/vt-d: Allow SVA with device-specific IOPF Lu Baolu
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Lu Baolu @ 2023-03-24 12:02 UTC (permalink / raw)
  To: iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian, Fenghua Yu,
	Dave Jiang, Vinod Koul, Jacob Pan, linux-kernel, Lu Baolu

The iommu subsystem requires IOMMU_DEV_FEAT_IOPF must be enabled before
and disabled after IOMMU_DEV_FEAT_SVA, if device's I/O page faults rely
on the IOMMU. Add explicit IOMMU_DEV_FEAT_IOPF enabling/disabling in this
driver.

At present, missing IOPF enabling/disabling doesn't cause any real issue,
because the IOMMU driver places the IOPF enabling/disabling in the path
of SVA feature handling. But this may change.

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Fenghua Yu <fenghua.yu@intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/dma/idxd/init.c | 31 +++++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c
index 640d3048368e..09ef62aa0635 100644
--- a/drivers/dma/idxd/init.c
+++ b/drivers/dma/idxd/init.c
@@ -516,6 +516,27 @@ static void idxd_disable_system_pasid(struct idxd_device *idxd)
 	idxd->sva = NULL;
 }
 
+static int idxd_enable_sva(struct pci_dev *pdev)
+{
+	int ret;
+
+	ret = iommu_dev_enable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
+	if (ret)
+		return ret;
+
+	ret = iommu_dev_enable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
+	if (ret)
+		iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
+
+	return ret;
+}
+
+static void idxd_disable_sva(struct pci_dev *pdev)
+{
+	iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
+	iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_IOPF);
+}
+
 static int idxd_probe(struct idxd_device *idxd)
 {
 	struct pci_dev *pdev = idxd->pdev;
@@ -530,7 +551,7 @@ static int idxd_probe(struct idxd_device *idxd)
 	dev_dbg(dev, "IDXD reset complete\n");
 
 	if (IS_ENABLED(CONFIG_INTEL_IDXD_SVM) && sva) {
-		if (iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA)) {
+		if (idxd_enable_sva(pdev)) {
 			dev_warn(dev, "Unable to turn on user SVA feature.\n");
 		} else {
 			set_bit(IDXD_FLAG_USER_PASID_ENABLED, &idxd->flags);
@@ -578,21 +599,19 @@ static int idxd_probe(struct idxd_device *idxd)
 	if (device_pasid_enabled(idxd))
 		idxd_disable_system_pasid(idxd);
 	if (device_user_pasid_enabled(idxd))
-		iommu_dev_disable_feature(dev, IOMMU_DEV_FEAT_SVA);
+		idxd_disable_sva(pdev);
 	return rc;
 }
 
 static void idxd_cleanup(struct idxd_device *idxd)
 {
-	struct device *dev = &idxd->pdev->dev;
-
 	perfmon_pmu_remove(idxd);
 	idxd_cleanup_interrupts(idxd);
 	idxd_cleanup_internals(idxd);
 	if (device_pasid_enabled(idxd))
 		idxd_disable_system_pasid(idxd);
 	if (device_user_pasid_enabled(idxd))
-		iommu_dev_disable_feature(dev, IOMMU_DEV_FEAT_SVA);
+		idxd_disable_sva(idxd->pdev);
 }
 
 static int idxd_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
@@ -710,7 +729,7 @@ static void idxd_remove(struct pci_dev *pdev)
 	pci_free_irq_vectors(pdev);
 	pci_iounmap(pdev, idxd->reg_base);
 	if (device_user_pasid_enabled(idxd))
-		iommu_dev_disable_feature(&pdev->dev, IOMMU_DEV_FEAT_SVA);
+		idxd_disable_sva(pdev);
 	pci_disable_device(pdev);
 	destroy_workqueue(idxd->wq);
 	perfmon_pmu_remove(idxd);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 2/6] iommu/vt-d: Allow SVA with device-specific IOPF
  2023-03-24 12:02 [PATCH v3 0/6] iommu/vt-d: Refactor code for non-PRI IOPF Lu Baolu
  2023-03-24 12:02 ` [PATCH v3 1/6] dmaengine: idxd: Add enable/disable device IOPF feature Lu Baolu
@ 2023-03-24 12:02 ` Lu Baolu
  2023-03-28  2:10   ` Tian, Kevin
  2023-03-24 12:02 ` [PATCH v3 3/6] iommu/vt-d: Move iopf code from SVA to IOPF enabling path Lu Baolu
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Lu Baolu @ 2023-03-24 12:02 UTC (permalink / raw)
  To: iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian, Fenghua Yu,
	Dave Jiang, Vinod Koul, Jacob Pan, linux-kernel, Lu Baolu

Currently enabling SVA requires IOPF support from the IOMMU and device
PCI PRI. However, some devices can handle IOPF by itself without ever
sending PCI page requests nor advertising PRI capability.

Allow SVA support with IOPF handled either by IOMMU (PCI PRI) or device
driver (device-specific IOPF). As long as IOPF could be handled, SVA
should continue to work.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 7c2f4bd33582..caf664448ee9 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4650,7 +4650,21 @@ static int intel_iommu_enable_sva(struct device *dev)
 	if (!(iommu->flags & VTD_FLAG_SVM_CAPABLE))
 		return -ENODEV;
 
-	if (!info->pasid_enabled || !info->pri_enabled || !info->ats_enabled)
+	if (!info->pasid_enabled || !info->ats_enabled)
+		return -EINVAL;
+
+	/*
+	 * Devices having device-specific I/O fault handling should not
+	 * support PCI/PRI. The IOMMU side has no means to check the
+	 * capability of device-specific IOPF.  Therefore, IOMMU can only
+	 * default that if the device driver enables SVA on a non-PRI
+	 * device, it will handle IOPF in its own way.
+	 */
+	if (!info->pri_supported)
+		return 0;
+
+	/* Devices supporting PRI should have it enabled. */
+	if (!info->pri_enabled)
 		return -EINVAL;
 
 	ret = iopf_queue_add_device(iommu->iopf_queue, dev);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 3/6] iommu/vt-d: Move iopf code from SVA to IOPF enabling path
  2023-03-24 12:02 [PATCH v3 0/6] iommu/vt-d: Refactor code for non-PRI IOPF Lu Baolu
  2023-03-24 12:02 ` [PATCH v3 1/6] dmaengine: idxd: Add enable/disable device IOPF feature Lu Baolu
  2023-03-24 12:02 ` [PATCH v3 2/6] iommu/vt-d: Allow SVA with device-specific IOPF Lu Baolu
@ 2023-03-24 12:02 ` Lu Baolu
  2023-03-24 12:02 ` [PATCH v3 4/6] iommu/vt-d: Move pfsid and ats_qdep calculation to device probe path Lu Baolu
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Lu Baolu @ 2023-03-24 12:02 UTC (permalink / raw)
  To: iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian, Fenghua Yu,
	Dave Jiang, Vinod Koul, Jacob Pan, linux-kernel, Lu Baolu

Generally enabling IOMMU_DEV_FEAT_SVA requires IOMMU_DEV_FEAT_IOPF, but
some devices manage I/O Page Faults themselves instead of relying on the
IOMMU. Move IOPF related code from SVA to IOPF enabling path.

For the device drivers that relies on the IOMMU for IOPF through PCI/PRI,
IOMMU_DEV_FEAT_IOPF must be enabled before and disabled after
IOMMU_DEV_FEAT_SVA.

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index caf664448ee9..a6f07c74da2d 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4638,7 +4638,6 @@ static int intel_iommu_enable_sva(struct device *dev)
 {
 	struct device_domain_info *info = dev_iommu_priv_get(dev);
 	struct intel_iommu *iommu;
-	int ret;
 
 	if (!info || dmar_disabled)
 		return -EINVAL;
@@ -4667,6 +4666,21 @@ static int intel_iommu_enable_sva(struct device *dev)
 	if (!info->pri_enabled)
 		return -EINVAL;
 
+	return 0;
+}
+
+static int intel_iommu_enable_iopf(struct device *dev)
+{
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+	struct intel_iommu *iommu;
+	int ret;
+
+	if (!info || !info->ats_enabled || !info->pri_enabled)
+		return -ENODEV;
+	iommu = info->iommu;
+	if (!iommu)
+		return -EINVAL;
+
 	ret = iopf_queue_add_device(iommu->iopf_queue, dev);
 	if (ret)
 		return ret;
@@ -4678,7 +4692,7 @@ static int intel_iommu_enable_sva(struct device *dev)
 	return ret;
 }
 
-static int intel_iommu_disable_sva(struct device *dev)
+static int intel_iommu_disable_iopf(struct device *dev)
 {
 	struct device_domain_info *info = dev_iommu_priv_get(dev);
 	struct intel_iommu *iommu = info->iommu;
@@ -4695,16 +4709,6 @@ static int intel_iommu_disable_sva(struct device *dev)
 	return ret;
 }
 
-static int intel_iommu_enable_iopf(struct device *dev)
-{
-	struct device_domain_info *info = dev_iommu_priv_get(dev);
-
-	if (info && info->pri_supported)
-		return 0;
-
-	return -ENODEV;
-}
-
 static int
 intel_iommu_dev_enable_feat(struct device *dev, enum iommu_dev_features feat)
 {
@@ -4725,10 +4729,10 @@ intel_iommu_dev_disable_feat(struct device *dev, enum iommu_dev_features feat)
 {
 	switch (feat) {
 	case IOMMU_DEV_FEAT_IOPF:
-		return 0;
+		return intel_iommu_disable_iopf(dev);
 
 	case IOMMU_DEV_FEAT_SVA:
-		return intel_iommu_disable_sva(dev);
+		return 0;
 
 	default:
 		return -ENODEV;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 4/6] iommu/vt-d: Move pfsid and ats_qdep calculation to device probe path
  2023-03-24 12:02 [PATCH v3 0/6] iommu/vt-d: Refactor code for non-PRI IOPF Lu Baolu
                   ` (2 preceding siblings ...)
  2023-03-24 12:02 ` [PATCH v3 3/6] iommu/vt-d: Move iopf code from SVA to IOPF enabling path Lu Baolu
@ 2023-03-24 12:02 ` Lu Baolu
  2023-03-24 12:02 ` [PATCH v3 5/6] iommu/vt-d: Move PRI handling to IOPF feature path Lu Baolu
  2023-03-24 12:02 ` [PATCH v3 6/6] iommu/vt-d: Remove unnecessary checks in iopf disabling path Lu Baolu
  5 siblings, 0 replies; 13+ messages in thread
From: Lu Baolu @ 2023-03-24 12:02 UTC (permalink / raw)
  To: iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian, Fenghua Yu,
	Dave Jiang, Vinod Koul, Jacob Pan, linux-kernel, Lu Baolu

They should be part of the per-device iommu private data initialization.

Reviewed-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 26 +++++++++++---------------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index a6f07c74da2d..6d77b4072fdd 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1406,20 +1406,6 @@ static void iommu_enable_pci_caps(struct device_domain_info *info)
 		return;
 
 	pdev = to_pci_dev(info->dev);
-	/* For IOMMU that supports device IOTLB throttling (DIT), we assign
-	 * PFSID to the invalidation desc of a VF such that IOMMU HW can gauge
-	 * queue depth at PF level. If DIT is not set, PFSID will be treated as
-	 * reserved, which should be set to 0.
-	 */
-	if (!ecap_dit(info->iommu->ecap))
-		info->pfsid = 0;
-	else {
-		struct pci_dev *pf_pdev;
-
-		/* pdev will be returned if device is not a vf */
-		pf_pdev = pci_physfn(pdev);
-		info->pfsid = pci_dev_id(pf_pdev);
-	}
 
 	/* The PCIe spec, in its wisdom, declares that the behaviour of
 	   the device if you enable PASID support after ATS support is
@@ -1438,7 +1424,6 @@ static void iommu_enable_pci_caps(struct device_domain_info *info)
 	    !pci_enable_ats(pdev, VTD_PAGE_SHIFT)) {
 		info->ats_enabled = 1;
 		domain_update_iotlb(info->domain);
-		info->ats_qdep = pci_ats_queue_depth(pdev);
 	}
 }
 
@@ -4521,6 +4506,17 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
 		    dmar_ats_supported(pdev, iommu)) {
 			info->ats_supported = 1;
 			info->dtlb_extra_inval = dev_needs_extra_dtlb_flush(pdev);
+
+			/*
+			 * For IOMMU that supports device IOTLB throttling
+			 * (DIT), we assign PFSID to the invalidation desc
+			 * of a VF such that IOMMU HW can gauge queue depth
+			 * at PF level. If DIT is not set, PFSID will be
+			 * treated as reserved, which should be set to 0.
+			 */
+			if (ecap_dit(iommu->ecap))
+				info->pfsid = pci_dev_id(pci_physfn(pdev));
+			info->ats_qdep = pci_ats_queue_depth(pdev);
 		}
 		if (sm_supported(iommu)) {
 			if (pasid_supported(iommu)) {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 5/6] iommu/vt-d: Move PRI handling to IOPF feature path
  2023-03-24 12:02 [PATCH v3 0/6] iommu/vt-d: Refactor code for non-PRI IOPF Lu Baolu
                   ` (3 preceding siblings ...)
  2023-03-24 12:02 ` [PATCH v3 4/6] iommu/vt-d: Move pfsid and ats_qdep calculation to device probe path Lu Baolu
@ 2023-03-24 12:02 ` Lu Baolu
  2023-03-28  2:11   ` Tian, Kevin
  2023-03-24 12:02 ` [PATCH v3 6/6] iommu/vt-d: Remove unnecessary checks in iopf disabling path Lu Baolu
  5 siblings, 1 reply; 13+ messages in thread
From: Lu Baolu @ 2023-03-24 12:02 UTC (permalink / raw)
  To: iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian, Fenghua Yu,
	Dave Jiang, Vinod Koul, Jacob Pan, linux-kernel, Lu Baolu

PRI is only used for IOPF. With this move, the PCI/PRI feature could be
controlled by the device driver through iommu_dev_enable/disable_feature()
interfaces.

Reviewed-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 53 ++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 6d77b4072fdd..cd3a3c4b5e64 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1415,11 +1415,6 @@ static void iommu_enable_pci_caps(struct device_domain_info *info)
 	if (info->pasid_supported && !pci_enable_pasid(pdev, info->pasid_supported & ~1))
 		info->pasid_enabled = 1;
 
-	if (info->pri_supported &&
-	    (info->pasid_enabled ? pci_prg_resp_pasid_required(pdev) : 1)  &&
-	    !pci_reset_pri(pdev) && !pci_enable_pri(pdev, PRQ_DEPTH))
-		info->pri_enabled = 1;
-
 	if (info->ats_supported && pci_ats_page_aligned(pdev) &&
 	    !pci_enable_ats(pdev, VTD_PAGE_SHIFT)) {
 		info->ats_enabled = 1;
@@ -1442,11 +1437,6 @@ static void iommu_disable_pci_caps(struct device_domain_info *info)
 		domain_update_iotlb(info->domain);
 	}
 
-	if (info->pri_enabled) {
-		pci_disable_pri(pdev);
-		info->pri_enabled = 0;
-	}
-
 	if (info->pasid_enabled) {
 		pci_disable_pasid(pdev);
 		info->pasid_enabled = 0;
@@ -4667,23 +4657,48 @@ static int intel_iommu_enable_sva(struct device *dev)
 
 static int intel_iommu_enable_iopf(struct device *dev)
 {
+	struct pci_dev *pdev = dev_is_pci(dev) ? to_pci_dev(dev) : NULL;
 	struct device_domain_info *info = dev_iommu_priv_get(dev);
 	struct intel_iommu *iommu;
 	int ret;
 
-	if (!info || !info->ats_enabled || !info->pri_enabled)
+	if (!pdev || !info || !info->ats_enabled || !info->pri_supported)
 		return -ENODEV;
+
+	if (info->pri_enabled)
+		return -EBUSY;
+
 	iommu = info->iommu;
 	if (!iommu)
 		return -EINVAL;
 
+	/* PASID is required in PRG Response Message. */
+	if (info->pasid_enabled && !pci_prg_resp_pasid_required(pdev))
+		return -EINVAL;
+
+	ret = pci_reset_pri(pdev);
+	if (ret)
+		return ret;
+
 	ret = iopf_queue_add_device(iommu->iopf_queue, dev);
 	if (ret)
 		return ret;
 
 	ret = iommu_register_device_fault_handler(dev, iommu_queue_iopf, dev);
 	if (ret)
-		iopf_queue_remove_device(iommu->iopf_queue, dev);
+		goto iopf_remove_device;
+
+	ret = pci_enable_pri(pdev, PRQ_DEPTH);
+	if (ret)
+		goto iopf_unregister_handler;
+	info->pri_enabled = 1;
+
+	return 0;
+
+iopf_unregister_handler:
+	iommu_unregister_device_fault_handler(dev);
+iopf_remove_device:
+	iopf_queue_remove_device(iommu->iopf_queue, dev);
 
 	return ret;
 }
@@ -4694,6 +4709,20 @@ static int intel_iommu_disable_iopf(struct device *dev)
 	struct intel_iommu *iommu = info->iommu;
 	int ret;
 
+	if (!info->pri_enabled)
+		return -EINVAL;
+
+	/*
+	 * PCIe spec states that by clearing PRI enable bit, the Page
+	 * Request Interface will not issue new page requests, but has
+	 * outstanding page requests that have been transmitted or are
+	 * queued for transmission. This is supposed to be called after
+	 * the device driver has stopped DMA, all PASIDs have been
+	 * unbound and the outstanding PRQs have been drained.
+	 */
+	pci_disable_pri(to_pci_dev(dev));
+	info->pri_enabled = 0;
+
 	ret = iommu_unregister_device_fault_handler(dev);
 	if (ret)
 		return ret;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v3 6/6] iommu/vt-d: Remove unnecessary checks in iopf disabling path
  2023-03-24 12:02 [PATCH v3 0/6] iommu/vt-d: Refactor code for non-PRI IOPF Lu Baolu
                   ` (4 preceding siblings ...)
  2023-03-24 12:02 ` [PATCH v3 5/6] iommu/vt-d: Move PRI handling to IOPF feature path Lu Baolu
@ 2023-03-24 12:02 ` Lu Baolu
  2023-03-28  2:11   ` Tian, Kevin
  5 siblings, 1 reply; 13+ messages in thread
From: Lu Baolu @ 2023-03-24 12:02 UTC (permalink / raw)
  To: iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian, Fenghua Yu,
	Dave Jiang, Vinod Koul, Jacob Pan, linux-kernel, Lu Baolu

iommu_unregister_device_fault_handler() and iopf_queue_remove_device()
are called after device has stopped issuing new page falut requests and
all outstanding page requests have been drained. They should never fail.
Trigger a warning if it happens unfortunately.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/iommu.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index cd3a3c4b5e64..c771233d6f2a 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4707,7 +4707,6 @@ static int intel_iommu_disable_iopf(struct device *dev)
 {
 	struct device_domain_info *info = dev_iommu_priv_get(dev);
 	struct intel_iommu *iommu = info->iommu;
-	int ret;
 
 	if (!info->pri_enabled)
 		return -EINVAL;
@@ -4723,15 +4722,15 @@ static int intel_iommu_disable_iopf(struct device *dev)
 	pci_disable_pri(to_pci_dev(dev));
 	info->pri_enabled = 0;
 
-	ret = iommu_unregister_device_fault_handler(dev);
-	if (ret)
-		return ret;
-
-	ret = iopf_queue_remove_device(iommu->iopf_queue, dev);
-	if (ret)
-		iommu_register_device_fault_handler(dev, iommu_queue_iopf, dev);
+	/*
+	 * With PRI disabled and outstanding PRQs drained, unregistering
+	 * fault handler and removing device from iopf queue should never
+	 * fail.
+	 */
+	WARN_ON(iommu_unregister_device_fault_handler(dev));
+	WARN_ON(iopf_queue_remove_device(iommu->iopf_queue, dev));
 
-	return ret;
+	return 0;
 }
 
 static int
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* RE: [PATCH v3 2/6] iommu/vt-d: Allow SVA with device-specific IOPF
  2023-03-24 12:02 ` [PATCH v3 2/6] iommu/vt-d: Allow SVA with device-specific IOPF Lu Baolu
@ 2023-03-28  2:10   ` Tian, Kevin
  0 siblings, 0 replies; 13+ messages in thread
From: Tian, Kevin @ 2023-03-28  2:10 UTC (permalink / raw)
  To: Lu Baolu, iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Yu, Fenghua, Jiang,
	Dave, Vinod Koul, Jacob Pan, linux-kernel

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, March 24, 2023 8:03 PM
> 
> Currently enabling SVA requires IOPF support from the IOMMU and device
> PCI PRI. However, some devices can handle IOPF by itself without ever
> sending PCI page requests nor advertising PRI capability.
> 
> Allow SVA support with IOPF handled either by IOMMU (PCI PRI) or device
> driver (device-specific IOPF). As long as IOPF could be handled, SVA
> should continue to work.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v3 5/6] iommu/vt-d: Move PRI handling to IOPF feature path
  2023-03-24 12:02 ` [PATCH v3 5/6] iommu/vt-d: Move PRI handling to IOPF feature path Lu Baolu
@ 2023-03-28  2:11   ` Tian, Kevin
  0 siblings, 0 replies; 13+ messages in thread
From: Tian, Kevin @ 2023-03-28  2:11 UTC (permalink / raw)
  To: Lu Baolu, iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Yu, Fenghua, Jiang,
	Dave, Vinod Koul, Jacob Pan, linux-kernel

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, March 24, 2023 8:03 PM
> 
> PRI is only used for IOPF. With this move, the PCI/PRI feature could be
> controlled by the device driver through iommu_dev_enable/disable_feature()
> interfaces.
> 
> Reviewed-by: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* RE: [PATCH v3 6/6] iommu/vt-d: Remove unnecessary checks in iopf disabling path
  2023-03-24 12:02 ` [PATCH v3 6/6] iommu/vt-d: Remove unnecessary checks in iopf disabling path Lu Baolu
@ 2023-03-28  2:11   ` Tian, Kevin
  0 siblings, 0 replies; 13+ messages in thread
From: Tian, Kevin @ 2023-03-28  2:11 UTC (permalink / raw)
  To: Lu Baolu, iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Yu, Fenghua, Jiang,
	Dave, Vinod Koul, Jacob Pan, linux-kernel

> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Friday, March 24, 2023 8:03 PM
> 
> iommu_unregister_device_fault_handler() and iopf_queue_remove_device()
> are called after device has stopped issuing new page falut requests and
> all outstanding page requests have been drained. They should never fail.
> Trigger a warning if it happens unfortunately.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/6] dmaengine: idxd: Add enable/disable device IOPF feature
  2023-03-24 12:02 ` [PATCH v3 1/6] dmaengine: idxd: Add enable/disable device IOPF feature Lu Baolu
@ 2023-04-03  5:49   ` Baolu Lu
  2023-04-03 16:57     ` Dave Jiang
  0 siblings, 1 reply; 13+ messages in thread
From: Baolu Lu @ 2023-04-03  5:49 UTC (permalink / raw)
  To: iommu, dmaengine
  Cc: baolu.lu, Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian,
	Fenghua Yu, Dave Jiang, Vinod Koul, Jacob Pan, linux-kernel

On 3/24/23 8:02 PM, Lu Baolu wrote:
> The iommu subsystem requires IOMMU_DEV_FEAT_IOPF must be enabled before
> and disabled after IOMMU_DEV_FEAT_SVA, if device's I/O page faults rely
> on the IOMMU. Add explicit IOMMU_DEV_FEAT_IOPF enabling/disabling in this
> driver.
> 
> At present, missing IOPF enabling/disabling doesn't cause any real issue,
> because the IOMMU driver places the IOPF enabling/disabling in the path
> of SVA feature handling. But this may change.
> 
> Reviewed-by: Dave Jiang<dave.jiang@intel.com>
> Reviewed-by: Fenghua Yu<fenghua.yu@intel.com>
> Reviewed-by: Kevin Tian<kevin.tian@intel.com>
> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>

Hi Dave and Fenghua,

The following iommu patches depends on this one. Can I route it to
Linus through the iommu tree?

Best regards,
baolu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/6] dmaengine: idxd: Add enable/disable device IOPF feature
  2023-04-03  5:49   ` Baolu Lu
@ 2023-04-03 16:57     ` Dave Jiang
  2023-04-04  5:19       ` Baolu Lu
  0 siblings, 1 reply; 13+ messages in thread
From: Dave Jiang @ 2023-04-03 16:57 UTC (permalink / raw)
  To: Baolu Lu, iommu, dmaengine
  Cc: Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian, Fenghua Yu,
	Vinod Koul, Jacob Pan, linux-kernel



On 4/2/23 10:49 PM, Baolu Lu wrote:
> On 3/24/23 8:02 PM, Lu Baolu wrote:
>> The iommu subsystem requires IOMMU_DEV_FEAT_IOPF must be enabled before
>> and disabled after IOMMU_DEV_FEAT_SVA, if device's I/O page faults rely
>> on the IOMMU. Add explicit IOMMU_DEV_FEAT_IOPF enabling/disabling in this
>> driver.
>>
>> At present, missing IOPF enabling/disabling doesn't cause any real issue,
>> because the IOMMU driver places the IOPF enabling/disabling in the path
>> of SVA feature handling. But this may change.
>>
>> Reviewed-by: Dave Jiang<dave.jiang@intel.com>
>> Reviewed-by: Fenghua Yu<fenghua.yu@intel.com>
>> Reviewed-by: Kevin Tian<kevin.tian@intel.com>
>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
> 
> Hi Dave and Fenghua,
> 
> The following iommu patches depends on this one. Can I route it to
> Linus through the iommu tree?

Hi Baolu, you'll need an ack from Vinod, who is the dmaengine subsystem 
maintainer. I have no objections.


> 
> Best regards,
> baolu

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v3 1/6] dmaengine: idxd: Add enable/disable device IOPF feature
  2023-04-03 16:57     ` Dave Jiang
@ 2023-04-04  5:19       ` Baolu Lu
  0 siblings, 0 replies; 13+ messages in thread
From: Baolu Lu @ 2023-04-04  5:19 UTC (permalink / raw)
  To: Dave Jiang, iommu, dmaengine
  Cc: baolu.lu, Joerg Roedel, Will Deacon, Robin Murphy, Kevin Tian,
	Fenghua Yu, Vinod Koul, Jacob Pan, linux-kernel

On 4/4/23 12:57 AM, Dave Jiang wrote:
> On 4/2/23 10:49 PM, Baolu Lu wrote:
>> On 3/24/23 8:02 PM, Lu Baolu wrote:
>>> The iommu subsystem requires IOMMU_DEV_FEAT_IOPF must be enabled before
>>> and disabled after IOMMU_DEV_FEAT_SVA, if device's I/O page faults rely
>>> on the IOMMU. Add explicit IOMMU_DEV_FEAT_IOPF enabling/disabling in 
>>> this
>>> driver.
>>>
>>> At present, missing IOPF enabling/disabling doesn't cause any real 
>>> issue,
>>> because the IOMMU driver places the IOPF enabling/disabling in the path
>>> of SVA feature handling. But this may change.
>>>
>>> Reviewed-by: Dave Jiang<dave.jiang@intel.com>
>>> Reviewed-by: Fenghua Yu<fenghua.yu@intel.com>
>>> Reviewed-by: Kevin Tian<kevin.tian@intel.com>
>>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>>
>> Hi Dave and Fenghua,
>>
>> The following iommu patches depends on this one. Can I route it to
>> Linus through the iommu tree?
> 
> Hi Baolu, you'll need an ack from Vinod, who is the dmaengine subsystem 
> maintainer. I have no objections.

Hi Vinod,

Does this work work for you?

Best regards,
baolu

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2023-04-04  5:19 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-24 12:02 [PATCH v3 0/6] iommu/vt-d: Refactor code for non-PRI IOPF Lu Baolu
2023-03-24 12:02 ` [PATCH v3 1/6] dmaengine: idxd: Add enable/disable device IOPF feature Lu Baolu
2023-04-03  5:49   ` Baolu Lu
2023-04-03 16:57     ` Dave Jiang
2023-04-04  5:19       ` Baolu Lu
2023-03-24 12:02 ` [PATCH v3 2/6] iommu/vt-d: Allow SVA with device-specific IOPF Lu Baolu
2023-03-28  2:10   ` Tian, Kevin
2023-03-24 12:02 ` [PATCH v3 3/6] iommu/vt-d: Move iopf code from SVA to IOPF enabling path Lu Baolu
2023-03-24 12:02 ` [PATCH v3 4/6] iommu/vt-d: Move pfsid and ats_qdep calculation to device probe path Lu Baolu
2023-03-24 12:02 ` [PATCH v3 5/6] iommu/vt-d: Move PRI handling to IOPF feature path Lu Baolu
2023-03-28  2:11   ` Tian, Kevin
2023-03-24 12:02 ` [PATCH v3 6/6] iommu/vt-d: Remove unnecessary checks in iopf disabling path Lu Baolu
2023-03-28  2:11   ` Tian, Kevin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).