[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@lists.infradead.org; kbusch@kernel.org; hch@infradead.org;
Chaitanya.Kulkarni@wdc.com; gregkh@linuxfoundation.org; linux-
pci@vger.kernel.org; Bjorn Helgaas [off-list ref]
Cc: stable@vger.kernel.org; Deucher, Alexander
[off-list ref]; S-k, Shyam-sundar <Shyam-sundar.S-
k@amd.com>; Chaitanya Kulkarni [off-list ref];
rjw@rjwysocki.net
Subject: RE: [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt

+ linux-pci for further review.

Thanks,

quoted

-----Original Message-----
From: Liang, Prike <redacted>
Sent: Thursday, April 22, 2021 9:19 AM
To: linux-nvme@lists.infradead.org; kbusch@kernel.org;
hch@infradead.org; Chaitanya.Kulkarni@wdc.com;
gregkh@linuxfoundation.org
Cc: stable@vger.kernel.org; Deucher, Alexander
[off-list ref]; S-k, Shyam-sundar <Shyam-sundar.S-
k@amd.com>; Liang, Prike [off-list ref]; Chaitanya Kulkarni
[off-list ref]
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.

quoted

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.

quoted

In this patch prepare a PCIe RC bus flag to identify the platform
whether need the quirk.

Cc: <redacted> # 5.10+
Signed-off-by: Prike Liang <redacted>
Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
[ck: split patches for nvme and pcie]
Signed-off-by: Chaitanya Kulkarni <redacted>
Suggested-by: Keith Busch <kbusch@kernel.org>
Acked-by: Keith Busch <kbusch@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