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

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

[AMD Public Use]

+ linux-pci for further review.

Thanks,
> -----Original Message-----
> From: Liang, Prike <Prike.Liang@amd.com>
> 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
> <Alexander.Deucher@amd.com>; S-k, Shyam-sundar <Shyam-sundar.S-
> k@amd.com>; Liang, Prike <Prike.Liang@amd.com>; Chaitanya Kulkarni
> <chaitanya.kulkarni@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@vger.kernel.org> # 5.10+
> Signed-off-by: Prike Liang <Prike.Liang@amd.com>
> 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 <chaitanya.kulkarni@wdc.com>
> 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


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