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

[Liang, Prike]

[Public]

> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello at amd.com>
> Sent: Friday, May 21, 2021 4:34 AM
> To: Deucher, Alexander <Alexander.Deucher at amd.com>; Keith Busch
> <kbusch at kernel.org>
> 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
>
> [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.

Thanks for all proposal and will draft a new patch for this.