[PATCH v3 0/3] iommu/arm-smmu-v3: Fix ATS robustness and state tracking

[PATCH v3 0/3] iommu/arm-smmu-v3: Fix ATS robustness and state tracking

[Pranjal Shrivastava]

This series addresses a set of related ATS robustness issues in the PCI
core and the ARM SMMUv3 driver.

The primary motivation is a state mismatch observed under heavy load
where a failure in pci_enable_ats() leaves the SMMUv3 driver with
inconsistent internal counters (nr_ats_masters), leading to memory
leaks and PCI core warnings during device detach.

While David's recent work [1] addressed the 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.

This series addresses these gaps and hardens the SMMUv3 driver:
 - Patch 1 makes pci_ats_supported() PF-aware, ensuring PF-level
   constraints correctly propagate to VFs.
 - Patch 2 adds STU validation to pci_prepare_ats() for VFs, ensuring
   consistent page shift configurations.
 - Patch 3 introduces an 'ats_prepared' gate in the SMMUv3 driver to
   ensure that configuration failures at probe result in a clean
   fallback to non-ATS mode, preventing domain state pollution.

Thanks,
Pranjal

[1] https://lore.kernel.org/linux-pci/20260403222750.1215002-1-dmatlack@google.com/

v2: https://lore.kernel.org/all/20260504163842.2692314-1-praan@google.com/

Pranjal Shrivastava (3):
  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 ATS state tracking via ats_prepared gate

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  5 +++--
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  1 +
 drivers/pci/ats.c                           | 14 +++++++++++---
 3 files changed, 15 insertions(+), 5 deletions(-)

-- 
2.54.0.563.g4f69b47b94-goog

[PATCH v3 1/3] 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.

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 ec6c8dbdc5e9..a5fa7745bce8 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.563.g4f69b47b94-goog

[PATCH v3 2/3] 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.

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

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index a5fa7745bce8..54319854bfd8 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -73,8 +73,13 @@ 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) {
+		struct pci_dev *pdev = pci_physfn(dev);
+
+		if (pdev->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.563.g4f69b47b94-goog

[PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Pranjal Shrivastava]

The SMMUv3 driver currently has a two-phase commit in its ATS enablement
flow. During arm_smmu_attach_prepare(), it predicts whether ATS will be
enabled using arm_smmu_ats_supported() and accordingly increments
nr_ats_masters and merges ATS invalidations into the domain's invs array.

However, the actual hardware enablement via pci_enable_ats() happens
later in arm_smmu_attach_commit(). If this call to pci_enable_ats fails
the SMMU driver's ATS state tracking remains polluted, i.e. the driver
tracks ATS enabled on a master that is not actually using ATS. This leads
to an incorrect nr_ats_masters and triggers a warning in the PCI core
during detach:

[  127.925080] ------------[ cut here ]------------
[  127.925084] WARNING: drivers/pci/ats.c:132 at pci_disable_ats+0x94/0xa8, CPU#42: iova_stress/12240
[  127.949761] Modules linked in: vfat fat dummy bridge stp llc cdc_ncm cdc_eem cdc_ether usbnet mii xhci_pci xhci_hcd ehci_pci ehci_hcd
[  127.961760] CPU: 42 UID: 0 PID: 12240 Comm: iova_stress Not tainted 7.1.0-smp-DEV #4 PREEMPTLAZY
[  127.970619] Hardware name: <REDACTED>
[  127.977655] pstate: 61400009 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[  127.984603] pc : pci_disable_ats+0x94/0xa8
[  127.988687] lr : arm_smmu_attach_prepare+0x104/0x310
...
[  128.068169] Call trace:
[  128.070603]  pci_disable_ats+0x94/0xa8 (P)
[  128.074688]  arm_smmu_attach_prepare+0x104/0x310
[  128.079292]  arm_smmu_attach_dev_ste+0x128/0x1e0
[  128.083899]  arm_smmu_attach_dev_blocked+0x50/0x88
[  128.088677]  __iommu_attach_device+0x30/0x138
[  128.093026]  __iommu_group_set_domain_internal+0xdc/0x228
[  128.098412]  __iommu_take_dma_ownership+0x118/0x150
[  128.103278]  iommu_group_claim_dma_owner+0x48/0x80
[  128.108056]  vfio_container_attach_group+0xc8/0x1b0
[  128.112927]  vfio_group_fops_unl_ioctl+0x578/0x968
[  128.117706]  __arm64_sys_ioctl+0x90/0xe8

The issue was exposed under heavy load when running a VFIO-based DMA
map stress test (iova_stress).

Introduce an 'ats_prepared' flag in struct arm_smmu_master.
This flag is only set during arm_smmu_probe_device() if the initial
pci_prepare_ats() configuration (which sets the STU) succeeds.

Update arm_smmu_attach_prepare() to gate ats_enabled on this new flag.
This ensures that if a master failed its initial ATS configuration, the
driver gracefully falls back to a non-ATS mode, skipping all ATS-related
resource allocation for the domain. This syncs the driver state with the
HW state.

Fixes: 7497f4211f4f ("iommu/arm-smmu-v3: Make changing domains be hitless for ATS")
Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 5 +++--
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 +
 2 files changed, 4 insertions(+), 2 deletions(-)

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 811d859291e9..af39bf8d2774 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -3718,7 +3718,7 @@ int arm_smmu_attach_prepare(struct arm_smmu_attach_state *state,
 		 * Disable ATS if we are going to create a normal 0b100 bypass
 		 * STE.
 		 */
-		state->ats_enabled = !state->disable_ats &&
+		state->ats_enabled = !state->disable_ats && master->ats_prepared &&
 				     arm_smmu_ats_supported(master);
 	}
 
@@ -4450,7 +4450,8 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
 	if (dev_is_pci(dev)) {
 		unsigned int stu = __ffs(smmu->pgsize_bitmap);
 
-		pci_prepare_ats(to_pci_dev(dev), stu);
+		if (!pci_prepare_ats(to_pci_dev(dev), stu))
+			master->ats_prepared = true;
 	}
 
 	device_link_add(dev, smmu->dev,
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index a697fa3c457b..1fc1c420613f 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -947,6 +947,7 @@ struct arm_smmu_master {
 	struct arm_smmu_ctx_desc_cfg	cd_table;
 	unsigned int			num_streams;
 	bool				ats_enabled : 1;
+	bool				ats_prepared : 1;
 	bool				ste_ats_enabled : 1;
 	bool				stall_enabled;
 	unsigned int			ssid_bits;
-- 
2.54.0.563.g4f69b47b94-goog

Re: [PATCH v3 1/3] PCI/ATS: Ensure pci_ats_supported() is PF-aware for VFs

[Jason Gunthorpe]

On Tue, May 19, 2026 at 01:53:20PM +0000, Pranjal Shrivastava wrote:
> 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.
> 
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
>  drivers/pci/ats.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)

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

Jason

Re: [PATCH v3 2/3] PCI/ATS: Validate STU for VFs in pci_prepare_ats()

[Jason Gunthorpe]

On Tue, May 19, 2026 at 01:53:21PM +0000, Pranjal Shrivastava wrote:
> 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.

I would be clearer here, this is not a "mismatch", it reflects a
kernel bug.

> 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.
> 
> Signed-off-by: Pranjal Shrivastava <praan@google.com>
> ---
>  drivers/pci/ats.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

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

Jason

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Jason Gunthorpe]

On Tue, May 19, 2026 at 01:53:22PM +0000, Pranjal Shrivastava wrote:
> @@ -4450,7 +4450,8 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
>  	if (dev_is_pci(dev)) {
>  		unsigned int stu = __ffs(smmu->pgsize_bitmap);
>  
> -		pci_prepare_ats(to_pci_dev(dev), stu);
> +		if (!pci_prepare_ats(to_pci_dev(dev), stu))
> +			master->ats_prepared = true;
>  	}

This should fail not keep going, it is a kernel bug if
pci_prepare_ats() fails.

Jason

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Pranjal Shrivastava]

On Tue, May 19, 2026 at 11:44:30AM -0300, Jason Gunthorpe wrote:
> On Tue, May 19, 2026 at 01:53:22PM +0000, Pranjal Shrivastava wrote:
> > @@ -4450,7 +4450,8 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
> >  	if (dev_is_pci(dev)) {
> >  		unsigned int stu = __ffs(smmu->pgsize_bitmap);
> >  
> > -		pci_prepare_ats(to_pci_dev(dev), stu);
> > +		if (!pci_prepare_ats(to_pci_dev(dev), stu))
> > +			master->ats_prepared = true;
> >  	}
> 
> This should fail not keep going, it is a kernel bug if
> pci_prepare_ats() fails.

Are you suggesting it's a kernel bug if the STU doesn't match?
IIUC, it can fail because of the following cases (including the PF STU):

	if (!pci_ats_supported(dev))
		return -EINVAL;

	if (WARN_ON(dev->ats_enabled))
		return -EBUSY;

	if (ps < PCI_ATS_MIN_STU)
		return -EINVAL;

	if (dev->is_virtfn) {
		struct pci_dev *pdev = pci_physfn(dev);

		if (pdev->ats_stu != ps)
			return -EINVAL;
		return 0;
	}

If ats isn't supported or if there's a STU mismatch, shuoldn't we just
ensure that we don't try enabling ATS for that device? 

Returning an error here would fail the device probe.. which seems like a
big hammer for devices that can't reliably use ATS.. I see Intel and AMD
IOMMU drivers don't check the retval for this pci_prepare_ats but check
it for pci_enable_ats.

We have a special case in SMMUv3 where we pre-allocate stuff in prepare
but actually enable ATS in attach_commit, which IMO neeeds to be gated 
based on client device's ATS capabilities?

Praan

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Jason Gunthorpe]

On Tue, May 19, 2026 at 02:55:54PM +0000, Pranjal Shrivastava wrote:
> On Tue, May 19, 2026 at 11:44:30AM -0300, Jason Gunthorpe wrote:
> > On Tue, May 19, 2026 at 01:53:22PM +0000, Pranjal Shrivastava wrote:
> > > @@ -4450,7 +4450,8 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
> > >  	if (dev_is_pci(dev)) {
> > >  		unsigned int stu = __ffs(smmu->pgsize_bitmap);
> > >  
> > > -		pci_prepare_ats(to_pci_dev(dev), stu);
> > > +		if (!pci_prepare_ats(to_pci_dev(dev), stu))
> > > +			master->ats_prepared = true;
> > >  	}
> > 
> > This should fail not keep going, it is a kernel bug if
> > pci_prepare_ats() fails.
> 
> Are you suggesting it's a kernel bug if the STU doesn't match?
> IIUC, it can fail because of the following cases (including the PF STU):
> 
> 	if (!pci_ats_supported(dev))
> 		return -EINVAL;

It should not be called if ats is not supported, iommu driver bug.

> 	if (WARN_ON(dev->ats_enabled))
> 		return -EBUSY;

It should not be called if ats is already enabled, iommu driver bug.

> 	if (ps < PCI_ATS_MIN_STU)
> 		return -EINVAL;

PAGE_SIZE is illegal somehow, build bug.
 
> 	if (dev->is_virtfn) {
> 		struct pci_dev *pdev = pci_physfn(dev);
> 
> 		if (pdev->ats_stu != ps)
> 			return -EINVAL;

The kernel failed to setup the PF, iommu driver bug.

> If ats isn't supported or if there's a STU mismatch, shuoldn't we just
> ensure that we don't try enabling ATS for that device?

No, the kernel is broken somehow.

> Returning an error here would fail the device probe.. which seems like a
> big hammer for devices that can't reliably use ATS.. 

It has nothing to do with devices, the kernel is broken if it fails.

> We have a special case in SMMUv3 where we pre-allocate stuff in prepare
> but actually enable ATS in attach_commit, which IMO neeeds to be gated 
> based on client device's ATS capabilities?

Yes, all of this should be gated by pci_ats_supported()

But also failing to enable should translate into a failed attach.

Jason

Re: [PATCH v3 1/3] PCI/ATS: Ensure pci_ats_supported() is PF-aware for VFs

[Samiullah Khawaja]

On Tue, May 19, 2026 at 01:53:20PM +0000, Pranjal Shrivastava wrote:
>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.
>
>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 ec6c8dbdc5e9..a5fa7745bce8 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.563.g4f69b47b94-goog
>

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

Re: [PATCH v3 2/3] PCI/ATS: Validate STU for VFs in pci_prepare_ats()

[Samiullah Khawaja]

On Tue, May 19, 2026 at 01:53:21PM +0000, Pranjal Shrivastava wrote:
>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.
>
>Signed-off-by: Pranjal Shrivastava <praan@google.com>
>---
> drivers/pci/ats.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
>index a5fa7745bce8..54319854bfd8 100644
>--- a/drivers/pci/ats.c
>+++ b/drivers/pci/ats.c
>@@ -73,8 +73,13 @@ 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) {
>+		struct pci_dev *pdev = pci_physfn(dev);
>+
>+		if (pdev->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.563.g4f69b47b94-goog
>

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

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Samiullah Khawaja]

On Tue, May 19, 2026 at 11:59:47AM -0300, Jason Gunthorpe wrote:
>On Tue, May 19, 2026 at 02:55:54PM +0000, Pranjal Shrivastava wrote:
>> On Tue, May 19, 2026 at 11:44:30AM -0300, Jason Gunthorpe wrote:
>> > On Tue, May 19, 2026 at 01:53:22PM +0000, Pranjal Shrivastava wrote:
>> > > @@ -4450,7 +4450,8 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
>> > >  	if (dev_is_pci(dev)) {
>> > >  		unsigned int stu = __ffs(smmu->pgsize_bitmap);
>> > >
>> > > -		pci_prepare_ats(to_pci_dev(dev), stu);
>> > > +		if (!pci_prepare_ats(to_pci_dev(dev), stu))
>> > > +			master->ats_prepared = true;
>> > >  	}
>> >
>> > This should fail not keep going, it is a kernel bug if
>> > pci_prepare_ats() fails.

I agree with this approach, as it captures the bug at right place and we
can fix it. This maybe changes the simantics of prepare_ats(), but I
agreed with this.
>>
>> Are you suggesting it's a kernel bug if the STU doesn't match?
>> IIUC, it can fail because of the following cases (including the PF STU):
>>
>> 	if (!pci_ats_supported(dev))
>> 		return -EINVAL;
>
>It should not be called if ats is not supported, iommu driver bug.

So basically iommu drivers should call pci_ats_supported(dev) before
calling prepare_ats() and probe fails if prepare_ats() fails.
>
>> 	if (WARN_ON(dev->ats_enabled))
>> 		return -EBUSY;
>
>It should not be called if ats is already enabled, iommu driver bug.
>
>> 	if (ps < PCI_ATS_MIN_STU)
>> 		return -EINVAL;
>
>PAGE_SIZE is illegal somehow, build bug.
>
>> 	if (dev->is_virtfn) {
>> 		struct pci_dev *pdev = pci_physfn(dev);
>>
>> 		if (pdev->ats_stu != ps)
>> 			return -EINVAL;
>
>The kernel failed to setup the PF, iommu driver bug.
>
>> If ats isn't supported or if there's a STU mismatch, shuoldn't we just
>> ensure that we don't try enabling ATS for that device?
>
>No, the kernel is broken somehow.
>
>> Returning an error here would fail the device probe.. which seems like a
>> big hammer for devices that can't reliably use ATS..
>
>It has nothing to do with devices, the kernel is broken if it fails.
>
>> We have a special case in SMMUv3 where we pre-allocate stuff in prepare
>> but actually enable ATS in attach_commit, which IMO neeeds to be gated
>> based on client device's ATS capabilities?
>
>Yes, all of this should be gated by pci_ats_supported()
>
>But also failing to enable should translate into a failed attach.
>
>Jason

Sami

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Pranjal Shrivastava]

On Tue, May 19, 2026 at 11:59:47AM -0300, Jason Gunthorpe wrote:
> On Tue, May 19, 2026 at 02:55:54PM +0000, Pranjal Shrivastava wrote:
> > On Tue, May 19, 2026 at 11:44:30AM -0300, Jason Gunthorpe wrote:
> > > On Tue, May 19, 2026 at 01:53:22PM +0000, Pranjal Shrivastava wrote:
> > > > @@ -4450,7 +4450,8 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
> > > >  	if (dev_is_pci(dev)) {
> > > >  		unsigned int stu = __ffs(smmu->pgsize_bitmap);
> > > >  
> > > > -		pci_prepare_ats(to_pci_dev(dev), stu);
> > > > +		if (!pci_prepare_ats(to_pci_dev(dev), stu))
> > > > +			master->ats_prepared = true;
> > > >  	}
> > > 
> > > This should fail not keep going, it is a kernel bug if
> > > pci_prepare_ats() fails.
> > 

[...]
 
> > If ats isn't supported or if there's a STU mismatch, shuoldn't we just
> > ensure that we don't try enabling ATS for that device?
> 
> No, the kernel is broken somehow.
> 
> > Returning an error here would fail the device probe.. which seems like a
> > big hammer for devices that can't reliably use ATS.. 
> 
> It has nothing to do with devices, the kernel is broken if it fails.

Ack. I see your point.. none of these should happen if the kernel isn't
broken somewhere.

> 
> > We have a special case in SMMUv3 where we pre-allocate stuff in prepare
> > but actually enable ATS in attach_commit, which IMO neeeds to be gated 
> > based on client device's ATS capabilities?
> 
> Yes, all of this should be gated by pci_ats_supported()
> 
> But also failing to enable should translate into a failed attach.

Ack. I guess we already check pci_ats_supported during attach_prepare()
while setting the state->ats_enable. Based on this state->ats_enable we
call pci_enable_ats() in attach_commit().

However, I'm thinking what's the right thing to do if pci_enable_ats() 
fails in attach_commit():

1. Fail to attach but unmerging the invs_array entry is complicated.
2. Move the pci_enable_ats() to attach_prepare()
3. Continue without *really* enabling ATS.. 

Thanks,
Praan

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Pranjal Shrivastava]

On Tue, May 19, 2026 at 08:01:55PM +0000, Samiullah Khawaja wrote:
> On Tue, May 19, 2026 at 11:59:47AM -0300, Jason Gunthorpe wrote:
> > On Tue, May 19, 2026 at 02:55:54PM +0000, Pranjal Shrivastava wrote:
> > > On Tue, May 19, 2026 at 11:44:30AM -0300, Jason Gunthorpe wrote:
> > > > On Tue, May 19, 2026 at 01:53:22PM +0000, Pranjal Shrivastava wrote:
> > > > > @@ -4450,7 +4450,8 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev)
> > > > >  	if (dev_is_pci(dev)) {
> > > > >  		unsigned int stu = __ffs(smmu->pgsize_bitmap);
> > > > >
> > > > > -		pci_prepare_ats(to_pci_dev(dev), stu);
> > > > > +		if (!pci_prepare_ats(to_pci_dev(dev), stu))
> > > > > +			master->ats_prepared = true;
> > > > >  	}
> > > >
> > > > This should fail not keep going, it is a kernel bug if
> > > > pci_prepare_ats() fails.
> 
> I agree with this approach, as it captures the bug at right place and we
> can fix it. This maybe changes the simantics of prepare_ats(), but I
> agreed with this.
> > > 
> > > Are you suggesting it's a kernel bug if the STU doesn't match?
> > > IIUC, it can fail because of the following cases (including the PF STU):
> > > 
> > > 	if (!pci_ats_supported(dev))
> > > 		return -EINVAL;
> > 
> > It should not be called if ats is not supported, iommu driver bug.
> 
> So basically iommu drivers should call pci_ats_supported(dev) before
> calling prepare_ats() and probe fails if prepare_ats() fails.

Ack. I'll add those checks to Intel and AMD IOMMU too..
AFAICT, both of those too, don't fail an attach if pci_enable_ats fails
[1][2]. I'll add those in the next version as well.

Thanks,
Praan

[1] https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/iommu/amd/iommu.c#L2373
[2] https://elixir.bootlin.com/linux/v7.1-rc3/source/drivers/iommu/intel/iommu.c#L3340

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Jason Gunthorpe]

On Wed, May 20, 2026 at 02:24:47PM +0000, Pranjal Shrivastava wrote:

> However, I'm thinking what's the right thing to do if pci_enable_ats() 
> fails in attach_commit():
> 
> 1. Fail to attach but unmerging the invs_array entry is complicated.

Is it? We should be able to keep the the original invs and not free
it, just put the pointer back?

> 2. Move the pci_enable_ats() to attach_prepare()

IIRC ATS cannot be enabled until the invs are loaded with ATS
invalidations, hence the issue.

So you'd put the new invs list, turn on ATS, if it fails restore the
invs list back to what it was and RCU free the new one(s).

Jason

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Pranjal Shrivastava]

On Wed, May 20, 2026 at 11:51:42AM -0300, Jason Gunthorpe wrote:
> On Wed, May 20, 2026 at 02:24:47PM +0000, Pranjal Shrivastava wrote:
> 
> > However, I'm thinking what's the right thing to do if pci_enable_ats() 
> > fails in attach_commit():
> > 
> > 1. Fail to attach but unmerging the invs_array entry is complicated.
> 
> Is it? We should be able to keep the the original invs and not free
> it, just put the pointer back?

Reverting the invs pointer itself is straightforward, but IIUC the 
current implementation of arm_smmu_install_new_domain_invs() is a
destructive update as it calls kfree_rcu() on the old list immediately
after the swap.

Although, I guess we could refactor the RCU lifecycle to defer the 
kfree_rcu until after pci_enable_ats() succeeds in the commit phase?

> 
> > 2. Move the pci_enable_ats() to attach_prepare()
> 
> IIRC ATS cannot be enabled until the invs are loaded with ATS
> invalidations, hence the issue.

Ahh, right.
> 
> So you'd put the new invs list, turn on ATS, if it fails restore the
> invs list back to what it was and RCU free the new one(s).

I guess we'll have to to do the following:

1. Defer kfree_rcu of the old invs until attach_commit succeeds.
2. Implement a roll-back (writing the old STE/CD back) on enable failiure

Thanks,
Praan

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Jason Gunthorpe]

On Wed, May 20, 2026 at 04:24:04PM +0000, Pranjal Shrivastava wrote:
> On Wed, May 20, 2026 at 11:51:42AM -0300, Jason Gunthorpe wrote:
> > On Wed, May 20, 2026 at 02:24:47PM +0000, Pranjal Shrivastava wrote:
> > 
> > > However, I'm thinking what's the right thing to do if pci_enable_ats() 
> > > fails in attach_commit():
> > > 
> > > 1. Fail to attach but unmerging the invs_array entry is complicated.
> > 
> > Is it? We should be able to keep the the original invs and not free
> > it, just put the pointer back?
> 
> Reverting the invs pointer itself is straightforward, but IIUC the 
> current implementation of arm_smmu_install_new_domain_invs() is a
> destructive update as it calls kfree_rcu() on the old list immediately
> after the swap.
> 
> Although, I guess we could refactor the RCU lifecycle to defer the 
> kfree_rcu until after pci_enable_ats() succeeds in the commit phase?

Yeah..

Though really pci_enable_ats is not working the way we want, writing
the enable register can't actually fail - it is all the sanity
checking for kernel bugs that is the problem here.

How about call pci_ats_supported() early in attach and fail, then if
pci_enable_ats() fails just WARN_ON and keep going, kernel bug.

Re-organize pci_enable_ats() so it relies on pci_ats_supported() for
all the sanity checks and cannot fail if supported is true.

??

Jason

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Pranjal Shrivastava]

On Wed, May 20, 2026 at 01:31:17PM -0300, Jason Gunthorpe wrote:
> On Wed, May 20, 2026 at 04:24:04PM +0000, Pranjal Shrivastava wrote:
> > On Wed, May 20, 2026 at 11:51:42AM -0300, Jason Gunthorpe wrote:
> > > On Wed, May 20, 2026 at 02:24:47PM +0000, Pranjal Shrivastava wrote:
> > > 
> > > > However, I'm thinking what's the right thing to do if pci_enable_ats() 
> > > > fails in attach_commit():
> > > > 
> > > > 1. Fail to attach but unmerging the invs_array entry is complicated.
> > > 
> > > Is it? We should be able to keep the the original invs and not free
> > > it, just put the pointer back?
> > 
> > Reverting the invs pointer itself is straightforward, but IIUC the 
> > current implementation of arm_smmu_install_new_domain_invs() is a
> > destructive update as it calls kfree_rcu() on the old list immediately
> > after the swap.
> > 
> > Although, I guess we could refactor the RCU lifecycle to defer the 
> > kfree_rcu until after pci_enable_ats() succeeds in the commit phase?
> 
> Yeah..
> 
> Though really pci_enable_ats is not working the way we want, writing
> the enable register can't actually fail - it is all the sanity
> checking for kernel bugs that is the problem here.
> 
> How about call pci_ats_supported() early in attach and fail, then if
> pci_enable_ats() fails just WARN_ON and keep going, kernel bug.
> 
> Re-organize pci_enable_ats() so it relies on pci_ats_supported() for
> all the sanity checks and cannot fail if supported is true.

Hmm, IIUC, the current situation says: 

pci_ats_supported ==> ATS cap check
pci_prepare_ats ==> ATS enablement prep 

Thus, I feel if the prepare fails, we should expect enable to fail. But
if prepare succeeds but enable didn't, that's the place we should
WARN_ON and move on.. because we (IOMMU) did the best we could have..

With the updated variants of pci_ats_support / pci_prepare_ats (in
patches 1 & 2), I believe we cover all the cases now..

And we plan to call prepare_ats only if (pci_ats_supported == true)

Thus, if prepare_ats failed, we fail at probe. And if probe succeeds but
enable ATS failed, that's something the IOMMU can't help with, hence we
WARN and move on? Does that sound good?

Thanks,
Praan

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Jason Gunthorpe]

On Fri, May 22, 2026 at 04:14:01PM +0000, Pranjal Shrivastava wrote:
> > 
> > Though really pci_enable_ats is not working the way we want, writing
> > the enable register can't actually fail - it is all the sanity
> > checking for kernel bugs that is the problem here.
> > 
> > How about call pci_ats_supported() early in attach and fail, then if
> > pci_enable_ats() fails just WARN_ON and keep going, kernel bug.
> > 
> > Re-organize pci_enable_ats() so it relies on pci_ats_supported() for
> > all the sanity checks and cannot fail if supported is true.
> 
> Hmm, IIUC, the current situation says: 
> 
> pci_ats_supported ==> ATS cap check
> pci_prepare_ats ==> ATS enablement prep 
> 
> Thus, I feel if the prepare fails, we should expect enable to fail. But
> if prepare succeeds but enable didn't, that's the place we should
> WARN_ON and move on.. because we (IOMMU) did the best we could have..
> 
> With the updated variants of pci_ats_support / pci_prepare_ats (in
> patches 1 & 2), I believe we cover all the cases now..
> 
> And we plan to call prepare_ats only if (pci_ats_supported == true)
> 
> Thus, if prepare_ats failed, we fail at probe. And if probe succeeds but
> enable ATS failed, that's something the IOMMU can't help with, hence we
> WARN and move on? Does that sound good?

OK, what I really want to avoid is wrecking the attach flow. It is
designed so the failable things happen early and after a certain point
failure is no longer permitted, the enable_ats is after the point.

So it should continue to be structued like that - enable_ats
failing is a kernel bug because earlier checks preclude it..

Jason

Re: [PATCH v3 3/3] iommu/arm-smmu-v3: Fix ATS state tracking via ats_prepared gate

[Pranjal Shrivastava]

On Sat, May 23, 2026 at 09:34:18AM -0300, Jason Gunthorpe wrote:
> On Fri, May 22, 2026 at 04:14:01PM +0000, Pranjal Shrivastava wrote:
> > > 
> > > Though really pci_enable_ats is not working the way we want, writing
> > > the enable register can't actually fail - it is all the sanity
> > > checking for kernel bugs that is the problem here.
> > > 
> > > How about call pci_ats_supported() early in attach and fail, then if
> > > pci_enable_ats() fails just WARN_ON and keep going, kernel bug.
> > > 
> > > Re-organize pci_enable_ats() so it relies on pci_ats_supported() for
> > > all the sanity checks and cannot fail if supported is true.
> > 
> > Hmm, IIUC, the current situation says: 
> > 
> > pci_ats_supported ==> ATS cap check
> > pci_prepare_ats ==> ATS enablement prep 
> > 
> > Thus, I feel if the prepare fails, we should expect enable to fail. But
> > if prepare succeeds but enable didn't, that's the place we should
> > WARN_ON and move on.. because we (IOMMU) did the best we could have..
> > 
> > With the updated variants of pci_ats_support / pci_prepare_ats (in
> > patches 1 & 2), I believe we cover all the cases now..
> > 
> > And we plan to call prepare_ats only if (pci_ats_supported == true)
> > 
> > Thus, if prepare_ats failed, we fail at probe. And if probe succeeds but
> > enable ATS failed, that's something the IOMMU can't help with, hence we
> > WARN and move on? Does that sound good?
> 
> OK, what I really want to avoid is wrecking the attach flow. It is
> designed so the failable things happen early and after a certain point
> failure is no longer permitted, the enable_ats is after the point.
> 
> So it should continue to be structued like that - enable_ats
> failing is a kernel bug because earlier checks preclude it..

Ack. I see your point about maintaining the integrity of the attach flow
Will post another version

Thanks,
Praan