[Public] > -----Original Message----- > From: Limonciello, Mario <Mario.Limonciello@xxxxxxx> > Sent: Friday, May 21, 2021 4:34 AM > To: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Keith Busch > <kbusch@xxxxxxxxxx> > Cc: Bjorn Helgaas <helgaas@xxxxxxxxxx>; Liang, Prike > <Prike.Liang@xxxxxxx>; linux-pci@xxxxxxxxxxxxxxx; axboe@xxxxxx; > hch@xxxxxx; sagi@xxxxxxxxxxx; linux-nvme@xxxxxxxxxxxxxxxxxxx; > stable@xxxxxxxxxxxxxxx; S-k, Shyam-sundar <Shyam-sundar.S-k@xxxxxxx>; > Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>; Rafael J. Wysocki > <rjw@xxxxxxxxxxxxx>; linux-pm@xxxxxxxxxxxxxxx > Subject: RE: [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt > > [AMD Public Use] > > > > -----Original Message----- > > > From: Keith Busch <kbusch@xxxxxxxxxx> > > > Sent: Thursday, May 20, 2021 2:04 PM > > > To: Deucher, Alexander <Alexander.Deucher@xxxxxxx> > > > Cc: Bjorn Helgaas <helgaas@xxxxxxxxxx>; Liang, Prike > > > <Prike.Liang@xxxxxxx>; linux-pci@xxxxxxxxxxxxxxx; axboe@xxxxxx; > > > hch@xxxxxx; sagi@xxxxxxxxxxx; linux-nvme@xxxxxxxxxxxxxxxxxxx; > > > stable@xxxxxxxxxxxxxxx; S-k, Shyam-sundar <Shyam-sundar.S- > > k@xxxxxxx>; > > > Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>; Rafael J. Wysocki > > > <rjw@xxxxxxxxxxxxx>; linux-pm@xxxxxxxxxxxxxxx > > > 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.
