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

[Liang, Prike]

[AMD Public Use]

Ping... Could I get some comments or RB on this latest series.


Thanks,
Prike
> -----Original Message-----
> From: Liang, Prike
> Sent: Thursday, May 6, 2021 11:08 AM
> To: linux-nvme at lists.infradead.org; kbusch at kernel.org; hch at infradead.org;
> Chaitanya.Kulkarni at wdc.com; gregkh at linuxfoundation.org; linux-
> pci at vger.kernel.org; Bjorn Helgaas <helgaas at kernel.org>
> Cc: stable at vger.kernel.org; Deucher, Alexander
> <Alexander.Deucher at amd.com>; S-k, Shyam-sundar <Shyam-sundar.S-
> k at amd.com>; Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>;
> rjw at rjwysocki.net
> Subject: RE: [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt
>
> + linux-pci for further review.
>
> Thanks,
> > -----Original Message-----
> > From: Liang, Prike <Prike.Liang at amd.com>
> > Sent: Thursday, April 22, 2021 9:19 AM
> > To: linux-nvme at lists.infradead.org; kbusch at kernel.org;
> > hch at infradead.org; Chaitanya.Kulkarni at wdc.com;
> > gregkh at linuxfoundation.org
> > Cc: stable at vger.kernel.org; Deucher, Alexander
> > <Alexander.Deucher at amd.com>; S-k, Shyam-sundar <Shyam-sundar.S-
> > k at amd.com>; Liang, Prike <Prike.Liang at amd.com>; Chaitanya Kulkarni
> > <chaitanya.kulkarni at wdc.com>
> > Subject: [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt
> >
> > In the NVMe controller default suspend-resume seems only save/restore
> > the NVMe link state by APST opt and the NVMe remains in D0 during this
> time.
> > Then the NVMe device will be shutdown by SMU firmware in the s2idle
> > entry and then will lost the NVMe power context during s2idle
> > resume.Finally, the NVMe command queue request will be processed
> > abnormally and result in access timeout.This issue can be settled by
> > using PCIe power set with simple suspend-resume process path instead of
> APST get/set opt.
> >
> > In this patch prepare a PCIe RC bus flag to identify the platform
> > whether need the quirk.
> >
> > Cc: <stable at vger.kernel.org> # 5.10+
> > Signed-off-by: Prike Liang <Prike.Liang at amd.com>
> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k at amd.com>
> > [ck: split patches for nvme and pcie]
> > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
> > Suggested-by: Keith Busch <kbusch at kernel.org>
> > Acked-by: Keith Busch <kbusch at kernel.org>
> > ---
> > Changes in v2:
> > Fix the patch format and check chip root complex DID instead of PCIe
> > RP to avoid the storage device plugged in internal PCIe RP by USB adaptor.
> >
> > Changes in v3:
> > According to Christoph Hellwig do NVME PCIe related identify opt
> > better in PCIe quirk driver rather than in NVME module.
> >
> > Changes in v4:
> > Split the fix to PCIe and NVMe part and then call the pci_dev_put()
> > put the device reference count and finally refine the commit info.
> >
> > Changes in v5:
> > According to Christoph Hellwig and Keith Busch better use a
> > passthrough
> > device(bus) gloable flag to identify the NVMe shutdown opt rather than
> > look up the device BDF.
> > ---
> >  drivers/pci/probe.c  | 5 ++++-
> >  drivers/pci/quirks.c | 7 +++++++
> >  include/linux/pci.h  | 2 ++
> >  3 files changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index
> > 953f15a..34ba691e 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -558,10 +558,13 @@ static struct pci_bus *pci_alloc_bus(struct
> > pci_bus
> > *parent)
> >     INIT_LIST_HEAD(&b->resources);
> >     b->max_bus_speed = PCI_SPEED_UNKNOWN;
> >     b->cur_bus_speed = PCI_SPEED_UNKNOWN;
> > +   if (parent) {
> >  #ifdef CONFIG_PCI_DOMAINS_GENERIC
> > -   if (parent)
> >             b->domain_nr = parent->domain_nr;
> >  #endif
> > +           if (parent->bus_flags & PCI_BUS_FLAGS_DISABLE_ON_S2I)
> > +                   b->bus_flags |= PCI_BUS_FLAGS_DISABLE_ON_S2I;
> > +   }
> >     return b;
> >  }
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index
> > 653660e3..7c4bb8e 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -312,6 +312,13 @@ static void quirk_nopciamd(struct pci_dev *dev)
> > }  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_AMD,
> >     PCI_DEVICE_ID_AMD_8151_0,       quirk_nopciamd);
> >
> > +static void quirk_amd_s2i_fixup(struct pci_dev *dev) {
> > +   dev->bus->bus_flags |= PCI_BUS_FLAGS_DISABLE_ON_S2I;
> > +   pci_info(dev, "AMD simple suspend opt enabled\n"); }
> > +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_AMD, 0x1630,
> > +quirk_amd_s2i_fixup);
> > +
> >  /* Triton requires workarounds to be used by the drivers */  static
> > void quirk_triton(struct pci_dev *dev)  { diff --git
> > a/include/linux/pci.h b/include/linux/pci.h index 53f4904..dc65219
> > 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -240,6 +240,8 @@ enum pci_bus_flags {
> >     PCI_BUS_FLAGS_NO_MMRBC  = (__force pci_bus_flags_t) 2,
> >     PCI_BUS_FLAGS_NO_AERSID = (__force pci_bus_flags_t) 4,
> >     PCI_BUS_FLAGS_NO_EXTCFG = (__force pci_bus_flags_t) 8,
> > +   /* Driver must pci_disable_device() for suspend-to-idle */
> > +   PCI_BUS_FLAGS_DISABLE_ON_S2I    = (__force pci_bus_flags_t) 16,
> >  };
> >
> >  /* Values from Link Status register, PCIe r3.1, sec 7.8.8 */
> > --
> > 2.7.4