Re: [RFC PATCH 1/1] PCI: Add Extended Tag + MRRS quirk for Xeon 6

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 1478 bytes --]

On Fri, 7 Mar 2025, Lukas Wunner wrote:

> 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.

I'll do those in v2. Thanks for the comments.

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

We're apparently also not supposed to "finalize a design with this 
information". :-)

-- 
 i.