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

["Liang, Prike" <Prike.Liang@amd.com>]

[Public]

> -----Original Message-----
> From: Limonciello, Mario <Mario.Limonciello@amd.com>
> Sent: Friday, May 21, 2021 4:34 AM
> To: Deucher, Alexander <Alexander.Deucher@amd.com>; Keith Busch
> <kbusch@kernel.org>
> Cc: Bjorn Helgaas <helgaas@kernel.org>; Liang, Prike
> <Prike.Liang@amd.com>; linux-pci@vger.kernel.org; axboe@fb.com;
> hch@lst.de; sagi@grimberg.me; linux-nvme@lists.infradead.org;
> stable@vger.kernel.org; S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>;
> Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>; Rafael J. Wysocki
> <rjw@rjwysocki.net>; linux-pm@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@kernel.org>
> > > Sent: Thursday, May 20, 2021 2:04 PM
> > > To: Deucher, Alexander <Alexander.Deucher@amd.com>
> > > Cc: Bjorn Helgaas <helgaas@kernel.org>; Liang, Prike
> > > <Prike.Liang@amd.com>; linux-pci@vger.kernel.org; axboe@fb.com;
> > > hch@lst.de; sagi@grimberg.me; linux-nvme@lists.infradead.org;
> > > stable@vger.kernel.org; S-k, Shyam-sundar <Shyam-sundar.S-
> > k@amd.com>;
> > > Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>; Rafael J. Wysocki
> > > <rjw@rjwysocki.net>; linux-pm@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.



_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme