* RE: [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt [not found] ` <1619054346-4566-2-git-send-email-Prike.Liang@amd.com> @ 2021-05-06 3:08 ` Liang, Prike 2021-05-10 1:18 ` Liang, Prike 0 siblings, 1 reply; 4+ messages in thread From: Liang, Prike @ 2021-05-06 3:08 UTC (permalink / raw) To: linux-nvme, kbusch, hch, Chaitanya.Kulkarni, gregkh, linux-pci, Bjorn Helgaas Cc: stable, Deucher, Alexander, S-k, Shyam-sundar, Chaitanya Kulkarni, rjw [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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt 2021-05-06 3:08 ` [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt Liang, Prike @ 2021-05-10 1:18 ` Liang, Prike 0 siblings, 0 replies; 4+ messages in thread From: Liang, Prike @ 2021-05-10 1:18 UTC (permalink / raw) To: linux-nvme, kbusch, hch, Chaitanya.Kulkarni, gregkh, linux-pci, Bjorn Helgaas Cc: stable, Deucher, Alexander, S-k, Shyam-sundar, Chaitanya Kulkarni, rjw [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 <helgaas@kernel.org> > Cc: stable@vger.kernel.org; Deucher, Alexander > <Alexander.Deucher@amd.com>; S-k, Shyam-sundar <Shyam-sundar.S- > k@amd.com>; Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>; > rjw@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@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 ^ permalink raw reply [flat|nested] 4+ messages in thread
[parent not found: <1619054346-4566-3-git-send-email-Prike.Liang@amd.com>]
* RE: [PATCH v5 2/2] nvme-pci: add AMD PCIe quirk for simple suspend/resume [not found] ` <1619054346-4566-3-git-send-email-Prike.Liang@amd.com> @ 2021-05-06 3:12 ` Liang, Prike 0 siblings, 0 replies; 4+ messages in thread From: Liang, Prike @ 2021-05-06 3:12 UTC (permalink / raw) To: linux-nvme, kbusch, hch, Chaitanya.Kulkarni, gregkh, linux-pci, Bjorn Helgaas, rjw Cc: stable, Deucher, Alexander, S-k, Shyam-sundar, Chaitanya Kulkarni [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 2/2] nvme-pci: add AMD PCIe quirk for simple > suspend/resume > > 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. > > This patch is updating the nvme_acpi_storage_d3() with previously added > 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/nvme/host/pci.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index > 6bad4d4..617256e 100644 > --- a/drivers/nvme/host/pci.c > +++ b/drivers/nvme/host/pci.c > @@ -2836,6 +2836,8 @@ static bool nvme_acpi_storage_d3(struct pci_dev > *dev) > acpi_status status; > u8 val; > > +if (dev->bus->bus_flags & PCI_BUS_FLAGS_DISABLE_ON_S2I) > +return true; > /* > * Look for _DSD property specifying that the storage device on the > port > * must use D3 to support deep platform power savings during > -- > 2.7.4 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v5 0/2] nvme-pci: add AMD PCIe quirk for NVMe simple suspend/resume @ 2021-05-18 2:24 Prike Liang 2021-05-18 2:24 ` [PATCH v5 2/2] nvme-pci: add AMD PCIe quirk for " Prike Liang 0 siblings, 1 reply; 4+ messages in thread From: Prike Liang @ 2021-05-18 2:24 UTC (permalink / raw) To: helgaas, linux-pci, kbusch, axboe, hch, sagi, linux-nvme Cc: Alexander.Deucher, stable, Shyam-sundar.S-k, Prike Liang Those patch series can handle NVMe can't suspend to D3 during s2idle entry on some AMD platform. In this case, can be settld by assigning and passing a PCIe bus flag to the armed device which need NVMe shutdown opt in s2idle suspend and then use PCIe power setting to put the NVMe device to D3. Prike Liang (2): PCI: add AMD PCIe quirk for nvme shutdown opt nvme-pci: add AMD PCIe quirk for simple suspend/resume drivers/nvme/host/pci.c | 2 ++ drivers/pci/probe.c | 5 ++++- drivers/pci/quirks.c | 7 +++++++ include/linux/pci.h | 2 ++ 4 files changed, 15 insertions(+), 1 deletion(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v5 2/2] nvme-pci: add AMD PCIe quirk for simple suspend/resume 2021-05-18 2:24 [PATCH v5 0/2] nvme-pci: add AMD PCIe quirk for NVMe " Prike Liang @ 2021-05-18 2:24 ` Prike Liang 0 siblings, 0 replies; 4+ messages in thread From: Prike Liang @ 2021-05-18 2:24 UTC (permalink / raw) To: helgaas, linux-pci, kbusch, axboe, hch, sagi, linux-nvme Cc: Alexander.Deucher, stable, Shyam-sundar.S-k, Prike Liang, Chaitanya Kulkarni 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. This patch is updating the nvme_acpi_storage_d3() with previously added 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/nvme/host/pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 6bad4d4..617256e 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -2836,6 +2836,8 @@ static bool nvme_acpi_storage_d3(struct pci_dev *dev) acpi_status status; u8 val; + if (dev->bus->bus_flags & PCI_BUS_FLAGS_DISABLE_ON_S2I) + return true; /* * Look for _DSD property specifying that the storage device on the port * must use D3 to support deep platform power savings during -- 2.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-05-18 2:25 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1619054346-4566-1-git-send-email-Prike.Liang@amd.com> [not found] ` <1619054346-4566-2-git-send-email-Prike.Liang@amd.com> 2021-05-06 3:08 ` [PATCH v5 1/2] PCI: add AMD PCIe quirk for nvme shutdown opt Liang, Prike 2021-05-10 1:18 ` Liang, Prike [not found] ` <1619054346-4566-3-git-send-email-Prike.Liang@amd.com> 2021-05-06 3:12 ` [PATCH v5 2/2] nvme-pci: add AMD PCIe quirk for simple suspend/resume Liang, Prike 2021-05-18 2:24 [PATCH v5 0/2] nvme-pci: add AMD PCIe quirk for NVMe " Prike Liang 2021-05-18 2:24 ` [PATCH v5 2/2] nvme-pci: add AMD PCIe quirk for " Prike Liang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).