[PATCH] PCI: Fix Intel Xeon 6 x2 quirk collateral damage on adjacent x4 endpoints

[PATCH] PCI: Fix Intel Xeon 6 x2 quirk collateral damage on adjacent x4 endpoints

[Rick Warner]

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. This enforces Intel's stability parameters
locally on the bottlenecked lane branches while fully protecting the
parallel x4 highway channels.

Fixes: a22250fe933d ("PCI: Add Extended Tag + MRRS quirk for Xeon 6")
Signed-off-by: Rick Warner <rick@microway.com>
---
 arch/x86/pci/fixup.c | 41 +++++++++++++++++++++++------------------
 1 file changed, 23 insertions(+), 18 deletions(-)

diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
index b301c6c8df75..c2bdf580692a 100644
--- a/arch/x86/pci/fixup.c
+++ b/arch/x86/pci/fixup.c
@@ -301,38 +301,43 @@ 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)
+static int pci_xeon_x2_bifurc_quirk_perdev(struct pci_dev *pdev, void *data)
 {
-	int readrq = pcie_get_readrq(pdev);
-
-	if (readrq > 128)
-		pcie_set_readrq(pdev, 128);
-
+	/* Only clamp actual endpoints, skip downstream switches/bridges */
+	if (!pci_is_bridge(pdev)) {
+		pci_info(pdev, "Disabling Extended Tags and Clamping MRRS to 128B due to upstream Xeon 6 x2 bifurcation\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);
+	}
 	return 0;
 }
 
 static void pci_xeon_x2_bifurc_quirk(struct pci_dev *pdev)
 {
-	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
 	u32 linkcap;
 
 	pcie_capability_read_dword(pdev, PCI_EXP_LNKCAP, &linkcap);
 	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");
+	pci_info(pdev, "Xeon 6 x2 lane bifurcation detected; restricting subordinate devices\n");
+
+	/*
+	 * Walk down only this specific root port's downstream bus tree
+	 * and clear extended tags specifically for devices on this branch.
+	 */
+	if (pdev->subordinate)
+		pci_walk_bus(pdev->subordinate, pci_xeon_x2_bifurc_quirk_perdev, NULL);
 }
 
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db0, pci_xeon_x2_bifurc_quirk);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db1, pci_xeon_x2_bifurc_quirk);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db2, pci_xeon_x2_bifurc_quirk);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db3, pci_xeon_x2_bifurc_quirk);
-DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_INTEL, 0x0db6, pci_xeon_x2_bifurc_quirk);
-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);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0db0, pci_xeon_x2_bifurc_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0db1, pci_xeon_x2_bifurc_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0db2, pci_xeon_x2_bifurc_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0db3, pci_xeon_x2_bifurc_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0db6, pci_xeon_x2_bifurc_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0db7, pci_xeon_x2_bifurc_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0db8, pci_xeon_x2_bifurc_quirk);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x0db9, pci_xeon_x2_bifurc_quirk);
 
 /*
  * Fixup to mark boot BIOS video selected by BIOS before it changes
-- 
2.43.0

Re: [PATCH] PCI: Fix Intel Xeon 6 x2 quirk collateral damage on adjacent x4 endpoints

[sashiko-bot]

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

New issues:
- [High] The one-time bus walk completely misses dynamically hotplugged downstream endpoints, breaking the workaround and exposing the system to hardware instability.

Pre-existing issues:
- [High] Endpoint drivers and VFIO guests can trivially overwrite the manual 128B MRRS clamp and Extended Tag restrictions.
--

--- Patch [1]: [PATCH] PCI: Fix Intel Xeon 6 x2 quirk collateral damage on adjacent x4 endpoints ---
commit 437863bd6a6bd870ba3147da1f484eb59fd2282e
Author: Rick Warner <rick@microway.com>

PCI: Fix Intel Xeon 6 x2 quirk collateral damage on adjacent x4 endpoints

This commit refactors the Intel Xeon 6 x2 traffic mitigation quirk to apply
locally to subordinate devices rather than globally on the host bridge. It
removes the host bridge property assignments and instead performs a one-time
bus walk during enumeration to manually clear Extended Tags and clamp MRRS
directly on endpoints.

> diff --git a/arch/x86/pci/fixup.c b/arch/x86/pci/fixup.c
> index b301c6c8df753..c2bdf580692aa 100644
> --- a/arch/x86/pci/fixup.c
> +++ b/arch/x86/pci/fixup.c
> @@ -301,38 +301,43 @@ 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)
> +static int pci_xeon_x2_bifurc_quirk_perdev(struct pci_dev *pdev, void *data)
>  {
> -	int readrq = pcie_get_readrq(pdev);
> -
> -	if (readrq > 128)
> -		pcie_set_readrq(pdev, 128);
> -
> +	/* Only clamp actual endpoints, skip downstream switches/bridges */
> +	if (!pci_is_bridge(pdev)) {
> +		pci_info(pdev, "Disabling Extended Tags and Clamping MRRS to 128B due to upstream Xeon 6 x2 bifurcation\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);

[Severity: High]
This is a pre-existing issue, but can endpoint drivers and VFIO guests
trivially overwrite these manual register changes?

This code clears the MRRS and Extended Tag bits directly via configuration
writes, but it does not set a subsystem constraint such as no_inc_mrrs on the
bridge. During device probe or VFIO guest execution, could a subsequent call
to pcie_set_readrq() unconditionally overwrite this 128B limit and reintroduce
the hardware instability?

> +	}
>  	return 0;
>  }
>  
>  static void pci_xeon_x2_bifurc_quirk(struct pci_dev *pdev)
>  {

[ ... ]

> +	/*
> +	 * Walk down only this specific root port's downstream bus tree
> +	 * and clear extended tags specifically for devices on this branch.
> +	 */
> +	if (pdev->subordinate)
> +		pci_walk_bus(pdev->subordinate, pci_xeon_x2_bifurc_quirk_perdev, NULL);

[Severity: High]
Does this one-time bus walk completely miss dynamically hotplugged downstream
endpoints?

By removing the dynamic enable_device hook, this mitigation relies entirely
on the DECLARE_PCI_FIXUP_FINAL sweep which executes exclusively once at boot
during root port enumeration. If a device is dynamically added later via
hotplug or if an SR-IOV VF is instantiated, the root port is not
re-enumerated and the callback won't be triggered for the new endpoints.
Could this leave hotplugged devices unmitigated, allowing them to negotiate
full MRRS and break the workaround?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729144508.12337-1-rick@microway.com?part=1