[PATCH v7 0/5] iommu: Standardize ATS robustness and state tracking

[PATCH v7 0/5] iommu: Standardize ATS robustness and state tracking

[Pranjal Shrivastava]

The primary motivation for this series is an ATS state mismatch observed
under heavy load (via iova_stress). A failure in pci_enable_ats() leaves
IOMMU drivers like arm-smmu-v3 with inconsistent state leading to PCI core
warnings during device detach.

While David's recent work [1] addressed a discovery race for specific
quirked devices by moving them to the HEADER phase, gaps remained
regarding how Virtual Functions (VFs) inherit state from their Physical
Functions (PFs). Specifically, pci_ats_supported() did not account for
PF-level quirked status, and pci_prepare_ats() lacked STU validation for
VFs.

Based on discussion with Jason and Baolu in v3/v5, it was decided that the
IOMMU drivers should explicitly check pci_ats_supported() before calling
pci_prepare_ats(). To enforce this, pci_prepare_ats() now noisily checks
for support via WARN_ON(). Furthermore, the device probe should fail if
pci_prepare_ats() fails. Since these early gates preclude software
configuration errors, any remaining failure during pci_enable_ats() is
treated as a kernel bug.

Additionally, while fixing probe paths for Intel, I also spotted a
minor fix for arm-smmu-v3 (patch 3).

Following the discussion with the community, the driver-specific series
have been posted separately:

 - Intel IOMMU fixes reported by Sashiko [2]
 - Refactors for AMD IOMMU [3]

[1] https://lore.kernel.org/linux-pci/20260403222750.1215002-1-dmatlack@google.com/
[2] https://lore.kernel.org/all/20260531170254.60493-1-praan@google.com/
[3] https://lore.kernel.org/all/20260601134204.2150602-1-praan@google.com/

[v7]
 - Moved patch PCI/ATS: Mandate checking pci_ats_supported() before pci_prepare_ats()
    to the AMD series [3] to maintain bisectibility
 - Added a UAF fix for arm-smmu-v3 to set iommu->priv = NULL

[v6]
  - Reverted the decoupling of pci_ats_supported() from pci_prepare_ats().
  - Added a WARN_ON() to the internal support check in pci_prepare_ats().
  - Dropped the standalone Intel bugfixes (RB-tree and UAF) to be sent as a
   separate standalone series per maintainer request.
  - Kept the folded UAF fix in the AMD IOMMU patch to ensure the new error
   path is immediately safe.
  - Collected Reviewed-by tags from Lu Baolu for PCI core patches.

[v5]
  - https://lore.kernel.org/all/20260528202353.3422206-1-praan@google.com/
  - Decoupled pci_ats_supported() from pci_prepare_ats() in the PCI core.
  - Rebased SMMUv3 support on top of Nicolin Chen's "Always-On ATS" series.
  - Fixed pre-existing RB-tree corruption in VT-d probe (Baolu/Sashiko).
  - Addressed the pre-existing UAF in AMD IOMMU probe suggested by Sashiko.

[v4]
  - https://lore.kernel.org/all/20260525184347.4059549-1-praan@google.com/
  - Standardized the pattern across Intel VT-d and AMD IOMMU drivers.
  - Replaced the SMMUv3 ats_prepared gate with a fatal probe-fail logic.
  - Utilized WARN() macros for runtime enablement failures in all drivers.
  - Collected R-b tags from Jason and Sami.

Pranjal Shrivastava (5):
  PCI/ATS: Ensure pci_ats_supported() is PF-aware for VFs
  PCI/ATS: Validate STU for VFs in pci_prepare_ats()
  iommu/arm-smmu-v3: Fix a UAF in the probe_device error path
  iommu/arm-smmu-v3: Standardize ATS enablement failure reporting
  iommu/vt-d: Fail probe on ATS configuration failure

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  8 +++++++-
 drivers/iommu/intel/iommu.c                 | 15 ++++++++++++---
 drivers/pci/ats.c                           | 13 ++++++++++---
 3 files changed, 29 insertions(+), 7 deletions(-)

-- 
2.54.0.823.g6e5bcc1fc9-goog

[PATCH v7 1/5] PCI/ATS: Ensure pci_ats_supported() is PF-aware for VFs

[Pranjal Shrivastava]

Update pci_ats_supported() to additionally check the associated PF's
status when called on a VF. This ensures that PF-level quirks and
untrusted status are correctly propagated to VFs, providing a robust
support check that aligns with the kernel's PF-centric ATS configuration
model and is immune to the timing of VF-specific fixups.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/pci/ats.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 96efa00d9743..679a3c3c1d54 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -40,10 +40,13 @@ void pci_ats_init(struct pci_dev *dev)
  */
 bool pci_ats_supported(struct pci_dev *dev)
 {
-	if (!dev->ats_cap)
+	if (!dev->ats_cap || dev->untrusted)
 		return false;
 
-	return (dev->untrusted == 0);
+	if (dev->is_virtfn)
+		return pci_ats_supported(pci_physfn(dev));
+
+	return true;
 }
 EXPORT_SYMBOL_GPL(pci_ats_supported);
 
-- 
2.54.0.823.g6e5bcc1fc9-goog

RE: [PATCH v7 1/5] PCI/ATS: Ensure pci_ats_supported() is PF-aware for VFs

[Tian, Kevin]

> From: Pranjal Shrivastava <praan@google.com>
> Sent: Monday, June 1, 2026 10:37 PM
> 
> Update pci_ats_supported() to additionally check the associated PF's
> status when called on a VF. This ensures that PF-level quirks and
> untrusted status are correctly propagated to VFs, providing a robust
> support check that aligns with the kernel's PF-centric ATS configuration
> model and is immune to the timing of VF-specific fixups.
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Pranjal Shrivastava <praan@google.com>

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

[PATCH v7 2/5] PCI/ATS: Validate STU for VFs in pci_prepare_ats()

[Pranjal Shrivastava]

While every PCI Function that implements ATS has an independent ATS
Extended Capability structure with a Read/Write Smallest Translation
Unit (STU) field, the kernel manages SR-IOV ATS by requiring the IOMMU
driver to configure the STU on the Physical Function (PF) before any
any Virtual Functions (VFs) are created.

Currently, pci_prepare_ats() bails out early for VFs, assuming that the
PF has already been correctly prepared. However, this creates a potential
mismatch if a VF is subsequently prepared with a different page shift.

Update pci_prepare_ats() to validate that the requested page shift (ps)
matches the STU already configured in the associated PF. This ensures
early detection of incompatible configurations and maintains the kernel's
policy of consistent STU sizing across all functions associated with a
given SMMU.

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/pci/ats.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index 679a3c3c1d54..9cb23780093d 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -73,8 +73,12 @@ int pci_prepare_ats(struct pci_dev *dev, int ps)
 	if (ps < PCI_ATS_MIN_STU)
 		return -EINVAL;
 
-	if (dev->is_virtfn)
+	if (dev->is_virtfn) {
+		if (pci_physfn(dev)->ats_stu != ps)
+			return -EINVAL;
+
 		return 0;
+	}
 
 	dev->ats_stu = ps;
 	ctrl = PCI_ATS_CTRL_STU(dev->ats_stu - PCI_ATS_MIN_STU);
-- 
2.54.0.823.g6e5bcc1fc9-goog

RE: [PATCH v7 2/5] PCI/ATS: Validate STU for VFs in pci_prepare_ats()

[Tian, Kevin]

> From: Pranjal Shrivastava <praan@google.com>
> Sent: Monday, June 1, 2026 10:37 PM
> 
> While every PCI Function that implements ATS has an independent ATS
> Extended Capability structure with a Read/Write Smallest Translation
> Unit (STU) field, the kernel manages SR-IOV ATS by requiring the IOMMU
> driver to configure the STU on the Physical Function (PF) before any
> any Virtual Functions (VFs) are created.

It's the PCIe spec defining STU must be hardwired to zero for VF, i.e.
the PF's value applies to all related VFs.

above reads like a software decision.

> 
> Currently, pci_prepare_ats() bails out early for VFs, assuming that the
> PF has already been correctly prepared. However, this creates a potential
> mismatch if a VF is subsequently prepared with a different page shift.
> 
> Update pci_prepare_ats() to validate that the requested page shift (ps)
> matches the STU already configured in the associated PF. This ensures
> early detection of incompatible configurations and maintains the kernel's
> policy of consistent STU sizing across all functions associated with a
> given SMMU.
> 
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
> Signed-off-by: Pranjal Shrivastava <praan@google.com>

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

[PATCH v7 3/5] iommu/arm-smmu-v3: Fix a UAF in the probe_device error path

[Pranjal Shrivastava]

Clear the iommu->priv to NULL while returning an error from probe_device.

Fixes: a2be6218e649 ("iommu/arm-smmu-v3: Improve add_device() error handling")
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index a10affb483a4..54c7f1ae3a84 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4206,6 +4206,7 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
 	arm_smmu_disable_pasid(master);
 	arm_smmu_remove_master(master);
 err_free_master:
+	dev_iommu_priv_set(dev, NULL);
 	kfree(master);
 	return ERR_PTR(ret);
 }
-- 
2.54.0.823.g6e5bcc1fc9-goog

Re: [PATCH v7 3/5] iommu/arm-smmu-v3: Fix a UAF in the probe_device error path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Calling `rb_erase()` on an unlinked, zero-initialized stream node silently corrupts the SMMU streams rbtree by zeroing the tree root.
--

commit 785579083a69b017d9dafe6dce3c844bea656662
Author: Pranjal Shrivastava <praan@google.com>

iommu/arm-smmu-v3: Fix a UAF in the probe_device error path

Clear the iommu->priv to NULL while returning an error from probe_device.

> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index a10affb483a4f..54c7f1ae3a841 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -4206,6 +4206,7 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
>  	arm_smmu_disable_pasid(master);
>  	arm_smmu_remove_master(master);

[Severity: Critical]
This is a pre-existing issue, but could calling arm_smmu_remove_master()
here silently corrupt the smmu streams rbtree if there are duplicate
Stream IDs?

In arm_smmu_insert_master(), master->streams is allocated with kzalloc,
leaving the rb_node zero-initialized. If a device has duplicate Stream IDs
(expected for bridged PCI devices), it skips inserting the duplicate node
into smmu->streams:

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:arm_smmu_insert_master() {
    ...
			/* Bridged PCI devices may end up with duplicated IDs */
			if (existing_master == master)
				continue;
    ...
}

However, arm_smmu_remove_master() unconditionally calls rb_erase() on all
nodes:

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:arm_smmu_remove_master() {
    ...
	for (i = 0; i < fwspec->num_ids; i++)
		rb_erase(&master->streams[i].node, &smmu->streams);
    ...
}

Since the rb_erase() implementation interprets a zero-initialized node as a
root node with no children, it clears it by setting root->rb_node = NULL.
Does this detach the entire stream tracking tree?

The error path in arm_smmu_insert_master() also unconditionally calls
rb_erase() on processed nodes:

drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c:arm_smmu_insert_master() {
    ...
		for (i--; i >= 0; i--)
			rb_erase(&master->streams[i].node, &smmu->streams);
    ...
}

Would this also trigger the same corruption if a failure happens after
skipping a duplicate ID?

>  err_free_master:
> +	dev_iommu_priv_set(dev, NULL);
>  	kfree(master);
>  	return ERR_PTR(ret);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260601143644.2358771-1-praan@google.com?part=3

RE: [PATCH v7 3/5] iommu/arm-smmu-v3: Fix a UAF in the probe_device error path

[Tian, Kevin]

> From: Pranjal Shrivastava <praan@google.com>
> Sent: Monday, June 1, 2026 10:37 PM
> 
> Clear the iommu->priv to NULL while returning an error from probe_device.
> 
> Fixes: a2be6218e649 ("iommu/arm-smmu-v3: Improve add_device() error
> handling")
> Signed-off-by: Pranjal Shrivastava <praan@google.com>

probably add a note that UAF is theoretical at this point.

iommu_init_device() calls dev_iommu_free() right after @probe_device()
fails...

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

Re: [PATCH v7 3/5] iommu/arm-smmu-v3: Fix a UAF in the probe_device error path

[Pranjal Shrivastava]

On Wed, Jun 03, 2026 at 07:31:38AM +0000, Tian, Kevin wrote:
> > From: Pranjal Shrivastava <praan@google.com>
> > Sent: Monday, June 1, 2026 10:37 PM
> > 
> > Clear the iommu->priv to NULL while returning an error from probe_device.
> > 
> > Fixes: a2be6218e649 ("iommu/arm-smmu-v3: Improve add_device() error
> > handling")
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> 
> probably add a note that UAF is theoretical at this point.
> 
> iommu_init_device() calls dev_iommu_free() right after @probe_device()
> fails...

Ack. This is just to prevent a UAF against future refactors. I saw the
intel & amd iommu drivers doing it and felt this is missing from smmuv3

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

Thanks,
Praan

Re: [PATCH v7 3/5] iommu/arm-smmu-v3: Fix a UAF in the probe_device error path

[Jason Gunthorpe]

On Wed, Jun 03, 2026 at 01:28:29PM +0000, Pranjal Shrivastava wrote:
> On Wed, Jun 03, 2026 at 07:31:38AM +0000, Tian, Kevin wrote:
> > > From: Pranjal Shrivastava <praan@google.com>
> > > Sent: Monday, June 1, 2026 10:37 PM
> > > 
> > > Clear the iommu->priv to NULL while returning an error from probe_device.
> > > 
> > > Fixes: a2be6218e649 ("iommu/arm-smmu-v3: Improve add_device() error
> > > handling")
> > > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > 
> > probably add a note that UAF is theoretical at this point.
> > 
> > iommu_init_device() calls dev_iommu_free() right after @probe_device()
> > fails...
> 
> Ack. This is just to prevent a UAF against future refactors. I saw the
> intel & amd iommu drivers doing it and felt this is missing from smmuv3

That is just pointless dead code, the core code immediately frees the
memory this is NULLing

static int iommu_init_device(struct device *dev)
{
[..]
err_free:
	dev->iommu->iommu_dev = NULL;
	dev_iommu_free(dev);
	return ret;

I would remove it from the other drivers not addd it here..

Jason

Re: [PATCH v7 3/5] iommu/arm-smmu-v3: Fix a UAF in the probe_device error path

[Baolu Lu]

On 6/3/26 22:59, Jason Gunthorpe wrote:
> On Wed, Jun 03, 2026 at 01:28:29PM +0000, Pranjal Shrivastava wrote:
>> On Wed, Jun 03, 2026 at 07:31:38AM +0000, Tian, Kevin wrote:
>>>> From: Pranjal Shrivastava<praan@google.com>
>>>> Sent: Monday, June 1, 2026 10:37 PM
>>>>
>>>> Clear the iommu->priv to NULL while returning an error from probe_device.
>>>>
>>>> Fixes: a2be6218e649 ("iommu/arm-smmu-v3: Improve add_device() error
>>>> handling")
>>>> Signed-off-by: Pranjal Shrivastava<praan@google.com>
>>> probably add a note that UAF is theoretical at this point.
>>>
>>> iommu_init_device() calls dev_iommu_free() right after @probe_device()
>>> fails...
>> Ack. This is just to prevent a UAF against future refactors. I saw the
>> intel & amd iommu drivers doing it and felt this is missing from smmuv3
> That is just pointless dead code, the core code immediately frees the
> memory this is NULLing
> 
> static int iommu_init_device(struct device *dev)
> {
> [..]
> err_free:
> 	dev->iommu->iommu_dev = NULL;
> 	dev_iommu_free(dev);
> 	return ret;
> 
> I would remove it from the other drivers not addd it here..

You are right. I ever mistakenly thought that release_device would be
called in the error path, as I noticed the following:

err_release:
         if (ops->release_device)
                 ops->release_device(dev);

That actually is not executed when probe_device() fails, so there is no
UAF issue here. Anyway, it would be better to add a comment line in the
driver to prevent any future misunderstandings.

Thanks,
baolu

Re: [PATCH v7 3/5] iommu/arm-smmu-v3: Fix a UAF in the probe_device error path

[Pranjal Shrivastava]

On Wed, Jun 03, 2026 at 11:59:03AM -0300, Jason Gunthorpe wrote:
> On Wed, Jun 03, 2026 at 01:28:29PM +0000, Pranjal Shrivastava wrote:
> > On Wed, Jun 03, 2026 at 07:31:38AM +0000, Tian, Kevin wrote:
> > > > From: Pranjal Shrivastava <praan@google.com>
> > > > Sent: Monday, June 1, 2026 10:37 PM
> > > > 
> > > > Clear the iommu->priv to NULL while returning an error from probe_device.
> > > > 
> > > > Fixes: a2be6218e649 ("iommu/arm-smmu-v3: Improve add_device() error
> > > > handling")
> > > > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > > 
> > > probably add a note that UAF is theoretical at this point.
> > > 
> > > iommu_init_device() calls dev_iommu_free() right after @probe_device()
> > > fails...
> > 
> > Ack. This is just to prevent a UAF against future refactors. I saw the
> > intel & amd iommu drivers doing it and felt this is missing from smmuv3
> 
> That is just pointless dead code, the core code immediately frees the
> memory this is NULLing
> 
> static int iommu_init_device(struct device *dev)
> {
> [..]
> err_free:
> 	dev->iommu->iommu_dev = NULL;
> 	dev_iommu_free(dev);
> 	return ret;
> 
> I would remove it from the other drivers not addd it here..

Ack. I tend to agree... I'll drop this patch here and request Baulu to
drop it from his PR too.

Thanks,
Praan

[PATCH v7 4/5] iommu/arm-smmu-v3: Standardize ATS enablement failure reporting

[Pranjal Shrivastava]

Update arm_smmu_enable_ats() to wrap the pci_enable_ats() call in
WARN_ON(). Since probe-time checks now preclude configuration errors
any failure during hardware enablement is considered a kernel bug.

Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 54c7f1ae3a84..dfb2b0d2d2a5 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2956,7 +2956,12 @@ static void arm_smmu_enable_ats(struct arm_smmu_master *master)
 	 * ATC invalidation of PASID 0 causes the entire ATC to be flushed.
 	 */
 	arm_smmu_atc_inv_master(master, IOMMU_NO_PASID);
-	if (pci_enable_ats(pdev, stu))
+
+	 /*
+	  * Since pci_prepare_ats() has already verified the HW capability
+	  * and programmed the STE, pci_enable_ats() should not fail here.
+	  */
+	if (WARN_ON(pci_enable_ats(pdev, stu)))
 		dev_err(master->dev, "Failed to enable ATS (STU %zu)\n", stu);
 }
 
-- 
2.54.0.823.g6e5bcc1fc9-goog

RE: [PATCH v7 4/5] iommu/arm-smmu-v3: Standardize ATS enablement failure reporting

[Tian, Kevin]

> From: Pranjal Shrivastava <praan@google.com>
> Sent: Monday, June 1, 2026 10:37 PM
> 
> Update arm_smmu_enable_ats() to wrap the pci_enable_ats() call in
> WARN_ON(). Since probe-time checks now preclude configuration errors
> any failure during hardware enablement is considered a kernel bug.
> 
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 54c7f1ae3a84..dfb2b0d2d2a5 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2956,7 +2956,12 @@ static void arm_smmu_enable_ats(struct
> arm_smmu_master *master)
>  	 * ATC invalidation of PASID 0 causes the entire ATC to be flushed.
>  	 */
>  	arm_smmu_atc_inv_master(master, IOMMU_NO_PASID);
> -	if (pci_enable_ats(pdev, stu))
> +
> +	 /*
> +	  * Since pci_prepare_ats() has already verified the HW capability
> +	  * and programmed the STE, pci_enable_ats() should not fail here.
> +	  */
> +	if (WARN_ON(pci_enable_ats(pdev, stu)))
>  		dev_err(master->dev, "Failed to enable ATS (STU %zu)\n",
> stu);
>  }
> 

looks smmu driver doesn't check return value of pci_prepare_ats() too.

Re: [PATCH v7 4/5] iommu/arm-smmu-v3: Standardize ATS enablement failure reporting

[Pranjal Shrivastava]

On Wed, Jun 03, 2026 at 07:34:14AM +0000, Tian, Kevin wrote:
> > From: Pranjal Shrivastava <praan@google.com>
> > Sent: Monday, June 1, 2026 10:37 PM
> > 
> > Update arm_smmu_enable_ats() to wrap the pci_enable_ats() call in
> > WARN_ON(). Since probe-time checks now preclude configuration errors
> > any failure during hardware enablement is considered a kernel bug.
> > 
> > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > ---
> >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index 54c7f1ae3a84..dfb2b0d2d2a5 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -2956,7 +2956,12 @@ static void arm_smmu_enable_ats(struct
> > arm_smmu_master *master)
> >  	 * ATC invalidation of PASID 0 causes the entire ATC to be flushed.
> >  	 */
> >  	arm_smmu_atc_inv_master(master, IOMMU_NO_PASID);
> > -	if (pci_enable_ats(pdev, stu))
> > +
> > +	 /*
> > +	  * Since pci_prepare_ats() has already verified the HW capability
> > +	  * and programmed the STE, pci_enable_ats() should not fail here.
> > +	  */
> > +	if (WARN_ON(pci_enable_ats(pdev, stu)))
> >  		dev_err(master->dev, "Failed to enable ATS (STU %zu)\n",
> > stu);
> >  }
> > 
> 
> looks smmu driver doesn't check return value of pci_prepare_ats() too.

It does check the reval of pci_prepare_ats() now with the a recent 
patch [1] (merged in Joerg's tree) that adds a new 
"arm_smmu_master_prepare_ats" helper function.

Thanks,
Praan

[1] https://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git/commit/?h=core&id=5ff98a0da6b48d8722eac0080e4185c417925d05

RE: [PATCH v7 4/5] iommu/arm-smmu-v3: Standardize ATS enablement failure reporting

[Tian, Kevin]

> From: Pranjal Shrivastava <praan@google.com>
> Sent: Wednesday, June 3, 2026 5:12 PM
> 
> On Wed, Jun 03, 2026 at 07:34:14AM +0000, Tian, Kevin wrote:
> > > From: Pranjal Shrivastava <praan@google.com>
> > > Sent: Monday, June 1, 2026 10:37 PM
> > >
> > > Update arm_smmu_enable_ats() to wrap the pci_enable_ats() call in
> > > WARN_ON(). Since probe-time checks now preclude configuration errors
> > > any failure during hardware enablement is considered a kernel bug.
> > >
> > > Signed-off-by: Pranjal Shrivastava <praan@google.com>
> > > ---
> > >  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > index 54c7f1ae3a84..dfb2b0d2d2a5 100644
> > > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > > @@ -2956,7 +2956,12 @@ static void arm_smmu_enable_ats(struct
> > > arm_smmu_master *master)
> > >  	 * ATC invalidation of PASID 0 causes the entire ATC to be flushed.
> > >  	 */
> > >  	arm_smmu_atc_inv_master(master, IOMMU_NO_PASID);
> > > -	if (pci_enable_ats(pdev, stu))
> > > +
> > > +	 /*
> > > +	  * Since pci_prepare_ats() has already verified the HW capability
> > > +	  * and programmed the STE, pci_enable_ats() should not fail here.
> > > +	  */
> > > +	if (WARN_ON(pci_enable_ats(pdev, stu)))
> > >  		dev_err(master->dev, "Failed to enable ATS (STU %zu)\n",
> > > stu);
> > >  }
> > >
> >
> > looks smmu driver doesn't check return value of pci_prepare_ats() too.
> 
> It does check the reval of pci_prepare_ats() now with the a recent
> patch [1] (merged in Joerg's tree) that adds a new
> "arm_smmu_master_prepare_ats" helper function.
> 
> Thanks,
> Praan
> 
> [1]
> https://git.kernel.org/pub/scm/linux/kernel/git/iommu/linux.git/commit/?h
> =core&id=5ff98a0da6b48d8722eac0080e4185c417925d05

Okay. Then:

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

[PATCH v7 5/5] iommu/vt-d: Fail probe on ATS configuration failure

[Pranjal Shrivastava]

Update the Intel VT-d driver to handle ATS configuration and enablement
more strictly. Specifically, update the device probe to fail if
pci_prepare_ats() returns an error. This ensures that any ATS-capable
master reaching the attach phase is guaranteed to have a valid config.

Additionally, update iommu_enable_pci_ats() to WARN() if pci_enable_ats
fails. Since earlier checks in the probe phase preclude config-related
failures, any failure during hardware enablement is considered a kernel
bug.

Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/intel/iommu.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 4d0e65bc131d..22308e4911e1 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -873,8 +873,14 @@ static void iommu_enable_pci_ats(struct device_domain_info *info)
 	if (!pci_ats_page_aligned(pdev))
 		return;
 
-	if (!pci_enable_ats(pdev, VTD_PAGE_SHIFT))
-		info->ats_enabled = 1;
+	/*
+	 * pci_enable_ats() should not fail here because earlier checks
+	 * have already verified support and configuration.
+	 */
+	if (WARN_ON(pci_enable_ats(pdev, VTD_PAGE_SHIFT)))
+		return;
+
+	info->ats_enabled = 1;
 }
 
 static void iommu_disable_pci_ats(struct device_domain_info *info)
@@ -3288,7 +3294,10 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
 
 	dev_iommu_priv_set(dev, info);
 	if (pdev && pci_ats_supported(pdev)) {
-		pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
+		ret = pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
+		if (ret)
+			goto free;
+
 		ret = device_rbtree_insert(iommu, info);
 		if (ret)
 			goto free;
-- 
2.54.0.823.g6e5bcc1fc9-goog

Re: [PATCH v7 5/5] iommu/vt-d: Fail probe on ATS configuration failure

[Pranjal Shrivastava]

On Mon, Jun 01, 2026 at 03:30:31PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Unconditional call to `device_rbtree_remove(info)` in the error path causes rbtree corruption for non-ATS devices.
> --
> 
> commit 73f5ec2d0aa9d509723e686b4cbf5b853c3a39c6
> Author: Pranjal Shrivastava <praan@google.com>
> 
>     iommu/vt-d: Fail probe on ATS configuration failure
>     
>     Update the Intel VT-d driver to handle ATS configuration and enablement
>     more strictly. Specifically, update the device probe to fail if
>     pci_prepare_ats() returns an error. This ensures that any ATS-capable
>     master reaching the attach phase is guaranteed to have a valid config.
> 
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index 4d0e65bc131d7..22308e4911e1f 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> [ ... ]
> > @@ -3288,7 +3294,10 @@ static struct iommu_device *intel_iommu_probe_device(struct device *dev)
> >  
> >  	dev_iommu_priv_set(dev, info);
> >  	if (pdev && pci_ats_supported(pdev)) {
> > -		pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
> > +		ret = pci_prepare_ats(pdev, VTD_PAGE_SHIFT);
> > +		if (ret)
> > +			goto free;
> > +
> >  		ret = device_rbtree_insert(iommu, info);
> >  		if (ret)
> >  			goto free;
> 
> [Severity: High]
> This is a pre-existing issue, but does the error path in this function
> unconditionally call device_rbtree_remove() on failure, even for non-ATS
> devices?
> 
> If intel_pasid_alloc_table() fails later in intel_iommu_probe_device(), it
> jumps to clear_rbtree:
> 
>         if (sm_supported(iommu) && !dev_is_real_dma_subdevice(dev)) {
>                 ret = intel_pasid_alloc_table(dev);
>                 if (ret) {
>                         dev_err(dev, "PASID table allocation failed\n");
>                         goto clear_rbtree;
>                 }
> 
> clear_rbtree unconditionally removes the info from the rbtree:
> 
> clear_rbtree:
>         device_rbtree_remove(info);
> 
> Since info->node is zero-initialized via kzalloc_obj(), calling rb_erase()
> on it for a non-ATS device (which was never inserted into the rbtree) causes
> the rbtree implementation to treat it as a root node and overwrite
> iommu->device_rbtree.rb_node with NULL.
> 
> This silently destroys the entire rbtree and detaches all previously
> registered ATS devices. Could we add a check to verify if the device was
> actually inserted before calling device_rbtree_remove(info)?

All of these have already been fixed in the Intel series:
https://lore.kernel.org/all/20260531170254.60493-1-praan@google.com/

Thanks,
Praan

Re: [PATCH v7 5/5] iommu/vt-d: Fail probe on ATS configuration failure

[Baolu Lu]

On 6/1/26 22:36, Pranjal Shrivastava wrote:
> Update the Intel VT-d driver to handle ATS configuration and enablement
> more strictly. Specifically, update the device probe to fail if
> pci_prepare_ats() returns an error. This ensures that any ATS-capable
> master reaching the attach phase is guaranteed to have a valid config.
> 
> Additionally, update iommu_enable_pci_ats() to WARN() if pci_enable_ats
> fails. Since earlier checks in the probe phase preclude config-related
> failures, any failure during hardware enablement is considered a kernel
> bug.
> 
> Signed-off-by: Pranjal Shrivastava<praan@google.com>
> ---
>   drivers/iommu/intel/iommu.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 deletions(-)

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

RE: [PATCH v7 5/5] iommu/vt-d: Fail probe on ATS configuration failure

[Tian, Kevin]

> From: Pranjal Shrivastava <praan@google.com>
> Sent: Monday, June 1, 2026 10:37 PM
> 
> Update the Intel VT-d driver to handle ATS configuration and enablement
> more strictly. Specifically, update the device probe to fail if
> pci_prepare_ats() returns an error. This ensures that any ATS-capable
> master reaching the attach phase is guaranteed to have a valid config.
> 
> Additionally, update iommu_enable_pci_ats() to WARN() if pci_enable_ats
> fails. Since earlier checks in the probe phase preclude config-related
> failures, any failure during hardware enablement is considered a kernel
> bug.
> 
> Signed-off-by: Pranjal Shrivastava <praan@google.com>

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