Re: [PATCH 1/2] PCI: Call MPS fixup quirks early

From: Ben Hutchings <ben@decadent.org.uk>
To: Bjorn Helgaas <helgaas@kernel.org>
Cc: "Marek Behún" <kabel@kernel.org>,
	linux-pci@vger.kernel.org, "Pali Rohár" <pali@kernel.org>,
	Rötti <espressobinboardarmbiantempmailaddress@posteo.de>,
	"Zachary Zhang" <zhangzg@marvell.com>,
	"Edward Cree" <ecree.xilinx@gmail.com>,
	"Martin Habets" <habetsm.xilinx@gmail.com>,
	"Keith Busch" <kbusch@kernel.org>
Subject: Re: [PATCH 1/2] PCI: Call MPS fixup quirks early
Date: Fri, 02 Jul 2021 23:53:48 +0200	[thread overview]
Message-ID: <4fa5e3e5e998aff53f30380087564a837420f4d6.camel@decadent.org.uk> (raw)
In-Reply-To: <20210702162448.GA192062@bjorn-Precision-5520>

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

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]