mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4 0/4] iommu/amd: Refactors for ATS robustness
@ 2026-09-10 14:26 Pranjal Shrivastava
  2026-09-10 14:26 ` [PATCH v4 1/4] iommu/amd: Refactor device probe and capability initialization Pranjal Shrivastava
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Pranjal Shrivastava @ 2026-09-10 14:26 UTC (permalink / raw)
  To: iommu, linux-pci, linux-kernel
  Cc: Joerg Roedel, Will Deacon, Suravee Suthikulpanit, Vasant Hegde,
	Ankit Soni, Jason Gunthorpe, Bjorn Helgaas, Samiullah Khawaja,
	Pranjal Shrivastava

This series forms the second half of the subsystem-wide ATS robustness
updates. The first part (focusing on the core subsystem, Intel, and ARM
SMMUv3) has already been merged upstream [1]. This half addresses the
AMD IOMMU driver and standardizes the PCI ATS API.

In v2, the series has been significantly restructured based on feedback
on v1. 

[v4]
 - https://lore.kernel.org/all/20260824122347.1588592-1-praan@google.com/
 - Dropped iommu_ignore_device() and dropped patch 3 per discussion with
   Jason and Vasant.
 - Renamed iommu_lookup_device() to lookup_device().
 - Cleaned up amd_iommu_probe_device() to return error pointers directly.
 - Collected Reviewed-by tags from Jason, Vasant and Sami.

[v3]
 - Collected Reviewed-by tags from Vasant.
 - Added a comment explaining why devices without DMA translation support
   are not ignored (to preserve interrupt remapping) per Vasant's feedback.
 - Renamed iommu_disable_device_dma() to iommu_disable_device() to
   accurately reflect that it disables both DMA and interrupt remapping.

[v2]
 - https://lore.kernel.org/all/20260814015647.3370124-1-praan@google.com/
 - Patch 1 Refctors the probe path, isolating capabilities into 
   iommu_init_device_caps().
 - Patch 2 renames iommu_ignore_device() to iommu_disable_device_dma().
   Following Jason's suggestion, it invalidates the hardware DTE by
   clearing the Valid bit (lower 128 bits) followed by the upper
   128 bits.  
 - Patch 3 splits the probe error paths to ensure that devices with
   config failures (like PD_MODE_NONE or ATS mismatches) can preserve
   their rlookup_table entries, successfully keeping IRQ remapping 
   functional for bypassed devices.
 - Patch 4 implements the "Fail Hard" pattern for ATS in the AMD driver,
   failing the probe and throwing a WARN_ON() upon ATS configuration or
   enablement failures.
 - Patch 5 enforces the checking of pci_ats_supported() prior to calling
   pci_prepare_ats() across the entire kernel PCI subsystem.

[v1]
 - https://lore.kernel.org/all/20260601134204.2150602-1-praan@google.com/

Thanks,
Praan

[1] https://lore.kernel.org/all/20260615235037.259909-1-praan@google.com/

Pranjal Shrivastava (4):
  iommu/amd: Refactor device probe and capability initialization
  iommu/amd: Remove iommu_ignore_device()
  iommu/amd: Fail probe on ATS configuration failure
  PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats()

 drivers/iommu/amd/iommu.c | 164 +++++++++++++++++++-------------------
 drivers/pci/ats.c         |   6 +-
 2 files changed, 86 insertions(+), 84 deletions(-)

-- 
2.55.0.1003.g10538fe699-goog


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

* [PATCH v4 1/4] iommu/amd: Refactor device probe and capability initialization
  2026-09-10 14:26 [PATCH v4 0/4] iommu/amd: Refactors for ATS robustness Pranjal Shrivastava
@ 2026-09-10 14:26 ` Pranjal Shrivastava
  2026-09-10 14:26 ` [PATCH v4 2/4] iommu/amd: Remove iommu_ignore_device() Pranjal Shrivastava
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Pranjal Shrivastava @ 2026-09-10 14:26 UTC (permalink / raw)
  To: iommu, linux-pci, linux-kernel
  Cc: Joerg Roedel, Will Deacon, Suravee Suthikulpanit, Vasant Hegde,
	Ankit Soni, Jason Gunthorpe, Bjorn Helgaas, Samiullah Khawaja,
	Pranjal Shrivastava

Restructure the device probe path to improve readability and prepare for
cleaner error handling. Refactor check_device() into lookup_device to
explicitly validate and return the amd_iommu ptr & devid. Refactor
iommu_init_device() to return the allocated dev_data. Consolidate all
PCI cap inits (MSI domains, PASID, ATS) into a new helper:
iommu_init_device_caps().

Suggested-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/amd/iommu.c | 117 +++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 58 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 4dc306a4b5c6..5e144a60c397 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -675,7 +675,8 @@ static void pdev_disable_caps(struct pci_dev *pdev)
  * This function checks if the driver got a valid device from the caller to
  * avoid dereferencing invalid pointers.
  */
-static bool check_device(struct device *dev)
+static bool lookup_device(struct device *dev,
+				struct amd_iommu **iommu_out, u16 *devid_out)
 {
 	struct amd_iommu_pci_seg *pci_seg;
 	struct amd_iommu *iommu;
@@ -690,7 +691,7 @@ static bool check_device(struct device *dev)
 	devid = PCI_SBDF_TO_DEVID(sbdf);
 
 	iommu = rlookup_amd_iommu(dev);
-	if (!iommu)
+	if (!iommu || !iommu->iommu.ops)
 		return false;
 
 	/* Out of our scope? */
@@ -698,47 +699,33 @@ static bool check_device(struct device *dev)
 	if (devid > pci_seg->last_bdf)
 		return false;
 
+	*iommu_out = iommu;
+	*devid_out = devid;
 	return true;
 }
 
-static int iommu_init_device(struct amd_iommu *iommu, struct device *dev)
+static struct iommu_dev_data *iommu_init_device(struct amd_iommu *iommu,
+						struct device *dev, u16 devid)
 {
 	struct iommu_dev_data *dev_data;
-	int devid, sbdf;
-
-	if (dev_iommu_priv_get(dev))
-		return 0;
 
-	sbdf = get_device_sbdf_id(dev);
-	if (sbdf < 0)
-		return sbdf;
-
-	devid = PCI_SBDF_TO_DEVID(sbdf);
 	dev_data = find_dev_data(iommu, devid);
 	if (!dev_data)
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 
 	dev_data->dev = dev;
 
 	/*
-	 * The dev_iommu_priv_set() needes to be called before setup_aliases.
+	 * The dev_iommu_priv_set() needs to be called before setup_aliases.
 	 * Otherwise, subsequent call to dev_iommu_priv_get() will fail.
 	 */
 	dev_iommu_priv_set(dev, dev_data);
 	setup_aliases(iommu, dev);
 
-	/*
-	 * By default we use passthrough mode for IOMMUv2 capable device.
-	 * But if amd_iommu=force_isolation is set (e.g. to debug DMA to
-	 * invalid address), we ignore the capability for the device so
-	 * it'll be forced to go into translation mode.
-	 */
-	if ((iommu_default_passthrough() || !amd_iommu_force_isolation) &&
-	    dev_is_pci(dev) && amd_iommu_gt_ppr_supported()) {
-		dev_data->flags = pdev_get_caps(to_pci_dev(dev));
-	}
+	/* Wait for DTE updates to go through */
+	iommu_completion_wait(iommu);
 
-	return 0;
+	return dev_data;
 }
 
 static void iommu_ignore_device(struct amd_iommu *iommu, struct device *dev)
@@ -2487,49 +2474,73 @@ static void detach_device(struct device *dev)
 	mutex_unlock(&dev_data->mutex);
 }
 
+static void iommu_init_device_caps(struct iommu_dev_data *dev_data,
+				   struct device *dev,
+				   struct amd_iommu *iommu)
+{
+	if (FEATURE_NUM_INT_REMAP_SUP_2K(amd_iommu_efr2))
+		dev_data->max_irqs = MAX_IRQS_PER_TABLE_2K;
+	else
+		dev_data->max_irqs = MAX_IRQS_PER_TABLE_512;
+
+	amd_iommu_set_pci_msi_domain(dev, iommu);
+
+	if (!dev_is_pci(dev))
+		return;
+
+	/*
+	 * By default we use passthrough mode for IOMMUv2 capable device.
+	 * But if amd_iommu=force_isolation is set (e.g. to debug DMA to
+	 * invalid address), we ignore the capability for the device so
+	 * it'll be forced to go into translation mode.
+	 */
+	if ((iommu_default_passthrough() || !amd_iommu_force_isolation) &&
+	    amd_iommu_gt_ppr_supported()) {
+		dev_data->flags = pdev_get_caps(to_pci_dev(dev));
+	}
+
+	/*
+	 * If IOMMU and device supports PASID then it will contain max
+	 * supported PASIDs, else it will be zero.
+	 */
+	if (amd_iommu_pasid_supported() &&
+	    pdev_pasid_supported(dev_data)) {
+		dev_data->max_pasids = min_t(u32, iommu->iommu.max_pasids,
+					     pci_max_pasids(to_pci_dev(dev)));
+	}
+
+	pci_prepare_ats(to_pci_dev(dev), PAGE_SHIFT);
+}
+
 static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 {
 	struct iommu_device *iommu_dev;
 	struct amd_iommu *iommu;
 	struct iommu_dev_data *dev_data;
-	int ret;
-
-	if (!check_device(dev))
-		return ERR_PTR(-ENODEV);
-
-	iommu = rlookup_amd_iommu(dev);
-	if (!iommu)
-		return ERR_PTR(-ENODEV);
+	u16 devid;
 
-	/* Not registered yet? */
-	if (!iommu->iommu.ops)
+	if (!lookup_device(dev, &iommu, &devid))
 		return ERR_PTR(-ENODEV);
 
 	if (dev_iommu_priv_get(dev))
 		return &iommu->iommu;
 
-	ret = iommu_init_device(iommu, dev);
-	if (ret) {
+	dev_data = iommu_init_device(iommu, dev, devid);
+	if (IS_ERR(dev_data)) {
 		dev_err(dev, "Failed to initialize - trying to proceed anyway\n");
-		iommu_dev = ERR_PTR(ret);
+		iommu_dev = ERR_CAST(dev_data);
 		iommu_ignore_device(iommu, dev);
 		goto out_err;
 	}
 
-	amd_iommu_set_pci_msi_domain(dev, iommu);
+	iommu_init_device_caps(dev_data, dev, iommu);
 	iommu_dev = &iommu->iommu;
 
 	/*
-	 * If IOMMU and device supports PASID then it will contain max
-	 * supported PASIDs, else it will be zero.
+	 * When DMA translation is unavailable return error so the iommu core
+	 * won't attempt domain attach for this device, while preserving its
+	 * rlookup entry for interrupt remapping.
 	 */
-	dev_data = dev_iommu_priv_get(dev);
-	if (amd_iommu_pasid_supported() && dev_is_pci(dev) &&
-	    pdev_pasid_supported(dev_data)) {
-		dev_data->max_pasids = min_t(u32, iommu->iommu.max_pasids,
-					     pci_max_pasids(to_pci_dev(dev)));
-	}
-
 	if (amd_iommu_pgtable == PD_MODE_NONE) {
 		pr_warn_once("%s: DMA translation not supported by iommu.\n",
 			     __func__);
@@ -2537,16 +2548,6 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 		goto out_err;
 	}
 
-	iommu_completion_wait(iommu);
-
-	if (FEATURE_NUM_INT_REMAP_SUP_2K(amd_iommu_efr2))
-		dev_data->max_irqs = MAX_IRQS_PER_TABLE_2K;
-	else
-		dev_data->max_irqs = MAX_IRQS_PER_TABLE_512;
-
-	if (dev_is_pci(dev))
-		pci_prepare_ats(to_pci_dev(dev), PAGE_SHIFT);
-
 out_err:
 	return iommu_dev;
 }
-- 
2.55.0.1003.g10538fe699-goog


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

* [PATCH v4 2/4] iommu/amd: Remove iommu_ignore_device()
  2026-09-10 14:26 [PATCH v4 0/4] iommu/amd: Refactors for ATS robustness Pranjal Shrivastava
  2026-09-10 14:26 ` [PATCH v4 1/4] iommu/amd: Refactor device probe and capability initialization Pranjal Shrivastava
@ 2026-09-10 14:26 ` Pranjal Shrivastava
  2026-09-10 14:40   ` Jason Gunthorpe
  2026-09-11  4:26   ` Vasant Hegde
  2026-09-10 14:26 ` [PATCH v4 3/4] iommu/amd: Fail probe on ATS configuration failure Pranjal Shrivastava
  2026-09-10 14:26 ` [PATCH v4 4/4] PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats() Pranjal Shrivastava
  3 siblings, 2 replies; 7+ messages in thread
From: Pranjal Shrivastava @ 2026-09-10 14:26 UTC (permalink / raw)
  To: iommu, linux-pci, linux-kernel
  Cc: Joerg Roedel, Will Deacon, Suravee Suthikulpanit, Vasant Hegde,
	Ankit Soni, Jason Gunthorpe, Bjorn Helgaas, Samiullah Khawaja,
	Pranjal Shrivastava, sashiko-bot

The iommu_ignore_device() helper was historically called on device
initialization failure to clear the primary Device Table Entry (DTE) via
memset() and nullify the rlookup_table entry.

However, clearing the DTE on probe failure is problematic:
1. During normal boot, DTEs start out unconfigured (blocking DMA), making
   clearing redundant.
2. During kdump boot, pre-existing translations should be kept running
   until deferred attach rather than abruptly clearing them, which risks
   breaking in-flight transfers.
3. Writing to the DTE table via memset without flushing the hardware DTE
   cache risks aliasing & torn writes.
4. Clearing the rlookup_table entry breaks interrupt remapping for
   devices that fail probe or operate in translation-less modes.

Remove iommu_ignore_device() entirely and simplify the error return
paths in amd_iommu_probe_device().

Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Suggested-by: Vasant Hegde <vasant.hegde@amd.com>
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260529153216.2AD1E1F00899@smtp.kernel.org/
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/amd/iommu.c | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 5e144a60c397..7f8b51c28a7e 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -728,22 +728,6 @@ static struct iommu_dev_data *iommu_init_device(struct amd_iommu *iommu,
 	return dev_data;
 }
 
-static void iommu_ignore_device(struct amd_iommu *iommu, struct device *dev)
-{
-	struct amd_iommu_pci_seg *pci_seg = iommu->pci_seg;
-	struct dev_table_entry *dev_table = get_dev_table(iommu);
-	int devid, sbdf;
-
-	sbdf = get_device_sbdf_id(dev);
-	if (sbdf < 0)
-		return;
-
-	devid = PCI_SBDF_TO_DEVID(sbdf);
-	pci_seg->rlookup_table[devid] = NULL;
-	memset(&dev_table[devid], 0, sizeof(struct dev_table_entry));
-
-	setup_aliases(iommu, dev);
-}
 
 
 /****************************************************************************
@@ -2528,9 +2512,7 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 	dev_data = iommu_init_device(iommu, dev, devid);
 	if (IS_ERR(dev_data)) {
 		dev_err(dev, "Failed to initialize - trying to proceed anyway\n");
-		iommu_dev = ERR_CAST(dev_data);
-		iommu_ignore_device(iommu, dev);
-		goto out_err;
+		return ERR_CAST(dev_data);
 	}
 
 	iommu_init_device_caps(dev_data, dev, iommu);
@@ -2544,11 +2526,9 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 	if (amd_iommu_pgtable == PD_MODE_NONE) {
 		pr_warn_once("%s: DMA translation not supported by iommu.\n",
 			     __func__);
-		iommu_dev = ERR_PTR(-ENODEV);
-		goto out_err;
+		return ERR_PTR(-ENODEV);
 	}
 
-out_err:
 	return iommu_dev;
 }
 
-- 
2.55.0.1003.g10538fe699-goog


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

* [PATCH v4 3/4] iommu/amd: Fail probe on ATS configuration failure
  2026-09-10 14:26 [PATCH v4 0/4] iommu/amd: Refactors for ATS robustness Pranjal Shrivastava
  2026-09-10 14:26 ` [PATCH v4 1/4] iommu/amd: Refactor device probe and capability initialization Pranjal Shrivastava
  2026-09-10 14:26 ` [PATCH v4 2/4] iommu/amd: Remove iommu_ignore_device() Pranjal Shrivastava
@ 2026-09-10 14:26 ` Pranjal Shrivastava
  2026-09-10 14:26 ` [PATCH v4 4/4] PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats() Pranjal Shrivastava
  3 siblings, 0 replies; 7+ messages in thread
From: Pranjal Shrivastava @ 2026-09-10 14:26 UTC (permalink / raw)
  To: iommu, linux-pci, linux-kernel
  Cc: Joerg Roedel, Will Deacon, Suravee Suthikulpanit, Vasant Hegde,
	Ankit Soni, Jason Gunthorpe, Bjorn Helgaas, Samiullah Khawaja,
	Pranjal Shrivastava

Update the driver to call pci_prepare_ats() after checking if
pci_ats_supported() and fail the probe_device if pci_prepare_ats()
returns an error. Additionally, update pdev_enable_cap_ats() to WARN_ON()
a failure in pci_enable_ats().

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/amd/iommu.c | 39 +++++++++++++++++++++++++++++----------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 7f8b51c28a7e..195097365413 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -573,10 +573,17 @@ static inline int pdev_enable_cap_ats(struct pci_dev *pdev)
 	if (amd_iommu_iotlb_sup &&
 	    (dev_data->flags & AMD_IOMMU_DEVICE_FLAG_ATS_SUP)) {
 		ret = pci_enable_ats(pdev, PAGE_SHIFT);
-		if (!ret) {
-			dev_data->ats_enabled = 1;
-			dev_data->ats_qdep    = pci_ats_queue_depth(pdev);
-		}
+
+		/*
+		 * pci_enable_ats() should not fail here because earlier
+		 * checks have already verified support & config.
+		 */
+		if (WARN_ON(ret))
+			return ret;
+
+		dev_data->ats_enabled = 1;
+		dev_data->ats_qdep    = pci_ats_queue_depth(pdev);
+		ret = 0;
 	}
 
 	return ret;
@@ -2458,10 +2465,12 @@ static void detach_device(struct device *dev)
 	mutex_unlock(&dev_data->mutex);
 }
 
-static void iommu_init_device_caps(struct iommu_dev_data *dev_data,
-				   struct device *dev,
-				   struct amd_iommu *iommu)
+static int iommu_init_device_caps(struct iommu_dev_data *dev_data,
+				  struct device *dev,
+				  struct amd_iommu *iommu)
 {
+	int ret;
+
 	if (FEATURE_NUM_INT_REMAP_SUP_2K(amd_iommu_efr2))
 		dev_data->max_irqs = MAX_IRQS_PER_TABLE_2K;
 	else
@@ -2470,7 +2479,7 @@ static void iommu_init_device_caps(struct iommu_dev_data *dev_data,
 	amd_iommu_set_pci_msi_domain(dev, iommu);
 
 	if (!dev_is_pci(dev))
-		return;
+		return 0;
 
 	/*
 	 * By default we use passthrough mode for IOMMUv2 capable device.
@@ -2493,7 +2502,13 @@ static void iommu_init_device_caps(struct iommu_dev_data *dev_data,
 					     pci_max_pasids(to_pci_dev(dev)));
 	}
 
-	pci_prepare_ats(to_pci_dev(dev), PAGE_SHIFT);
+	if (pci_ats_supported(to_pci_dev(dev))) {
+		ret = pci_prepare_ats(to_pci_dev(dev), PAGE_SHIFT);
+		if (ret)
+			return ret;
+	}
+
+	return 0;
 }
 
 static struct iommu_device *amd_iommu_probe_device(struct device *dev)
@@ -2502,6 +2517,7 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 	struct amd_iommu *iommu;
 	struct iommu_dev_data *dev_data;
 	u16 devid;
+	int ret;
 
 	if (!lookup_device(dev, &iommu, &devid))
 		return ERR_PTR(-ENODEV);
@@ -2515,7 +2531,10 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 		return ERR_CAST(dev_data);
 	}
 
-	iommu_init_device_caps(dev_data, dev, iommu);
+	ret = iommu_init_device_caps(dev_data, dev, iommu);
+	if (ret)
+		return ERR_PTR(ret);
+
 	iommu_dev = &iommu->iommu;
 
 	/*
-- 
2.55.0.1003.g10538fe699-goog


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

* [PATCH v4 4/4] PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats()
  2026-09-10 14:26 [PATCH v4 0/4] iommu/amd: Refactors for ATS robustness Pranjal Shrivastava
                   ` (2 preceding siblings ...)
  2026-09-10 14:26 ` [PATCH v4 3/4] iommu/amd: Fail probe on ATS configuration failure Pranjal Shrivastava
@ 2026-09-10 14:26 ` Pranjal Shrivastava
  3 siblings, 0 replies; 7+ messages in thread
From: Pranjal Shrivastava @ 2026-09-10 14:26 UTC (permalink / raw)
  To: iommu, linux-pci, linux-kernel
  Cc: Joerg Roedel, Will Deacon, Suravee Suthikulpanit, Vasant Hegde,
	Ankit Soni, Jason Gunthorpe, Bjorn Helgaas, Samiullah Khawaja,
	Pranjal Shrivastava, Baolu Lu, Nicolin Chen

Currently, pci_prepare_ats() internally calls pci_ats_supported() and
returns -EINVAL if the device does not support ATS. While this provides
a silent safety check, it conflates support detection with configuration.

Update pci_prepare_ats() to wrap the internal pci_ats_supported check in
a WARN_ON(). This mandates all callers to call pci_prepare_ats() only if
the function supports ATS.

Update the function documentation to mention that callers must verify
ATS support (via pci_ats_supported()) before calling pci_prepare_ats().

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
Suggested-by: Baolu Lu <baolu.lu@linux.intel.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/pci/ats.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 9cb23780093d..f1434f86ac40 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -56,7 +56,9 @@ EXPORT_SYMBOL_GPL(pci_ats_supported);
  * @ps: the IOMMU page shift
  *
  * This must be done by the IOMMU driver on the PF before any VFs are created to
- * ensure that the VF can have ATS enabled.
+ * ensure that the VF can have ATS enabled. Callers must verify that ATS is
+ * supported by the device (e.g. via pci_ats_supported()) before calling this
+ * function.
  *
  * Returns 0 on success, or negative on failure.
  */
@@ -64,7 +66,7 @@ int pci_prepare_ats(struct pci_dev *dev, int ps)
 {
 	u16 ctrl;
 
-	if (!pci_ats_supported(dev))
+	if (WARN_ON(!pci_ats_supported(dev)))
 		return -EINVAL;
 
 	if (WARN_ON(dev->ats_enabled))
-- 
2.55.0.1003.g10538fe699-goog


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

* Re: [PATCH v4 2/4] iommu/amd: Remove iommu_ignore_device()
  2026-09-10 14:26 ` [PATCH v4 2/4] iommu/amd: Remove iommu_ignore_device() Pranjal Shrivastava
@ 2026-09-10 14:40   ` Jason Gunthorpe
  2026-09-11  4:26   ` Vasant Hegde
  1 sibling, 0 replies; 7+ messages in thread
From: Jason Gunthorpe @ 2026-09-10 14:40 UTC (permalink / raw)
  To: Pranjal Shrivastava
  Cc: iommu, linux-pci, linux-kernel, Joerg Roedel, Will Deacon,
	Suravee Suthikulpanit, Vasant Hegde, Ankit Soni, Bjorn Helgaas,
	Samiullah Khawaja, sashiko-bot

On Thu, Sep 10, 2026 at 02:26:53PM +0000, Pranjal Shrivastava wrote:
> The iommu_ignore_device() helper was historically called on device
> initialization failure to clear the primary Device Table Entry (DTE) via
> memset() and nullify the rlookup_table entry.
> 
> However, clearing the DTE on probe failure is problematic:
> 1. During normal boot, DTEs start out unconfigured (blocking DMA), making
>    clearing redundant.
> 2. During kdump boot, pre-existing translations should be kept running
>    until deferred attach rather than abruptly clearing them, which risks
>    breaking in-flight transfers.
> 3. Writing to the DTE table via memset without flushing the hardware DTE
>    cache risks aliasing & torn writes.
> 4. Clearing the rlookup_table entry breaks interrupt remapping for
>    devices that fail probe or operate in translation-less modes.
> 
> Remove iommu_ignore_device() entirely and simplify the error return
> paths in amd_iommu_probe_device().
> 
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Suggested-by: Vasant Hegde <vasant.hegde@amd.com>
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/all/20260529153216.2AD1E1F00899@smtp.kernel.org/
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
>  drivers/iommu/amd/iommu.c | 24 ++----------------------
>  1 file changed, 2 insertions(+), 22 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason

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

* Re: [PATCH v4 2/4] iommu/amd: Remove iommu_ignore_device()
  2026-09-10 14:26 ` [PATCH v4 2/4] iommu/amd: Remove iommu_ignore_device() Pranjal Shrivastava
  2026-09-10 14:40   ` Jason Gunthorpe
@ 2026-09-11  4:26   ` Vasant Hegde
  1 sibling, 0 replies; 7+ messages in thread
From: Vasant Hegde @ 2026-09-11  4:26 UTC (permalink / raw)
  To: Pranjal Shrivastava, iommu, linux-pci, linux-kernel
  Cc: Joerg Roedel, Will Deacon, Suravee Suthikulpanit, Ankit Soni,
	Jason Gunthorpe, Bjorn Helgaas, Samiullah Khawaja, sashiko-bot



On 9/10/2026 7:56 PM, Pranjal Shrivastava wrote:
> The iommu_ignore_device() helper was historically called on device
> initialization failure to clear the primary Device Table Entry (DTE) via
> memset() and nullify the rlookup_table entry.
> 
> However, clearing the DTE on probe failure is problematic:
> 1. During normal boot, DTEs start out unconfigured (blocking DMA), making
>    clearing redundant.
> 2. During kdump boot, pre-existing translations should be kept running
>    until deferred attach rather than abruptly clearing them, which risks
>    breaking in-flight transfers.
> 3. Writing to the DTE table via memset without flushing the hardware DTE
>    cache risks aliasing & torn writes.
> 4. Clearing the rlookup_table entry breaks interrupt remapping for
>    devices that fail probe or operate in translation-less modes.
> 
> Remove iommu_ignore_device() entirely and simplify the error return
> paths in amd_iommu_probe_device().
> 
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Suggested-by: Vasant Hegde <vasant.hegde@amd.com>
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/all/20260529153216.2AD1E1F00899@smtp.kernel.org/
> Signed-off-by: Pranjal Shrivastava <praan@google.com>

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>

-Vasant


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

end of thread, other threads:[~2026-09-11  4:27 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-10 14:26 [PATCH v4 0/4] iommu/amd: Refactors for ATS robustness Pranjal Shrivastava
2026-09-10 14:26 ` [PATCH v4 1/4] iommu/amd: Refactor device probe and capability initialization Pranjal Shrivastava
2026-09-10 14:26 ` [PATCH v4 2/4] iommu/amd: Remove iommu_ignore_device() Pranjal Shrivastava
2026-09-10 14:40   ` Jason Gunthorpe
2026-09-11  4:26   ` Vasant Hegde
2026-09-10 14:26 ` [PATCH v4 3/4] iommu/amd: Fail probe on ATS configuration failure Pranjal Shrivastava
2026-09-10 14:26 ` [PATCH v4 4/4] PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats() Pranjal Shrivastava

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®