mbox series

[SRU,L/M/Unstable,0/9] Enable ASPM for NVMe behind VMD

Message ID 20230906075421.357683-1-kai.heng.feng@canonical.com
Headers show
Series Enable ASPM for NVMe behind VMD | expand

Message

Kai-Heng Feng Sept. 6, 2023, 7:54 a.m. UTC
BugLink: https://bugs.launchpad.net/bugs/2034504

[Impact]
ASPM of NVMe behind VMD is not enabled when BIOS FADT doesn't allow ASPM
to be modified.
That causes higher overall power consumption.

[Fix]
Allow ASPM to be enabled.
While at this, remove old cruft that can interfere upstream VMD driver.

[Test]
Check the value of `/sys/kernel/debug/pmc_core/slp_s0_residency_usec`.
Now system can reach deepest power saving state during sleep.

[Where problems could occur]
Some NVMe may have I/O issue when ASPM is enabled. But for this scenario
the issue will also appear when VMD is disabled.

David E. Box (3):
  PCI: vmd: Use PCI_VDEVICE in device list
  PCI: vmd: Create feature grouping for client products
  PCI: vmd: Add quirk to configure PCIe ASPM and LTR

Kai-Heng Feng (5):
  Revert "UBUNTU: SAUCE: vmd: fixup bridge ASPM by driver name instead"
  Revert "UBUNTU: SAUCE: PCI/ASPM: Enable LTR for endpoints behind VMD"
  Revert "UBUNTU: SAUCE: PCI/ASPM: Enable ASPM for links under VMD
    domain"
  UBUNTU: SAUCE: PCI/ASPM: Allow ASPM override over FADT default
  UBUNTU: SAUCE: PCI: vmd: Mark ASPM override for device behind VMD
    bridge

Michael Bottini (1):
  PCI/ASPM: Add pci_enable_link_state()

 drivers/pci/controller/vmd.c | 100 ++++++++++++++++++++++++++---------
 drivers/pci/pcie/aspm.c      |  54 +++++++++++++++++++
 drivers/pci/quirks.c         |  68 ------------------------
 include/linux/pci.h          |   7 +++
 4 files changed, 135 insertions(+), 94 deletions(-)

Comments

Stefan Bader Sept. 6, 2023, 8:08 a.m. UTC | #1
On 06.09.23 09:54, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/2034504
> 
> [Impact]
> ASPM of NVMe behind VMD is not enabled when BIOS FADT doesn't allow ASPM
> to be modified.
> That causes higher overall power consumption.
> 
> [Fix]
> Allow ASPM to be enabled.
> While at this, remove old cruft that can interfere upstream VMD driver.
> 
> [Test]
> Check the value of `/sys/kernel/debug/pmc_core/slp_s0_residency_usec`.
> Now system can reach deepest power saving state during sleep.
> 
> [Where problems could occur]
> Some NVMe may have I/O issue when ASPM is enabled. But for this scenario
> the issue will also appear when VMD is disabled.
> 
> David E. Box (3):
>    PCI: vmd: Use PCI_VDEVICE in device list
>    PCI: vmd: Create feature grouping for client products
>    PCI: vmd: Add quirk to configure PCIe ASPM and LTR
> 
> Kai-Heng Feng (5):
>    Revert "UBUNTU: SAUCE: vmd: fixup bridge ASPM by driver name instead"
>    Revert "UBUNTU: SAUCE: PCI/ASPM: Enable LTR for endpoints behind VMD"
>    Revert "UBUNTU: SAUCE: PCI/ASPM: Enable ASPM for links under VMD
>      domain"
>    UBUNTU: SAUCE: PCI/ASPM: Allow ASPM override over FADT default
>    UBUNTU: SAUCE: PCI: vmd: Mark ASPM override for device behind VMD
>      bridge
> 
> Michael Bottini (1):
>    PCI/ASPM: Add pci_enable_link_state()
> 
>   drivers/pci/controller/vmd.c | 100 ++++++++++++++++++++++++++---------
>   drivers/pci/pcie/aspm.c      |  54 +++++++++++++++++++
>   drivers/pci/quirks.c         |  68 ------------------------
>   include/linux/pci.h          |   7 +++
>   4 files changed, 135 insertions(+), 94 deletions(-)
> 

What is the outlook/current stage of these 2 new SAUCE patches. Under 
discussion, somewhere in the upstream pipe?

-Stefan
Kai-Heng Feng Sept. 7, 2023, 1:02 a.m. UTC | #2
On Wed, Sep 6, 2023 at 4:08 PM Stefan Bader <stefan.bader@canonical.com> wrote:
>
> On 06.09.23 09:54, Kai-Heng Feng wrote:
> > BugLink: https://bugs.launchpad.net/bugs/2034504
> >
> > [Impact]
> > ASPM of NVMe behind VMD is not enabled when BIOS FADT doesn't allow ASPM
> > to be modified.
> > That causes higher overall power consumption.
> >
> > [Fix]
> > Allow ASPM to be enabled.
> > While at this, remove old cruft that can interfere upstream VMD driver.
> >
> > [Test]
> > Check the value of `/sys/kernel/debug/pmc_core/slp_s0_residency_usec`.
> > Now system can reach deepest power saving state during sleep.
> >
> > [Where problems could occur]
> > Some NVMe may have I/O issue when ASPM is enabled. But for this scenario
> > the issue will also appear when VMD is disabled.
> >
> > David E. Box (3):
> >    PCI: vmd: Use PCI_VDEVICE in device list
> >    PCI: vmd: Create feature grouping for client products
> >    PCI: vmd: Add quirk to configure PCIe ASPM and LTR
> >
> > Kai-Heng Feng (5):
> >    Revert "UBUNTU: SAUCE: vmd: fixup bridge ASPM by driver name instead"
> >    Revert "UBUNTU: SAUCE: PCI/ASPM: Enable LTR for endpoints behind VMD"
> >    Revert "UBUNTU: SAUCE: PCI/ASPM: Enable ASPM for links under VMD
> >      domain"
> >    UBUNTU: SAUCE: PCI/ASPM: Allow ASPM override over FADT default
> >    UBUNTU: SAUCE: PCI: vmd: Mark ASPM override for device behind VMD
> >      bridge
> >
> > Michael Bottini (1):
> >    PCI/ASPM: Add pci_enable_link_state()
> >
> >   drivers/pci/controller/vmd.c | 100 ++++++++++++++++++++++++++---------
> >   drivers/pci/pcie/aspm.c      |  54 +++++++++++++++++++
> >   drivers/pci/quirks.c         |  68 ------------------------
> >   include/linux/pci.h          |   7 +++
> >   4 files changed, 135 insertions(+), 94 deletions(-)
> >
>
> What is the outlook/current stage of these 2 new SAUCE patches. Under
> discussion, somewhere in the upstream pipe?

I am actively working on a series to enable ASPM for both VMD and
hotplugged PCIe devices, so the final form may differ.

Kai-Heng

>
> -Stefan
Stefan Bader Sept. 7, 2023, 7:45 a.m. UTC | #3
On 06.09.23 09:54, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/2034504
> 
> [Impact]
> ASPM of NVMe behind VMD is not enabled when BIOS FADT doesn't allow ASPM
> to be modified.
> That causes higher overall power consumption.
> 
> [Fix]
> Allow ASPM to be enabled.
> While at this, remove old cruft that can interfere upstream VMD driver.
> 
> [Test]
> Check the value of `/sys/kernel/debug/pmc_core/slp_s0_residency_usec`.
> Now system can reach deepest power saving state during sleep.
> 
> [Where problems could occur]
> Some NVMe may have I/O issue when ASPM is enabled. But for this scenario
> the issue will also appear when VMD is disabled.
> 
> David E. Box (3):
>    PCI: vmd: Use PCI_VDEVICE in device list
>    PCI: vmd: Create feature grouping for client products
>    PCI: vmd: Add quirk to configure PCIe ASPM and LTR
> 
> Kai-Heng Feng (5):
>    Revert "UBUNTU: SAUCE: vmd: fixup bridge ASPM by driver name instead"
>    Revert "UBUNTU: SAUCE: PCI/ASPM: Enable LTR for endpoints behind VMD"
>    Revert "UBUNTU: SAUCE: PCI/ASPM: Enable ASPM for links under VMD
>      domain"
>    UBUNTU: SAUCE: PCI/ASPM: Allow ASPM override over FADT default
>    UBUNTU: SAUCE: PCI: vmd: Mark ASPM override for device behind VMD
>      bridge
> 
> Michael Bottini (1):
>    PCI/ASPM: Add pci_enable_link_state()
> 
>   drivers/pci/controller/vmd.c | 100 ++++++++++++++++++++++++++---------
>   drivers/pci/pcie/aspm.c      |  54 +++++++++++++++++++
>   drivers/pci/quirks.c         |  68 ------------------------
>   include/linux/pci.h          |   7 +++
>   4 files changed, 135 insertions(+), 94 deletions(-)
> 
Ok, with the outlook of ultimately replacing the 2 new SAUCE patches 
with the final upstream implementation.

Acked-by: Stefan Bader <stefan.bader@canonical.com>
Andrea Righi Sept. 13, 2023, 6:06 a.m. UTC | #4
On Wed, Sep 06, 2023 at 03:54:12PM +0800, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/2034504
> 
> [Impact]
> ASPM of NVMe behind VMD is not enabled when BIOS FADT doesn't allow ASPM
> to be modified.
> That causes higher overall power consumption.
> 
> [Fix]
> Allow ASPM to be enabled.
> While at this, remove old cruft that can interfere upstream VMD driver.
> 
> [Test]
> Check the value of `/sys/kernel/debug/pmc_core/slp_s0_residency_usec`.
> Now system can reach deepest power saving state during sleep.
> 
> [Where problems could occur]
> Some NVMe may have I/O issue when ASPM is enabled. But for this scenario
> the issue will also appear when VMD is disabled.
> 
> David E. Box (3):
>   PCI: vmd: Use PCI_VDEVICE in device list
>   PCI: vmd: Create feature grouping for client products
>   PCI: vmd: Add quirk to configure PCIe ASPM and LTR
> 
> Kai-Heng Feng (5):
>   Revert "UBUNTU: SAUCE: vmd: fixup bridge ASPM by driver name instead"
>   Revert "UBUNTU: SAUCE: PCI/ASPM: Enable LTR for endpoints behind VMD"
>   Revert "UBUNTU: SAUCE: PCI/ASPM: Enable ASPM for links under VMD
>     domain"
>   UBUNTU: SAUCE: PCI/ASPM: Allow ASPM override over FADT default
>   UBUNTU: SAUCE: PCI: vmd: Mark ASPM override for device behind VMD
>     bridge

Applied to mantic/linux.

Thanks,
-Andrea
Tim Gardner Sept. 13, 2023, 4:48 p.m. UTC | #5
On 9/6/23 1:54 AM, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/2034504
> 
> [Impact]
> ASPM of NVMe behind VMD is not enabled when BIOS FADT doesn't allow ASPM
> to be modified.
> That causes higher overall power consumption.
> 
> [Fix]
> Allow ASPM to be enabled.
> While at this, remove old cruft that can interfere upstream VMD driver.
> 
> [Test]
> Check the value of `/sys/kernel/debug/pmc_core/slp_s0_residency_usec`.
> Now system can reach deepest power saving state during sleep.
> 
> [Where problems could occur]
> Some NVMe may have I/O issue when ASPM is enabled. But for this scenario
> the issue will also appear when VMD is disabled.
> 
> David E. Box (3):
>    PCI: vmd: Use PCI_VDEVICE in device list
>    PCI: vmd: Create feature grouping for client products
>    PCI: vmd: Add quirk to configure PCIe ASPM and LTR
> 
> Kai-Heng Feng (5):
>    Revert "UBUNTU: SAUCE: vmd: fixup bridge ASPM by driver name instead"
>    Revert "UBUNTU: SAUCE: PCI/ASPM: Enable LTR for endpoints behind VMD"
>    Revert "UBUNTU: SAUCE: PCI/ASPM: Enable ASPM for links under VMD
>      domain"
>    UBUNTU: SAUCE: PCI/ASPM: Allow ASPM override over FADT default
>    UBUNTU: SAUCE: PCI: vmd: Mark ASPM override for device behind VMD
>      bridge
> 
> Michael Bottini (1):
>    PCI/ASPM: Add pci_enable_link_state()
> 
>   drivers/pci/controller/vmd.c | 100 ++++++++++++++++++++++++++---------
>   drivers/pci/pcie/aspm.c      |  54 +++++++++++++++++++
>   drivers/pci/quirks.c         |  68 ------------------------
>   include/linux/pci.h          |   7 +++
>   4 files changed, 135 insertions(+), 94 deletions(-)
> 
Acked-by: Tim Gardner <tim.gardner@canonical.com>
Roxana Nicolescu Sept. 20, 2023, 9:10 a.m. UTC | #6
On 06/09/2023 09:54, Kai-Heng Feng wrote:
> BugLink: https://bugs.launchpad.net/bugs/2034504
>
> [Impact]
> ASPM of NVMe behind VMD is not enabled when BIOS FADT doesn't allow ASPM
> to be modified.
> That causes higher overall power consumption.
>
> [Fix]
> Allow ASPM to be enabled.
> While at this, remove old cruft that can interfere upstream VMD driver.
>
> [Test]
> Check the value of `/sys/kernel/debug/pmc_core/slp_s0_residency_usec`.
> Now system can reach deepest power saving state during sleep.
>
> [Where problems could occur]
> Some NVMe may have I/O issue when ASPM is enabled. But for this scenario
> the issue will also appear when VMD is disabled.
>
> David E. Box (3):
>    PCI: vmd: Use PCI_VDEVICE in device list
>    PCI: vmd: Create feature grouping for client products
>    PCI: vmd: Add quirk to configure PCIe ASPM and LTR
>
> Kai-Heng Feng (5):
>    Revert "UBUNTU: SAUCE: vmd: fixup bridge ASPM by driver name instead"
>    Revert "UBUNTU: SAUCE: PCI/ASPM: Enable LTR for endpoints behind VMD"
>    Revert "UBUNTU: SAUCE: PCI/ASPM: Enable ASPM for links under VMD
>      domain"
>    UBUNTU: SAUCE: PCI/ASPM: Allow ASPM override over FADT default
>    UBUNTU: SAUCE: PCI: vmd: Mark ASPM override for device behind VMD
>      bridge
>
> Michael Bottini (1):
>    PCI/ASPM: Add pci_enable_link_state()
>
>   drivers/pci/controller/vmd.c | 100 ++++++++++++++++++++++++++---------
>   drivers/pci/pcie/aspm.c      |  54 +++++++++++++++++++
>   drivers/pci/quirks.c         |  68 ------------------------
>   include/linux/pci.h          |   7 +++
>   4 files changed, 135 insertions(+), 94 deletions(-)
>
Applied to lunar:master-next. Thanks!