linux-kernel - Re: [PATCH v3 1/1] PCI: Add Extended Tag + MRRS quirk for Xeon 6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <6854372adb554_1f9e100aa@dwillia2-xfh.jf.intel.com.notmuch>
Date: Thu, 19 Jun 2025 09:13:30 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, Dan Williams
	<dan.j.williams@...el.com>, Lukas Wunner <lukas@...ner.de>, Bjorn Helgaas
	<bhelgaas@...gle.com>, Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar
	<mingo@...hat.com>, Borislav Petkov <bp@...en8.de>, Dave Hansen
	<dave.hansen@...ux.intel.com>, <x86@...nel.org>, "H. Peter Anvin"
	<hpa@...or.com>, <linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Subject: Re: [PATCH v3 1/1] PCI: Add Extended Tag + MRRS quirk for Xeon 6

Ilpo Järvinen wrote:
> When bifurcated to x2, Xeon 6 Root Port performance is sensitive to the
> configuration of Extended Tags, Max Read Request Size (MRRS), and 10-Bit
> Tag Requester (note: there is currently no 10-Bit Tag support in the
> kernel). While those can be configured to the recommended values by FW,
> kernel may decide to overwrite the initial values.
> 
> Add a quirk that disallows enabling Extended Tags and setting MRRS
> larger than 128B for devices under Xeon 6 Root Ports if the Root Port
> is bifurcated to x2. Use the host bridge's enable_device hook to
> overwrite MRRS if it's set to >128B for the device to be enabled.
> 
> The earlier attempts to implement this quirk polluted PCI core code
> with the checks necessary to support this quirk. Using the
> enable_device hook keeps the quirk well-contained, away from the PCI
> core code.
> 
> Suggested-by: Lukas Wunner <lukas@...ner.de>
> Link: https://cdrdv2.intel.com/v1/dl/getContent/837176
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
> ---
> 
> Ingo gave his Ack on v2 but since the used approach has once again
> changed, I didn't add his Ack.
> 
> Mani did express his concern on using enable_device hook but suggested
> I send the patch anyway.
> 
> We're also looking into using _HPX Type 3 (suggested by Bjorn) eventually
> for this class of problems where FW settings get overwritten by the
> kernel (but it will take time to make it the sanctioned solution). In
> the meantime, this is a real problem for Xeon 6 out there so it warrants
> adding the quirk (which is now pretty well-contained).
> 
> v3:
> - Use enable_device hook to overwrite MRRS to 128B if needed. (Lukas)
> - Typo fix to comment (Ingo)
> 
> v2:
> - Explain in changelog why FW cannot solve this on its own
> - Moved the quirk under arch/x86/pci/
> - Don't NULL check value from pci_find_host_bridge()
> - Added comment above the quirk about the performance degradation
> - Removed all setup chain 128B quirk overrides expect for MRRS write
>   itself (assumes a sane initial value is set by FW)
> 
> 
>  arch/x86/pci/fixup.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)

Thanks for the work on this Ilpo, and thanks for the cleanup suggestions
Lukas. This version looks reasonable to carry the kernel until _HPX can
be relied upon to help the kernel apply quirks like this.

You can add:

Reviewed-by: Dan Williams <dan.j.williams@...el.com>