From: Bjorn Helgaas <helgaas@kernel.org>
To: David Matlack <dmatlack@google.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
Alexander Lobakin <aleksander.lobakin@intel.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Bartosz Pawlowski <bartosz.pawlowski@intel.com>,
David Woodhouse <dwmw2@infradead.org>,
linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
Lu Baolu <baolu.lu@linux.intel.com>,
Raghavendra Rao Ananta <rananta@google.com>
Subject: Re: [PATCH v2] PCI: Disable ATS via quirk before notifying IOMMU drivers
Date: Mon, 30 Mar 2026 17:30:02 -0500 [thread overview]
Message-ID: <20260330223002.GA111329@bhelgaas> (raw)
In-Reply-To: <20260327211649.3816010-1-dmatlack@google.com>
On Fri, Mar 27, 2026 at 09:16:40PM +0000, David Matlack wrote:
> Ensure that PCI devices with ATS disabled via quirk have it disabled
> before IOMMU drivers are notified about the device rather than after.
> Fix this by converting the existing quirks from final to early fixups
> and changing the quirk logic to set a new no_ats bit in struct pci_dev
> that prevents pci_dev.ats_cap from ever gettting set.
>
> This change ensures that pci_ats_supported() takes quirks into account
> during iommu_ops.probe_device(), when IOMMU drivers are first notified
> about devices. It also ensures that pci_ats_supported() returns the same
> value when the device is released in iommu_ops.release_device().
>
> Notably, the Intel IOMMU driver uses pci_ats_supported() in
> probe/release to determine whether to add/remove a device from a data
> structure, which easily leads to a use-after-free without this fix.
>
> This change also makes disabling ATS via quirk behave the same way as
> the pci=noats command line option, in that pci_ats_init() bails
> immediately and never intializes pci_dev.ats_cap.
>
> Fixes: a18615b1cfc0 ("PCI: Disable ATS for specific Intel IPU E2000 devices")
> Closes: https://lore.kernel.org/linux-iommu/aYUQ_HkDJU9kjsUl@google.com/
> Signed-off-by: David Matlack <dmatlack@google.com>
Applied to pci/enumeration for v7.1, thank you!
> ---
> v2:
> - Update the commit message with reasons why this is being fixed in the
> PCI core, rather than applying a point fix to the Intel IOMMU driver
> (Andy)
> - Condense the pci_ats_disabled() and dev->no_ats checks into a single
> line in pci_ats_init()
> - Reorder the no_ats bitfield to be after ats_stu since there is likely
> u8-sized gap there for alignment purposes
>
> v1: https://lore.kernel.org/linux-pci/20260223184017.688212-1-dmatlack@google.com/
>
> Cc: Raghavendra Rao Ananta <rananta@google.com>
> Cc: David Woodhouse <dwmw2@infradead.org>
> Cc: Lu Baolu <baolu.lu@linux.intel.com>
> Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> drivers/pci/ats.c | 2 +-
> drivers/pci/quirks.c | 50 ++++++++++++++++++++++----------------------
> include/linux/pci.h | 1 +
> 3 files changed, 27 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index ec6c8dbdc5e9..ceb6f5d3cb10 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -21,7 +21,7 @@ void pci_ats_init(struct pci_dev *dev)
> {
> int pos;
>
> - if (pci_ats_disabled())
> + if (pci_ats_disabled() || dev->no_ats)
> return;
>
> pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ATS);
> 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..850100f209e3 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -539,6 +539,7 @@ struct pci_dev {
> };
> u16 ats_cap; /* ATS Capability offset */
> u8 ats_stu; /* ATS Smallest Translation Unit */
> + unsigned int no_ats:1; /* ATS disabled via quirk */
> #endif
> #ifdef CONFIG_PCI_PRI
> u16 pri_cap; /* PRI Capability offset */
>
> base-commit: 7df48e36313029e4c0907b2023905dd7213fd678
> --
> 2.53.0.1018.g2bb0e51243-goog
>
next prev parent reply other threads:[~2026-03-30 22:30 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-27 21:16 [PATCH v2] PCI: Disable ATS via quirk before notifying IOMMU drivers David Matlack
2026-03-30 22:30 ` Bjorn Helgaas [this message]
2026-03-31 3:31 ` Baolu Lu
2026-03-31 15:32 ` David Matlack
2026-04-02 21:02 ` David Matlack
2026-03-31 16:01 ` David Matlack
2026-03-31 18:38 ` Bjorn Helgaas
2026-03-31 18:46 ` David Matlack
2026-03-31 19:00 ` Andy Shevchenko
2026-03-31 19:02 ` Bjorn Helgaas
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260330223002.GA111329@bhelgaas \
--to=helgaas@kernel.org \
--cc=aleksander.lobakin@intel.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=baolu.lu@linux.intel.com \
--cc=bartosz.pawlowski@intel.com \
--cc=bhelgaas@google.com \
--cc=dmatlack@google.com \
--cc=dwmw2@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=rananta@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox