linux-kernel - Re: [RFC PATCH 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z8qvo0_tuSbwwyIY@wunner.de>
Date: Fri, 7 Mar 2025 09:34:43 +0100
From: Lukas Wunner <lukas@...ner.de>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Dan Williams <dan.j.williams@...el.com>
Subject: Re: [RFC PATCH 1/1] PCI: Add Extended Tag + MRRS quirk for Xeon 6

On Tue, Mar 04, 2025 at 03:51:08PM +0200, Ilpo Järvinen wrote:
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -5564,6 +5564,33 @@ DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0144, quirk_no_ext_tags);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0420, quirk_no_ext_tags);
>  DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_SERVERWORKS, 0x0422, quirk_no_ext_tags);
>  
> +static void quirk_pcie2x_no_tags_no_mrrs(struct pci_dev *pdev)
> +{
> +	struct pci_host_bridge *bridge = pci_find_host_bridge(pdev->bus);
> +	u32 linkcap;
> +
> +	if (!bridge)
> +		return;

I note that in a lot of places where pci_find_host_bridge() is called,
no NULL pointer check is performed.  So omitting it would appear
to be safe.

The quirk is x86-specific, so compiling it into the kernel on other
arches creates unnecessary bloat.  Avoid by moving to arch/x86/pci/fixup.c.

There should definitely be a multi-line code comment above the function
explaining what defect this works around (slower performance apparently),
and also link to the PDF document.

BTW the PDF document says "Intel Confidential", I'm wondering why this
has been made public without stripping the confidentiality marker...

Thanks,

Lukas