[PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt

[Limonciello, Mario]

[AMD Public Use]

> > -----Original Message-----
> > From: Keith Busch <kbusch at kernel.org>
> > Sent: Thursday, May 20, 2021 2:04 PM
> > To: Deucher, Alexander <Alexander.Deucher at amd.com>
> > Cc: Bjorn Helgaas <helgaas at kernel.org>; Liang, Prike
> > <Prike.Liang at amd.com>; linux-pci at vger.kernel.org; axboe at fb.com;
> > hch at lst.de; sagi at grimberg.me; linux-nvme at lists.infradead.org;
> > stable at vger.kernel.org; S-k, Shyam-sundar <Shyam-sundar.S-
> k at amd.com>;
> > Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>; Rafael J. Wysocki
> > <rjw at rjwysocki.net>; linux-pm at vger.kernel.org
> > Subject: Re: [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown
> opt
> >
> > On Thu, May 20, 2021 at 05:40:54PM +0000, Deucher, Alexander wrote:
> > > It doesn't really have anything to do with PCI.  The PCI link is just
> > > a proxy for specific AMD platforms.  It's platform firmware behavior
> > > we are catering to.  This was originally posted as an nvme quirk, but
> > > during the review it was recommended to move the quirk into PCI
> > > because the quirk is not specific a particular NVMe device, but rather
> > > a set of AMD platforms.  Lots of other platforms seems to do similar
> > > things in the nvme driver based on ACPI or DMI flags, etc.  On our
> > > hardware this nvme flag is required for all cezanne and renoir platforms.
> >
> > The quirk was initially presented as specific to the pci root. Does it make
> > more sense for nvme to recognize the limitation from querying a different
> > platform component instead of the pci bus?
> 
> Maybe.  I'm not sure what the best way to tie this to a specific platform is.
> @Limonciello, Mario?
> 

I'll just remind folks that Prike mentioned this is a problem specific to the
Renoir and Cezanne ASICs.  These were the first ones that S2idle was used.
"Future" designs the problems that cause the need for this change should be fixed.

With that in mind, I can see the argument from Bjorn to not over-engineer it and
claim a PCI quirk that applies to all the downstream PCIe devices when this is just
needed for NVME devices.  The PCI device id selected (0x1630) is the root complex associated
specifically to these ASICs.  

Since these are mobile platforms that don't contain any way to connect other external
PCIe devices I think another way to safely do it could be an if #ifdef CONFIG_X86 and then
check if set for doing s2i and if so do a x86_cpu_match() to the model and families
matching the CPUs.

To me this seems like a fine compromise given there is a precedent for dmi_match on
OEM platforms and enumerating "all" of the OEM platforms that contain CZN/RN and enable
S2I may be an exercise in futility.