[PATCH v3 0/5] iommu/amd: Refactors for ATS robustness

[PATCH v3 0/5] iommu/amd: Refactors for ATS robustness

[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. 

[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 (5):
  iommu/amd: Refactor device probe and capability initialization
  iommu/amd: Fix DTE clearing and rename iommu_ignore_device()
  iommu/amd: Split probe error paths to preserve IRQ remapping
  iommu/amd: Fail probe on ATS configuration failure
  PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats()

 drivers/iommu/amd/iommu.c | 199 +++++++++++++++++++++++---------------
 drivers/pci/ats.c         |   6 +-
 2 files changed, 126 insertions(+), 79 deletions(-)

-- 
2.55.0.766.g2966f0265a-goog

[PATCH v3 1/5] iommu/amd: Refactor device probe and capability initialization

[Pranjal Shrivastava]

Restructure the device probe path to improve readability and prepare for
cleaner error handling. Refactor check_device() into iommu_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>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/amd/iommu.c | 117 +++++++++++++++++++-------------------
 1 file changed, 60 insertions(+), 57 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 4c31294fabc5..e1f9fbb63837 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 iommu_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)
@@ -2477,49 +2464,75 @@ 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;
+	u16 devid;
 
-	if (!check_device(dev))
-		return ERR_PTR(-ENODEV);
-
-	iommu = rlookup_amd_iommu(dev);
-	if (!iommu)
-		return ERR_PTR(-ENODEV);
-
-	/* Not registered yet? */
-	if (!iommu->iommu.ops)
+	if (!iommu_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)) {
+		ret = PTR_ERR(dev_data);
 		dev_err(dev, "Failed to initialize - trying to proceed anyway\n");
 		iommu_dev = ERR_PTR(ret);
 		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. But interrupt-remap
+	 * is still supported. Hence do not ignore the device.
 	 */
-	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__);
@@ -2527,16 +2540,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.766.g2966f0265a-goog

[PATCH v3 2/5] iommu/amd: Fix DTE clearing and rename iommu_ignore_device()

[Pranjal Shrivastava]

The iommu_ignore_device() function currently uses memset() to manually
clear the primary Device Table Entry (DTE), which risks torn writes as
the hardware reads DTEs as atomic 256-bit qwords. Furthermore, clearing
the primary devid in the lookup table before calling setup_aliases()
causes rlookup_amd_iommu() to fail for aliases. This prevents clearing
the DTEs for DMA aliases.

Fix this by replacing the manual memset with a dedicated helper that
invalidates the DTE by clearing the lower 128 bits (having the Valid bit)
first, followed by the upper 128 bits. The cleared state is then
explicitly cloned to all aliases before the lookup tables are nullified.

Rename the function to iommu_disable_device() more accurately
reflects its intent, as we still support IRQ remapping for these devices)

Fixes: 99fc4ac3d297 ("iommu/amd: Introduce per PCI segment alias_table")
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260529153216.2AD1E1F00899@smtp.kernel.org/
Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/amd/iommu.c | 53 ++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 17 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index e1f9fbb63837..8e382317709d 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);
-}
 
 
 /****************************************************************************
@@ -2233,6 +2217,41 @@ static void dev_update_dte(struct iommu_dev_data *dev_data, bool set)
 		clear_dte_entry(iommu, dev_data);
 }
 
+/*
+ * Invalidate a DTE by clearing the Valid bit first.
+ * Note: Lockless; Not to be used on a fully probed
+ * device with live dev_data.
+ */
+static void amd_iommu_disable_dte(struct dev_table_entry *ptr)
+{
+	struct dev_table_entry new = {};
+
+	write_dte_lower128(ptr, &new);
+	write_dte_upper128(ptr, &new);
+}
+
+static void iommu_disable_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);
+
+	/* Clear the primary DTE */
+	amd_iommu_disable_dte(&dev_table[devid]);
+
+	/* Clone the cleared DTE to all aliases before wiping the rlookup */
+	if (dev_iommu_priv_get(dev))
+		clone_aliases(iommu, dev);
+
+	pci_seg->rlookup_table[devid] = NULL;
+}
+
 /*
  * If domain is SVA capable then initialize GCR3 table. Also if domain is
  * in v2 page table mode then update GCR3[0].
@@ -2521,7 +2540,7 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 		ret = PTR_ERR(dev_data);
 		dev_err(dev, "Failed to initialize - trying to proceed anyway\n");
 		iommu_dev = ERR_PTR(ret);
-		iommu_ignore_device(iommu, dev);
+		iommu_disable_device(iommu, dev);
 		goto out_err;
 	}
 
-- 
2.55.0.766.g2966f0265a-goog

[PATCH v3 3/5] iommu/amd: Split probe error paths to preserve IRQ remapping

[Pranjal Shrivastava]

Split the amd_iommu_probe_device() error paths into err_deinit and
out_err. Proper init failures continue to call iommu_disable_device_dma()
while configuration failures (like PD_MODE_NONE or ATS mismatches) skip
it to preserve the rlookup_table entry required for IRQ remapping.

Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260529153216.2AD1E1F00899@smtp.kernel.org/
Suggested-by: Ankit Soni <ankit.soni@amd.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/amd/iommu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 8e382317709d..ed336bca1a2e 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2540,8 +2540,7 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 		ret = PTR_ERR(dev_data);
 		dev_err(dev, "Failed to initialize - trying to proceed anyway\n");
 		iommu_dev = ERR_PTR(ret);
-		iommu_disable_device(iommu, dev);
-		goto out_err;
+		goto err_deinit;
 	}
 
 	iommu_init_device_caps(dev_data, dev, iommu);
@@ -2559,6 +2558,10 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 		goto out_err;
 	}
 
+	return iommu_dev;
+
+err_deinit:
+	iommu_disable_device(iommu, dev);
 out_err:
 	return iommu_dev;
 }
-- 
2.55.0.766.g2966f0265a-goog

[PATCH v3 4/5] iommu/amd: Fail probe on ATS configuration failure

[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>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/amd/iommu.c | 40 +++++++++++++++++++++++++++++----------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index ed336bca1a2e..ecee4d097e4e 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;
@@ -2483,10 +2490,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
@@ -2495,7 +2504,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.
@@ -2518,7 +2527,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)
@@ -2543,7 +2558,12 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
 		goto err_deinit;
 	}
 
-	iommu_init_device_caps(dev_data, dev, iommu);
+	ret = iommu_init_device_caps(dev_data, dev, iommu);
+	if (ret) {
+		iommu_dev = ERR_PTR(ret);
+		goto out_err;
+	}
+
 	iommu_dev = &iommu->iommu;
 	
 	/*
-- 
2.55.0.766.g2966f0265a-goog

[PATCH v3 5/5] PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats()

[Pranjal Shrivastava]

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>
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 96efa00d9743..f18c06b73eb2 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -53,7 +53,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.
  */
@@ -61,7 +63,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.766.g2966f0265a-goog

Re: [PATCH v3 5/5] PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats()

[Jason Gunthorpe]

On Mon, 24 Aug 2026 12:23:47 +0000, Pranjal Shrivastava <praan@google.com> wrote:
> 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.
> 
> [...]

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

-- 
Jason

Re: [PATCH v3 3/5] iommu/amd: Split probe error paths to preserve IRQ remapping

[Jason Gunthorpe]

> [ ... 15 lines skipped ... ]
> @@ -2540,8 +2540,7 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
>  		ret = PTR_ERR(dev_data);
>  		dev_err(dev, "Failed to initialize - trying to proceed anyway\n");
>  		iommu_dev = ERR_PTR(ret);

I would drop this line and fix the return to have the ERR_PTR

> +err_deinit:
> +	iommu_disable_device(iommu, dev);
>  out_err:
>  	return iommu_dev;

Otherwise this landmine is going to hurt someday since there is only
one flow that would have an err ptr in iommu_dev.

-- 
Jason

Re: [PATCH v3 2/5] iommu/amd: Fix DTE clearing and rename iommu_ignore_device()

[Jason Gunthorpe]

> [ ... 52 lines skipped ... ]
> +/*
> + * Invalidate a DTE by clearing the Valid bit first.
> + * Note: Lockless; Not to be used on a fully probed
> + * device with live dev_data.
> + */
> +static void amd_iommu_disable_dte(struct dev_table_entry *ptr)
> +{
> +	struct dev_table_entry new = {};
> +
> +	write_dte_lower128(ptr, &new);
> +	write_dte_upper128(ptr, &new);
> +}

No need just call update_dte256() with a 0'd new. This is how all the
update flows work, and it flushes the DTE which this looks like it has
been missing all long.

-- 
Jason

Re: [PATCH v3 0/5] iommu/amd: Refactors for ATS robustness

[Jason Gunthorpe]

> [ ... 41 lines skipped ... ]
> Pranjal Shrivastava (5):
>   iommu/amd: Refactor device probe and capability initialization
>   iommu/amd: Fix DTE clearing and rename iommu_ignore_device()
>   iommu/amd: Split probe error paths to preserve IRQ remapping
>   iommu/amd: Fail probe on ATS configuration failure
>   PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats()
> 
>  drivers/iommu/amd/iommu.c | 199 +++++++++++++++++++++++---------------
>  drivers/pci/ats.c         |   6 +-
>  2 files changed, 126 insertions(+), 79 deletions(-)

FWIW something about this series is mangled, the first patch was
changed before it made it to lore. Maybe whitespace damage..

-- 
Jason

Re: [PATCH v3 4/5] iommu/amd: Fail probe on ATS configuration failure

[Jason Gunthorpe]

On Mon, 24 Aug 2026 12:23:46 +0000, Pranjal Shrivastava <praan@google.com> wrote:
> 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: Jason Gunthorpe <jgg@nvidia.com>

-- 
Jason

Re: [PATCH v3 1/5] iommu/amd: Refactor device probe and capability initialization

[Jason Gunthorpe]

> [ ... 14 lines skipped ... ]
> @@ -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 iommu_lookup_device(struct device *dev,
> +				struct amd_iommu **iommu_out, u16 *devid_out)
>  {

If we are touching the names driver functions should not be called
iommu_*, it is confusing..

Otherwise looks OK

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

-- 
Jason

Re: [PATCH v3 4/5] iommu/amd: Fail probe on ATS configuration failure

[Samiullah Khawaja]

On Mon, Aug 24, 2026 at 12:23:46PM +0000, Pranjal Shrivastava wrote:
>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>
>Signed-off-by: Pranjal Shrivastava <praan@google.com>
>---
> drivers/iommu/amd/iommu.c | 40 +++++++++++++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 10 deletions(-)
>

Reviewed-by: Samiullah Khawaja <skhawaja@google.com>

Re: [PATCH v3 2/5] iommu/amd: Fix DTE clearing and rename iommu_ignore_device()

[Samiullah Khawaja]

On Mon, Aug 24, 2026 at 12:23:44PM +0000, Pranjal Shrivastava wrote:
>The iommu_ignore_device() function currently uses memset() to manually
>clear the primary Device Table Entry (DTE), which risks torn writes as
>the hardware reads DTEs as atomic 256-bit qwords. Furthermore, clearing
>the primary devid in the lookup table before calling setup_aliases()
>causes rlookup_amd_iommu() to fail for aliases. This prevents clearing
>the DTEs for DMA aliases.
>
>Fix this by replacing the manual memset with a dedicated helper that
>invalidates the DTE by clearing the lower 128 bits (having the Valid bit)
>first, followed by the upper 128 bits. The cleared state is then
>explicitly cloned to all aliases before the lookup tables are nullified.
>
>Rename the function to iommu_disable_device() more accurately
>reflects its intent, as we still support IRQ remapping for these devices)
>
>Fixes: 99fc4ac3d297 ("iommu/amd: Introduce per PCI segment alias_table")
>Reported-by: sashiko-bot@kernel.org
>Closes: https://lore.kernel.org/all/20260529153216.2AD1E1F00899@smtp.kernel.org/
>Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
>Signed-off-by: Pranjal Shrivastava <praan@google.com>
>---
> drivers/iommu/amd/iommu.c | 53 ++++++++++++++++++++++++++-------------
> 1 file changed, 36 insertions(+), 17 deletions(-)
>

Reviewed-by: Samiullah Khawaja <skhawaja@google.com>

Re: [PATCH v3 1/5] iommu/amd: Refactor device probe and capability initialization

[Vasant Hegde]

On 8/24/2026 5:53 PM, Pranjal Shrivastava wrote:
> Restructure the device probe path to improve readability and prepare for
> cleaner error handling. Refactor check_device() into iommu_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>
> Signed-off-by: Pranjal Shrivastava <praan@google.com>

One minor nit. Otherwise patch looks good.

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



> ---
>  drivers/iommu/amd/iommu.c | 117 +++++++++++++++++++-------------------
>  1 file changed, 60 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 4c31294fabc5..e1f9fbb63837 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)


.../...

>  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;
> +	u16 devid;
>  
> -	if (!check_device(dev))
> -		return ERR_PTR(-ENODEV);
> -
> -	iommu = rlookup_amd_iommu(dev);
> -	if (!iommu)
> -		return ERR_PTR(-ENODEV);
> -
> -	/* Not registered yet? */
> -	if (!iommu->iommu.ops)
> +	if (!iommu_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)) {
> +		ret = PTR_ERR(dev_data);

Instead of this, you can change below iommu_Dev assignment directly.

-Vasant

>  		dev_err(dev, "Failed to initialize - trying to proceed anyway\n");
>  		iommu_dev = ERR_PTR(ret);
>  		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. But interrupt-remap
> +	 * is still supported. Hence do not ignore the device.
>  	 */
> -	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__);
> @@ -2527,16 +2540,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;
>  }

Re: [PATCH v3 2/5] iommu/amd: Fix DTE clearing and rename iommu_ignore_device()

[Vasant Hegde]

On 8/24/2026 5:53 PM, Pranjal Shrivastava wrote:
> The iommu_ignore_device() function currently uses memset() to manually
> clear the primary Device Table Entry (DTE), which risks torn writes as
> the hardware reads DTEs as atomic 256-bit qwords. Furthermore, clearing
> the primary devid in the lookup table before calling setup_aliases()
> causes rlookup_amd_iommu() to fail for aliases. This prevents clearing
> the DTEs for DMA aliases.
> 
> Fix this by replacing the manual memset with a dedicated helper that
> invalidates the DTE by clearing the lower 128 bits (having the Valid bit)
> first, followed by the upper 128 bits. The cleared state is then
> explicitly cloned to all aliases before the lookup tables are nullified.
> 
> Rename the function to iommu_disable_device() more accurately
> reflects its intent, as we still support IRQ remapping for these devices)
> 
> Fixes: 99fc4ac3d297 ("iommu/amd: Introduce per PCI segment alias_table")
> Reported-by: sashiko-bot@kernel.org
> Closes: https://lore.kernel.org/all/20260529153216.2AD1E1F00899@smtp.kernel.org/
> Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Pranjal Shrivastava <praan@google.com>

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

-Vasant

Re: [PATCH v3 0/5] iommu/amd: Refactors for ATS robustness

[Vasant Hegde]

Pranjal,


On 8/24/2026 5:53 PM, Pranjal Shrivastava wrote:
> 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. 

Overall this looks good to me. We did run our CI w/ v2 and no regression found.


-Vasant

Re: [PATCH v3 1/5] iommu/amd: Refactor device probe and capability initialization

[Pranjal Shrivastava]

On Mon, Aug 24, 2026 at 03:13:21PM -0300, Jason Gunthorpe wrote:
> > [ ... 14 lines skipped ... ]
> > @@ -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 iommu_lookup_device(struct device *dev,
> > +				struct amd_iommu **iommu_out, u16 *devid_out)
> >  {
> 
> If we are touching the names driver functions should not be called
> iommu_*, it is confusing..
> 

Ack. So just lookup_device works?

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

Thanks,
Praan

Re: [PATCH v3 1/5] iommu/amd: Refactor device probe and capability initialization

[Pranjal Shrivastava]

On Tue, Aug 25, 2026 at 12:29:35PM +0530, Vasant Hegde wrote:
> 
> 
> On 8/24/2026 5:53 PM, Pranjal Shrivastava wrote:
> > Restructure the device probe path to improve readability and prepare for
> > cleaner error handling. Refactor check_device() into iommu_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>
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> 
> One minor nit. Otherwise patch looks good.
> 
> Reviewed-by: Vasant Hegde <vasant.hegde@amd.com>
> 
> 
> 
> > ---
> >  drivers/iommu/amd/iommu.c | 117 +++++++++++++++++++-------------------
> >  1 file changed, 60 insertions(+), 57 deletions(-)
> > 
> > diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> > index 4c31294fabc5..e1f9fbb63837 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)
> 
> 
> .../...
> 
> >  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;
> > +	u16 devid;
> >  
> > -	if (!check_device(dev))
> > -		return ERR_PTR(-ENODEV);
> > -
> > -	iommu = rlookup_amd_iommu(dev);
> > -	if (!iommu)
> > -		return ERR_PTR(-ENODEV);
> > -
> > -	/* Not registered yet? */
> > -	if (!iommu->iommu.ops)
> > +	if (!iommu_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)) {
> > +		ret = PTR_ERR(dev_data);
> 
> Instead of this, you can change below iommu_Dev assignment directly.

Ack, I'll change iommu_dev assignment directly.

> 
> -Vasant
> 
> >  		dev_err(dev, "Failed to initialize - trying to proceed anyway\n");
> >  		iommu_dev = ERR_PTR(ret);
> >  		iommu_ignore_device(iommu, dev);
> >  		goto out_err;
> >  	}
> >  

Thanks,
Praan

Re: [PATCH v3 2/5] iommu/amd: Fix DTE clearing and rename iommu_ignore_device()

[Pranjal Shrivastava]

On Mon, Aug 24, 2026 at 03:13:21PM -0300, Jason Gunthorpe wrote:
> > [ ... 52 lines skipped ... ]
> > +/*
> > + * Invalidate a DTE by clearing the Valid bit first.
> > + * Note: Lockless; Not to be used on a fully probed
> > + * device with live dev_data.
> > + */
> > +static void amd_iommu_disable_dte(struct dev_table_entry *ptr)
> > +{
> > +	struct dev_table_entry new = {};
> > +
> > +	write_dte_lower128(ptr, &new);
> > +	write_dte_upper128(ptr, &new);
> > +}
> 
> No need just call update_dte256() with a 0'd new. This is how all the
> update flows work, and it flushes the DTE which this looks like it has
> been missing all long.
> 

I originally considered using update_dte256(), but iommu_disable_device
is called from the early probe error path (err_deinit).

If iommu_init_device() fails before dev_data is fully allocated and
attached to dev->iommu->priv, we don't have a valid dev_data pointer.
Passing NULL into update_dte256() would cause a NULL pointer deref when
it tries to acquire dev_data->dte_lock.

That's why I introduced this helper amd_iommu_disable_dte(),
specifically for this early teardown scenario where the device isn't
fully live yet.

Thanks,
Praan

Re: [PATCH v3 3/5] iommu/amd: Split probe error paths to preserve IRQ remapping

[Pranjal Shrivastava]

On Mon, Aug 24, 2026 at 03:13:21PM -0300, Jason Gunthorpe wrote:
> > [ ... 15 lines skipped ... ]
> > @@ -2540,8 +2540,7 @@ static struct iommu_device *amd_iommu_probe_device(struct device *dev)
> >  		ret = PTR_ERR(dev_data);
> >  		dev_err(dev, "Failed to initialize - trying to proceed anyway\n");
> >  		iommu_dev = ERR_PTR(ret);
> 
> I would drop this line and fix the return to have the ERR_PTR
> 
> > +err_deinit:
> > +	iommu_disable_device(iommu, dev);
> >  out_err:
> >  	return iommu_dev;
> 
> Otherwise this landmine is going to hurt someday since there is only
> one flow that would have an err ptr in iommu_dev.
> 

Ack. I'd refactor this.

Thanks,
Praan

Re: [PATCH v3 0/5] iommu/amd: Refactors for ATS robustness

[Pranjal Shrivastava]

On Tue, Aug 25, 2026 at 12:36:18PM +0530, Vasant Hegde wrote:
> Pranjal,
> 
> 
> On 8/24/2026 5:53 PM, Pranjal Shrivastava wrote:
> > 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. 
> 
> Overall this looks good to me. We did run our CI w/ v2 and no regression found.
> 

Ack. v3 Just addressed a few comments. I'll send out a v4 based on the
feedback on this one soon.

Thanks,
Praan

Re: [PATCH v3 0/5] iommu/amd: Refactors for ATS robustness

[Pranjal Shrivastava]

On Mon, Aug 24, 2026 at 03:13:21PM -0300, Jason Gunthorpe wrote:
> > [ ... 41 lines skipped ... ]
> > Pranjal Shrivastava (5):
> >   iommu/amd: Refactor device probe and capability initialization
> >   iommu/amd: Fix DTE clearing and rename iommu_ignore_device()
> >   iommu/amd: Split probe error paths to preserve IRQ remapping
> >   iommu/amd: Fail probe on ATS configuration failure
> >   PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats()
> > 
> >  drivers/iommu/amd/iommu.c | 199 +++++++++++++++++++++++---------------
> >  drivers/pci/ats.c         |   6 +-
> >  2 files changed, 126 insertions(+), 79 deletions(-)
> 
> FWIW something about this series is mangled, the first patch was
> changed before it made it to lore. Maybe whitespace damage..
> 

I see. I generally use checkpatch with --fix-inplace before sending and
it seems like it tried fixing something. I'd drop that arg now on.

Thanks,
Praan

Re: [PATCH v3 2/5] iommu/amd: Fix DTE clearing and rename iommu_ignore_device()

[Jason Gunthorpe]

On Tue, Aug 25, 2026 at 10:32:07AM +0000, Pranjal Shrivastava wrote:
> On Mon, Aug 24, 2026 at 03:13:21PM -0300, Jason Gunthorpe wrote:
> > > [ ... 52 lines skipped ... ]
> > > +/*
> > > + * Invalidate a DTE by clearing the Valid bit first.
> > > + * Note: Lockless; Not to be used on a fully probed
> > > + * device with live dev_data.
> > > + */
> > > +static void amd_iommu_disable_dte(struct dev_table_entry *ptr)
> > > +{
> > > +	struct dev_table_entry new = {};
> > > +
> > > +	write_dte_lower128(ptr, &new);
> > > +	write_dte_upper128(ptr, &new);
> > > +}
> > 
> > No need just call update_dte256() with a 0'd new. This is how all the
> > update flows work, and it flushes the DTE which this looks like it has
> > been missing all long.
> 
> I originally considered using update_dte256(), but iommu_disable_device
> is called from the early probe error path (err_deinit).

There is no reason to do that, the DTE isn't written by
amd_iommu_probe_device(), so there is no reason to clear it on an
error path.

It is wrong to write to the DTE table without flushing the
HW cache.

Jason

Re: [PATCH v3 1/5] iommu/amd: Refactor device probe and capability initialization

[Jason Gunthorpe]

On Tue, Aug 25, 2026 at 10:24:16AM +0000, Pranjal Shrivastava wrote:
> On Mon, Aug 24, 2026 at 03:13:21PM -0300, Jason Gunthorpe wrote:
> > > [ ... 14 lines skipped ... ]
> > > @@ -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 iommu_lookup_device(struct device *dev,
> > > +				struct amd_iommu **iommu_out, u16 *devid_out)
> > >  {
> > 
> > If we are touching the names driver functions should not be called
> > iommu_*, it is confusing..
> > 
> 
> Ack. So just lookup_device works?

Yeah that seems a little common in this driver, amd_xxx would be OK
too

Jason

Re: [PATCH v3 0/5] iommu/amd: Refactors for ATS robustness

[Jason Gunthorpe]

On Tue, Aug 25, 2026 at 10:39:21AM +0000, Pranjal Shrivastava wrote:
> On Mon, Aug 24, 2026 at 03:13:21PM -0300, Jason Gunthorpe wrote:
> > > [ ... 41 lines skipped ... ]
> > > Pranjal Shrivastava (5):
> > >   iommu/amd: Refactor device probe and capability initialization
> > >   iommu/amd: Fix DTE clearing and rename iommu_ignore_device()
> > >   iommu/amd: Split probe error paths to preserve IRQ remapping
> > >   iommu/amd: Fail probe on ATS configuration failure
> > >   PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats()
> > > 
> > >  drivers/iommu/amd/iommu.c | 199 +++++++++++++++++++++++---------------
> > >  drivers/pci/ats.c         |   6 +-
> > >  2 files changed, 126 insertions(+), 79 deletions(-)
> > 
> > FWIW something about this series is mangled, the first patch was
> > changed before it made it to lore. Maybe whitespace damage..
> > 
> 
> I see. I generally use checkpatch with --fix-inplace before sending and
> it seems like it tried fixing something. I'd drop that arg now on.

Never edit patches after git prepares them, it messes up the hashes
git puts the headers and desyncs the context lines in later patches.

Jason

Re: [PATCH v3 2/5] iommu/amd: Fix DTE clearing and rename iommu_ignore_device()

[Pranjal Shrivastava]

On Tue, Aug 25, 2026 at 08:49:16AM -0300, Jason Gunthorpe wrote:
> On Tue, Aug 25, 2026 at 10:32:07AM +0000, Pranjal Shrivastava wrote:
> > On Mon, Aug 24, 2026 at 03:13:21PM -0300, Jason Gunthorpe wrote:
> > > > [ ... 52 lines skipped ... ]
> > > > +/*
> > > > + * Invalidate a DTE by clearing the Valid bit first.
> > > > + * Note: Lockless; Not to be used on a fully probed
> > > > + * device with live dev_data.
> > > > + */
> > > > +static void amd_iommu_disable_dte(struct dev_table_entry *ptr)
> > > > +{
> > > > +	struct dev_table_entry new = {};
> > > > +
> > > > +	write_dte_lower128(ptr, &new);
> > > > +	write_dte_upper128(ptr, &new);
> > > > +}
> > > 
> > > No need just call update_dte256() with a 0'd new. This is how all the
> > > update flows work, and it flushes the DTE which this looks like it has
> > > been missing all long.
> > 
> > I originally considered using update_dte256(), but iommu_disable_device
> > is called from the early probe error path (err_deinit).
> 
> There is no reason to do that, the DTE isn't written by
> amd_iommu_probe_device(), so there is no reason to clear it on an
> error path.
> 
> It is wrong to write to the DTE table without flushing the
> HW cache.
> 

I agree, but I wonder why the existing code used memset here 
(in ignore_device):

memset(&dev_table[devid], 0, sizeof(struct dev_table_entry));

I was thinking it might've been done for probe failures in a kdump
kernel (normal kexec would've called shutdown for clearing all DTEs).
(I see this was added long time back and existed when PCI segments were
added [1]).

Are you suggesting to remove the DTE clearing from this path entirely?

Thanks,
Praan

[1] https://lore.kernel.org/all/20220706113825.25582-23-vasant.hegde@amd.com/