Re: [PATCH] PCI: Disable NoSnoop and Relaxed ordering for Intel wireless BE200

[Bjorn Helgaas <helgaas@kernel.org>]

On Sun, Jun 21, 2026 at 07:30:25AM +0000, Grumbach, Emmanuel wrote:
> On Sun, 2026-06-21 at 07:12 +0000, sashiko-bot@kernel.org wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue(s) to consider:
> > - [High] Missing suspend/resume fixup hook for Intel BE200 PCIe
> > erratum workaround
> > --
> > 
> > commit ebcdab6d68a92d23aa6cf1713e8e72d87f0e6279
> > Author: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> > 
> > PCI: Disable NoSnoop and Relaxed ordering for Intel wireless BE200
> > 
> > This commit disables NoSnoop and Relaxed ordering on the root port
> > for the
> > Intel wireless BE200 device because it fails to copy TLP Attributes
> > from a
> > Request to the Completion. 
> > 
> > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > > --- a/drivers/pci/quirks.c
> > > +++ b/drivers/pci/quirks.c
> > > @@ -4674,6 +4674,13 @@ static void
> > > quirk_chelsio_T5_disable_root_port_attributes(struct pci_dev *pdev)
> > >  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_CHELSIO, PCI_ANY_ID,
> > >  			
> > > quirk_chelsio_T5_disable_root_port_attributes);
> > >  
> > > +/*
> > > + * The Intel wireless BE200 fails to copy TLP Attributes from a
> > > Request to the
> > > + * Completion it generates.
> > > + */
> > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL, 0x272b,
> > > +			 quirk_disable_root_port_attributes);
> > 
> > [Severity: High]
> > Will this workaround survive a system suspend and resume cycle?
> > 
> > When the Intel BE200 device is hotplugged, the upstream Root Port's
> > configuration state is saved at boot time by pcie_portdrv_probe(),
> > which calls pci_save_state(). The workaround modifies the Root Port's
> > current configuration to disable Relaxed Ordering and No Snoop, but it
> > does not update the Root Port's saved state array.
> 
> That's a good point, but I'm not sure we really test well hotplug...

It looks like BE200 is normally an M.2 form factor, and I don't think
M.2 is designed to support hotplug.  I guess it could be done with an
M.2-to-PCIe adapter, but that seems a bit esoteric.

> > Upon system suspend, the PCI core skips saving the Root Port's state
> > because its state_saved flag is already true. During resume,
> > pci_restore_state() restores the Root Port to its original boot-time
> > state, which would re-enable the problematic attributes.
> > 
> > Should there also be a DECLARE_PCI_FIXUP_RESUME_EARLY hook registered
> > here to ensure the workaround is re-applied when the device resumes?

I'm pretty sure the header quirks would be run before portdrv probes, so I
guess this is part of the same theoretical scenario, where portdrv probes
before a BE200 is hot-added.

Can we include any relevant dmesg output, e.g., AER errors or whatever
motivated this quirk in the first place?

c56d4450eb68 ("PCI: Turn off Request Attributes to avoid Chelsio T5
Completion erratum") mentions Malformed TLP or Unexpected Completion
errors, so I suppose this is similar?