On Fri, 2021-07-02 at 11:24 -0500, Bjorn Helgaas wrote:
> On Fri, Jul 02, 2021 at 05:39:43PM +0200, Ben Hutchings wrote:
> > On Thu, 2021-07-01 at 10:25 -0500, Bjorn Helgaas wrote:
> > [...]
> > > After 27d868b5e6cfa, pci_configure_device() did actually call
> > > pcie_set_mps(), which updates the Device Control register (possibly
> > > restricted by dev->pcie_mpss, which is set by this quirk).
> > > 
> > > The fixup_mpss_256() quirk was added in 2011 by a94d072b2023 ("PCI:
> > > Add quirk for known incorrect MPSS").  Interesting that 27d868b5e6cfa
> > > was merged in 2015 but apparently nobody noticed until now.  I guess
> > > those Solarflare devices aren't widely used?
> > [...]
> > 
> > The key thing is that this quirk was working around an issue with
> > legacy interrupts, while the sfc and sfc-falcon drivers have always
> > preferred to use MSIs if available.  (But I also don't think many
> > SFC4000-based NICs were sold, and they were EOL'd about 10 years ago.)

Also, most of the read-only PCIe config registers on the SFC4000 are
initialised from flash, and the commit message implies MPSS was changed
on later boards.

> Just out of curiosity, do you happen to remember the legacy interrupt
> connection?  MPS has to do with the maximum TLP size, and it's not
> obvious to me why using INTx vs MSI would matter there.
[...]

No I don't.  I had completely forgotten about this, so I'm just
combining my commit message for the quirk with my general knowledge of
that chip.

(The bug I actually remember involving legacy interrupts, affecting
both SFC4000 and SFC9020, required a horrible workaround in the
driver.)

Ben.

-- 
Ben Hutchings
[W]e found...that it wasn't as easy to get programs right as we had
thought. I realized that a large part of my life from then on was going
to be spent in finding mistakes in my own programs.
                                                 - Maurice Wilkes, 1949