Re: [PATCH] PCI: Fix Intel Xeon 6 x2 quirk collateral damage on adjacent x4 endpoints
Rick Warner <[email protected]>
| Newsgroups | gmane.linux.kernel,gmane.linux.kernel.pci |
|---|---|
| Message-ID | <[email protected]> |
Hi All,
Here's an updated version of the fix. Now in the early setup it sets
no_inc_mrrs on the entire bridge instead of disabling ext tags and
limiting mrrs for the entire root bridge. Then it uses
DECLARE_PCI_FIXUP_HEADER to check every registering pci device to see if
it's a descendant of an x2 port, and if so, disables ext tags and limits
mrrs for that device. This should properly handle hotplugging now.
This has been tested on a Gigabyte MS74-HB0 motherboard with a Seagate
ZP4000GM30063 M.2 drive. With the stock kernel, that drive fails to
initialize and is unavailable once booted. Testing showed that extended
tags are required for it to work. The drive works properly with this
patch. Concerns were mentioned about vfio/VM usage being able to
re-enable extended tags if bridge->no_ext_tags is not set. I'm not sure
how to address that without adding additional a flag to pci device
structures that individually blocks extended tags per device instead of
relying on the bridge flag for it. I'm open to suggestions for that.
Thanks,
Rick Warner
On 7/29/26 2:43 PM, Lukas Wunner wrote:
> On Wed, Jul 29, 2026 at 10:45:08AM -0400, Rick Warner wrote:
>> commit a22250fe933d ("PCI: Add Extended Tag + MRRS quirk for Xeon 6")
>> introduced a traffic mitigation quirk for Intel Xeon 6 root ports that
>> negotiate down to an x2 lane width. However, that patch manipulated host
>> bridge properties globally via 'bridge->no_ext_tags = 1' and by hooking
>> 'bridge->enable_device'.
>>
>> Because multiple unrelated root ports can reside under the exact same
>> global pci_host_bridge domain structure, this overly aggressive mitigation
>> causes severe collateral damage. When a low-speed or bifurcated secondary
>> device (such as an onboard ASMedia SATA controller or BMC graphics link)
>> matches the x2 condition, the kernel strips away Extended Tags and forces
>> a 128B MRRS restriction across that ENTIRE host bridge. This instantly
>> breaks or starves adjacent high-performance, unrelated x4 endpoints
>> (such as NVMe drives), resulting in controller timeouts, initialization
>> failures, and missing drives at boot.
>>
>> Fix this by refactoring the quirk logic to be completely per-device and
>> downstream-isolated. Remove the broad host bridge structure assignments
>> entirely. Instead, convert the hook to an explicit
>> 'DECLARE_PCI_FIXUP_FINAL' sweep. When an x2 bifurcated root port is
>> discovered, leverage 'pci_walk_bus()' to step down only that specific root
>> port's subordinate tree, manually clearing 'PCI_EXP_DEVCTL_EXT_TAG' and
>> 'PCI_EXP_DEVCTL_READRQ' directly in the endpoint Device Control
>> configuration registers.
> One problem I see with this approach is that it won't work for hotplugged
> devices below a bifurcated Root Port: You're only adjusting Extended Tags
> and MRRS once on enumeration of the Root Ports, leaving devices that are
> hotplugged later at incorrect settings.
>
> But perhaps you could simply amend limit_mrrs_to_128() to walk up to the
> Root Port, check whether it is bifurcated, and bail out if it's not?
>
> Thanks,
>
> Lukas
x2-quirk-disable-extended-tags-and-limit-mrrs-per-device.patch
(text/x-patch, 2.9 KB)
diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index b301c6c8df75..0ab92a5cc0d1 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -301,15 +301,6 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_MCH_PC1, pcie_r
*
* https://cdrdv2.intel.com/v1/dl/getContent/837176
*/
-static int limit_mrrs_to_128(struct pci_host_bridge *b, struct pci_dev *pdev)
-{
- int readrq = pcie_get_readrq(pdev);
-
- if (readrq > 128)
- pcie_set_readrq(pdev, 128);
-
- return 0;
-}
static void pci_xeon_x2_bifurc_quirk(struct pci_dev *pdev)
{
@@ -320,9 +311,8 @@ static void pci_xeon_x2_bifurc_quirk(struct pci_dev *pdev)
if (FIELD_GET(PCI_EXP_LNKCAP_MLW, linkcap) != 0x2)
return;
- bridge->no_ext_tags = 1;
- bridge->enable_device = limit_mrrs_to_128;
- pci_info(pdev, "Disabling Extended Tags and limiting MRRS to 128B (performance reasons due to x2 PCIe link)\n");
+ bridge->no_inc_mrrs = 1;
+ pci_info(pdev, "Blocking devices on this bridge from increasing MRRS for performance reasons due to x2 PCIe link)\n");
}
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db0, pci_xeon_x2_bifurc_quirk);
@@ -334,6 +324,50 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db7, pci_xeon_x2_bifurc_quirk);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db8, pci_xeon_x2_bifurc_quirk);
DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db9, pci_xeon_x2_bifurc_quirk);
+/* Helper to check if a device descends from an affected Xeon 6 x2 Root Port */
+static bool is_descendant_of_xeon6_x2_rp(struct pci_dev *pdev)
+{
+ u32 linkcap;
+ struct pci_dev *upstream = pci_upstream_bridge(pdev);
+
+ while (upstream) {
+ if (upstream->vendor == PCI_VENDOR_ID_INTEL &&
+ upstream->device >= 0x0db0 &&
+ upstream->device <= 0x0db9 ) {
+ pcie_capability_read_dword(upstream, PCI_EXP_LNKCAP, &linkcap);
+ if (FIELD_GET(PCI_EXP_LNKCAP_MLW, linkcap) != 0x2)
+ return false;
+ else
+ return true;
+ }
+ upstream = pci_upstream_bridge(upstream);
+ }
+ return false;
+}
+
+static void pci_xeon6_x2_local_endpoint_fixup(struct pci_dev *pdev)
+{
+ /* Skip bridges/switches; only target actual endpoints */
+ if (pci_is_bridge(pdev))
+ return;
+
+ /* Only apply to devices under the x2 branch; leaves x4 branches completely untouched */
+ if (!is_descendant_of_xeon6_x2_rp(pdev))
+ return;
+
+ pci_info(pdev, "Applying local Xeon 6 x2 quirk: Disabling Extended Tags and locking MRRS to 128B\n");
+
+ pcie_capability_clear_word(pdev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_EXT_TAG);
+ pcie_capability_clear_word(pdev, PCI_EXP_DEVCTL, PCI_EXP_DEVCTL_READRQ);
+}
+
+/*
+ * HEADER fixups run for EVERY endpoint during its initial discovery phase.
+ * This natively catches boot devices, hotplugged devices, and SR-IOV VFs.
+ */
+DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_xeon6_x2_local_endpoint_fixup);
+
+
/*
* Fixup to mark boot BIOS video selected by BIOS before it changes
*