[PATCH] PCI: Disable ATS via quirk before notifying IOMMU drivers

[PATCH] PCI: Disable ATS via quirk before notifying IOMMU drivers

[David Matlack]

Ensure that PCI devices that have ATS disabled via quirk have it
disabled before IOMMU drivers are notified about the device. Otherwise
the IOMMU driver will see that the device has ATS enabled during probing
and then later it will get disabled.

This fixes at least one bug in the Intel IOMMU driver where it adds the
device to an rbtree because it sees ATS is enabled, but then ATS gets
disabled via quirk. When the device is destroyed (e.g. hot-unplug, VF
destruction, etc.) the driver sees that ATS is disabled and does not
remove it from the rbtree. This inevitably leads to a use-after-free
and corruption of the rbtree.

Fix this by disabling ATS via quirk during "early" fixups instead of
"final" fixups.

Fixes: a18615b1cfc0 ("PCI: Disable ATS for specific Intel IPU E2000 devices")
Closes: https://lore.kernel.org/linux-iommu/aYUQ_HkDJU9kjsUl@google.com/
Cc: Raghavendra Rao Ananta <rananta@google.com>
Cc: David Woodhouse <dwmw2@infradead.org>
Cc: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: David Matlack <dmatlack@google.com>
---
 drivers/pci/ats.c    |  3 +++
 drivers/pci/quirks.c | 50 ++++++++++++++++++++++----------------------
 include/linux/pci.h  |  1 +
 3 files changed, 29 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
index ec6c8dbdc5e9..85306198c232 100644
--- a/drivers/pci/ats.c
+++ b/drivers/pci/ats.c
@@ -24,6 +24,9 @@ void pci_ats_init(struct pci_dev *dev)
 	if (pci_ats_disabled())
 		return;
 
+	if (dev->no_ats)
+		return;
+
 	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ATS);
 	if (!pos)
 		return;
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 48946cca4be7..2c7e11830e45 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -5653,7 +5653,7 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0422, quirk_no_ext_tags);
 static void quirk_no_ats(struct pci_dev *pdev)
 {
 	pci_info(pdev, "disabling ATS\n");
-	pdev->ats_cap = 0;
+	pdev->no_ats = 1;
 }
 
 /*
@@ -5676,25 +5676,25 @@ static void quirk_amd_harvest_no_ats(struct pci_dev *pdev)
 }
 
 /* AMD Stoney platform GPU */
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x98e4, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x98e4, quirk_amd_harvest_no_ats);
 /* AMD Iceland dGPU */
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x6900, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x6900, quirk_amd_harvest_no_ats);
 /* AMD Navi10 dGPU */
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7310, quirk_amd_harvest_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7312, quirk_amd_harvest_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7318, quirk_amd_harvest_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7319, quirk_amd_harvest_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x731a, quirk_amd_harvest_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x731b, quirk_amd_harvest_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x731e, quirk_amd_harvest_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x731f, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x7310, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x7312, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x7318, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x7319, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x731a, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x731b, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x731e, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x731f, quirk_amd_harvest_no_ats);
 /* AMD Navi14 dGPU */
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7340, quirk_amd_harvest_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7341, quirk_amd_harvest_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7347, quirk_amd_harvest_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x734f, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x7340, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x7341, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x7347, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x734f, quirk_amd_harvest_no_ats);
 /* AMD Raven platform iGPU */
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x15d8, quirk_amd_harvest_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_ATI, 0x15d8, quirk_amd_harvest_no_ats);
 
 /*
  * Intel IPU E2000 revisions before C0 implement incorrect endianness
@@ -5705,15 +5705,15 @@ static void quirk_intel_e2000_no_ats(struct pci_dev *pdev)
 	if (pdev->revision < 0x20)
 		quirk_no_ats(pdev);
 }
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1451, quirk_intel_e2000_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1452, quirk_intel_e2000_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1453, quirk_intel_e2000_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1454, quirk_intel_e2000_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1455, quirk_intel_e2000_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1457, quirk_intel_e2000_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x1459, quirk_intel_e2000_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x145a, quirk_intel_e2000_no_ats);
-DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x145c, quirk_intel_e2000_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1451, quirk_intel_e2000_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1452, quirk_intel_e2000_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1453, quirk_intel_e2000_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1454, quirk_intel_e2000_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1455, quirk_intel_e2000_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1457, quirk_intel_e2000_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x1459, quirk_intel_e2000_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x145a, quirk_intel_e2000_no_ats);
+DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x145c, quirk_intel_e2000_no_ats);
 #endif /* CONFIG_PCI_ATS */
 
 /* Freescale PCIe doesn't support MSI in RC mode */
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1c270f1d5123..5880942bba65 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -537,6 +537,7 @@ struct pci_dev {
 		struct pci_sriov	*sriov;		/* PF: SR-IOV info */
 		struct pci_dev		*physfn;	/* VF: related PF */
 	};
+	unsigned int	no_ats:1;	/* ATS disabled via quirk */
 	u16		ats_cap;	/* ATS Capability offset */
 	u8		ats_stu;	/* ATS Smallest Translation Unit */
 #endif

base-commit: a95f71ad3e2e224277508e006580c333d0a5fe36
-- 
2.53.0.371.g1d285c8824-goog

Re: [PATCH] PCI: Disable ATS via quirk before notifying IOMMU drivers

[Andy Shevchenko]

On Mon, Feb 23, 2026 at 06:40:16PM +0000, David Matlack wrote:
> Ensure that PCI devices that have ATS disabled via quirk have it
> disabled before IOMMU drivers are notified about the device. Otherwise
> the IOMMU driver will see that the device has ATS enabled during probing
> and then later it will get disabled.
> 
> This fixes at least one bug in the Intel IOMMU driver where it adds the
> device to an rbtree because it sees ATS is enabled, but then ATS gets
> disabled via quirk. When the device is destroyed (e.g. hot-unplug, VF
> destruction, etc.) the driver sees that ATS is disabled and does not
> remove it from the rbtree. This inevitably leads to a use-after-free
> and corruption of the rbtree.
> 
> Fix this by disabling ATS via quirk during "early" fixups instead of
> "final" fixups.

Hmm... Sounds to me like a premature disablement, but I leave it the experts.
What I think about the case, that IOMMU should be probably fixed to avoid such
situation for all level of quirks. Can it be feasible?

> Fixes: a18615b1cfc0 ("PCI: Disable ATS for specific Intel IPU E2000 devices")
> Closes: https://lore.kernel.org/linux-iommu/aYUQ_HkDJU9kjsUl@google.com/

> Cc: Raghavendra Rao Ananta <rananta@google.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Lu Baolu <baolu.lu@linux.intel.com>

These...

> Signed-off-by: David Matlack <dmatlack@google.com>
> ---

...may go here with the same effect on email, but reducing the unneeded noise
in the actual Git history.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] PCI: Disable ATS via quirk before notifying IOMMU drivers

[David Matlack]

On Mon, Feb 23, 2026 at 12:37 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Mon, Feb 23, 2026 at 06:40:16PM +0000, David Matlack wrote:
> > Ensure that PCI devices that have ATS disabled via quirk have it
> > disabled before IOMMU drivers are notified about the device. Otherwise
> > the IOMMU driver will see that the device has ATS enabled during probing
> > and then later it will get disabled.
> >
> > This fixes at least one bug in the Intel IOMMU driver where it adds the
> > device to an rbtree because it sees ATS is enabled, but then ATS gets
> > disabled via quirk. When the device is destroyed (e.g. hot-unplug, VF
> > destruction, etc.) the driver sees that ATS is disabled and does not
> > remove it from the rbtree. This inevitably leads to a use-after-free
> > and corruption of the rbtree.
> >
> > Fix this by disabling ATS via quirk during "early" fixups instead of
> > "final" fixups.
>
> Hmm... Sounds to me like a premature disablement, but I leave it the experts.

What do you mean by "premature disablement"?

> What I think about the case, that IOMMU should be probably fixed to avoid such
> situation for all level of quirks. Can it be feasible?

What do you mean by the "IOMMU should be fixed"? Are you saying the
IOMMU should be prepared to handle quirks disabling features on
devices after the IOMMU driver is notified about a device?

>
> > Fixes: a18615b1cfc0 ("PCI: Disable ATS for specific Intel IPU E2000 devices")
> > Closes: https://lore.kernel.org/linux-iommu/aYUQ_HkDJU9kjsUl@google.com/
>
> > Cc: Raghavendra Rao Ananta <rananta@google.com>
> > Cc: David Woodhouse <dwmw2@infradead.org>
> > Cc: Lu Baolu <baolu.lu@linux.intel.com>
>
> These...
>
> > Signed-off-by: David Matlack <dmatlack@google.com>
> > ---
>
> ...may go here with the same effect on email, but reducing the unneeded noise
> in the actual Git history.

Ahh, will do. Thanks for the tip.

Re: [PATCH] PCI: Disable ATS via quirk before notifying IOMMU drivers

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 09:19:05AM -0800, David Matlack wrote:
> On Mon, Feb 23, 2026 at 12:37 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Feb 23, 2026 at 06:40:16PM +0000, David Matlack wrote:

> > > Fix this by disabling ATS via quirk during "early" fixups instead of
> > > "final" fixups.
> >
> > Hmm... Sounds to me like a premature disablement, but I leave it the experts.
> 
> What do you mean by "premature disablement"?

On early stage instead of final stage.


> > What I think about the case, that IOMMU should be probably fixed to avoid such
> > situation for all level of quirks. Can it be feasible?
> 
> What do you mean by the "IOMMU should be fixed"? Are you saying the
> IOMMU should be prepared to handle quirks disabling features on
> devices after the IOMMU driver is notified about a device?

Something like this, yes. At least the commit message is unclear why
"This fixes at least one bug in the Intel IOMMU driver..." not in IOMMU
driver code.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] PCI: Disable ATS via quirk before notifying IOMMU drivers

[David Matlack]

On Tue, Feb 24, 2026 at 9:25 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Feb 24, 2026 at 09:19:05AM -0800, David Matlack wrote:
> > On Mon, Feb 23, 2026 at 12:37 PM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Mon, Feb 23, 2026 at 06:40:16PM +0000, David Matlack wrote:
>
> > > > Fix this by disabling ATS via quirk during "early" fixups instead of
> > > > "final" fixups.
> > >
> > > Hmm... Sounds to me like a premature disablement, but I leave it the experts.
> >
> > What do you mean by "premature disablement"?
>
> On early stage instead of final stage.

Is your concern that applying the quirk at early stage won't be
effective because ATS will be enabled after early fixups are applied?
To prevent that the patch adds a no_ats bit that blocks ATS enablement
after early fixups.

If not that, I guess I am wondering what is your concern from a
functional level.

> > > What I think about the case, that IOMMU should be probably fixed to avoid such
> > > situation for all level of quirks. Can it be feasible?
> >
> > What do you mean by the "IOMMU should be fixed"? Are you saying the
> > IOMMU should be prepared to handle quirks disabling features on
> > devices after the IOMMU driver is notified about a device?
>
> Something like this, yes. At least the commit message is unclear why
> "This fixes at least one bug in the Intel IOMMU driver..." not in IOMMU
> driver code.

Gotcha. It felt wrong to have the IOMMU driver be notified about a
device with ATS enabled and then have ATS later disabled. It seem like
it would add complexity to the IOMMU drivers to handle such a case,
and would be much simpler to have ATS in its final state when the
IOMMU driver is notified about the device being created.

But maybe Lu Baolu and David Woodhouse (Intel IOMMU maintainers) can
comment if they prefer to handle this in the Intel IOMMU driver.

Re: [PATCH] PCI: Disable ATS via quirk before notifying IOMMU drivers

[Andy Shevchenko]

On Tue, Feb 24, 2026 at 09:41:30AM -0800, David Matlack wrote:
> On Tue, Feb 24, 2026 at 9:25 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Tue, Feb 24, 2026 at 09:19:05AM -0800, David Matlack wrote:
> > > On Mon, Feb 23, 2026 at 12:37 PM Andy Shevchenko
> > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > On Mon, Feb 23, 2026 at 06:40:16PM +0000, David Matlack wrote:

...

> > > > > Fix this by disabling ATS via quirk during "early" fixups instead of
> > > > > "final" fixups.
> > > >
> > > > Hmm... Sounds to me like a premature disablement, but I leave it the experts.
> > >
> > > What do you mean by "premature disablement"?
> >
> > On early stage instead of final stage.
> 
> Is your concern that applying the quirk at early stage won't be
> effective because ATS will be enabled after early fixups are applied?

My concern that applying this quirk too early may affect something else.
But I'm not an expert in the PCI mysterious ways, I just share my feelings.

> To prevent that the patch adds a no_ats bit that blocks ATS enablement
> after early fixups.
> 
> If not that, I guess I am wondering what is your concern from a
> functional level.
> 
> > > > What I think about the case, that IOMMU should be probably fixed to avoid such
> > > > situation for all level of quirks. Can it be feasible?
> > >
> > > What do you mean by the "IOMMU should be fixed"? Are you saying the
> > > IOMMU should be prepared to handle quirks disabling features on
> > > devices after the IOMMU driver is notified about a device?
> >
> > Something like this, yes. At least the commit message is unclear why
> > "This fixes at least one bug in the Intel IOMMU driver..." not in IOMMU
> > driver code.
> 
> Gotcha. It felt wrong to have the IOMMU driver be notified about a
> device with ATS enabled and then have ATS later disabled. It seem like
> it would add complexity to the IOMMU drivers to handle such a case,
> and would be much simpler to have ATS in its final state when the
> IOMMU driver is notified about the device being created.

Can this be elaborated in the commit message? Then a reviewer will not
have questions like me.

> But maybe Lu Baolu and David Woodhouse (Intel IOMMU maintainers) can
> comment if they prefer to handle this in the Intel IOMMU driver.

I agree that his input may be valuable in this discussion.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] PCI: Disable ATS via quirk before notifying IOMMU drivers

[David Matlack]

On Wed, Feb 25, 2026 at 1:43 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Tue, Feb 24, 2026 at 09:41:30AM -0800, David Matlack wrote:
> > On Tue, Feb 24, 2026 at 9:25 AM Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > On Tue, Feb 24, 2026 at 09:19:05AM -0800, David Matlack wrote:
> > > > On Mon, Feb 23, 2026 at 12:37 PM Andy Shevchenko
> > > > <andriy.shevchenko@linux.intel.com> wrote:
> > > > > On Mon, Feb 23, 2026 at 06:40:16PM +0000, David Matlack wrote:
>
> ...
>
> > > > > > Fix this by disabling ATS via quirk during "early" fixups instead of
> > > > > > "final" fixups.
> > > > >
> > > > > Hmm... Sounds to me like a premature disablement, but I leave it the experts.
> > > >
> > > > What do you mean by "premature disablement"?
> > >
> > > On early stage instead of final stage.
> >
> > Is your concern that applying the quirk at early stage won't be
> > effective because ATS will be enabled after early fixups are applied?
>
> My concern that applying this quirk too early may affect something else.
> But I'm not an expert in the PCI mysterious ways, I just share my feelings.

Ahh ok, gotcha. Thanks for the raising the concern.

> > > > > What I think about the case, that IOMMU should be probably fixed to avoid such
> > > > > situation for all level of quirks. Can it be feasible?
> > > >
> > > > What do you mean by the "IOMMU should be fixed"? Are you saying the
> > > > IOMMU should be prepared to handle quirks disabling features on
> > > > devices after the IOMMU driver is notified about a device?
> > >
> > > Something like this, yes. At least the commit message is unclear why
> > > "This fixes at least one bug in the Intel IOMMU driver..." not in IOMMU
> > > driver code.
> >
> > Gotcha. It felt wrong to have the IOMMU driver be notified about a
> > device with ATS enabled and then have ATS later disabled. It seem like
> > it would add complexity to the IOMMU drivers to handle such a case,
> > and would be much simpler to have ATS in its final state when the
> > IOMMU driver is notified about the device being created.
>
> Can this be elaborated in the commit message? Then a reviewer will not
> have questions like me.

Will do in v2, thanks for the suggestion.