[RFC PATCH v2 0/4] pcie/aspm: Enable all advertised ASPM states by default

[RFC PATCH v2 0/4] pcie/aspm: Enable all advertised ASPM states by default

[Thomas Falcon]

Hi Bjorn, Rafael, all,

This series follows up on the discussion from August 2025 about ASPM
default behavior and enabling all advertised link power states by
default [1].

Today, ASPM behavior is influenced first by build-time configuration
(CONFIG_PCIEASPM_*), along with firmware-provided defaults. The reliance
on Kconfig for policy selection and the current two-phase configuration
model (initial safe setup followed by a later, more aggressive
driver-triggered configuration) complicate both usage and maintenance.

The direction discussed was to move toward a model where the OS enables
all supported ASPM states by default, while still respecting platform
policy (FADT, _OSC), driver constraints, and user overrides.

This series is an RFC to explore that direction and gather feedback.

Specifically, it:

    Enables all advertised ASPM states by default via the existing
    policy framework, which respects capability masks, blacklist
    restrictions, and user configuration.
    Consolidates ASPM configuration into a single initialization step,
    instead of the current two-phase model (pre-driver “safe” init
    followed by driver-probe adjustments). This simplifies behavior and
    reduces maintenance complexity.
    Adds detailed debug instrumentation to make ASPM configuration
    decisions and transitions more visible, helping to evaluate impact
    and identify mismatches with firmware expectations.
    Limits the default behavior change to newer systems using a DMI BIOS
    year check (>= 2025). This is intended to reduce risk on legacy
    platforms where firmware expectations or device behavior may not
    tolerate more aggressive ASPM enablement.
    Removes CONFIG_PCIEASPM_* policy selections in favor of runtime
    policy selection, simplifying configuration and making behavior
    more consistent.

This RFC is not intended for immediate upstreaming, but to evaluate the
feasibility and impact of this approach.

This does not address synthetic PCIe hierarchies (e.g. VMD), which may
lack valid firmware policy entirely. Those cases likely require
targeted handling and are left for follow-on work so that the core
defaulting model can be evaluated independently.

Feedback is especially welcome on:

    The DMI-based gating approach for limiting rollout risk
    Whether this correctly captures “enable all advertised states”
    The removal of Kconfig-based policy selection
    The shift from a two-phase to single-phase configuration model

[1] https://lore.kernel.org/linux-pci/20250828204345.GA958461@bhelgaas/

v2: -- Removed extra whitespace
    -- Fixed unbalanced brackets
    -- Replaced second dmi_bios_year_check() with
       pcie_aspm_legacy_config_check()
    -- pcie_aspm_legacy_config_check() returns bool instead of int
    -- only log whether aspm is configured at build or boot time once

v1: https://lore.kernel.org/linux-pci/20260429180647.197072-1-thomas.falcon@intel.com/

Thomas Falcon (4):
  pcie/aspm: Add debug logging for aspm policy config
  pcie/aspm: Enable all power-saving states during link state
    initialization
  pcie/aspm: Enable all hardware power-saving states by default
  pcie/aspm: Remove CONFIG_PCIEASPM_* policy definitions

 Documentation/arch/x86/amd-debugging.rst |  5 +-
 arch/mips/configs/bmips_stb_defconfig    |  1 -
 arch/mips/configs/loongson2k_defconfig   |  1 -
 drivers/pci/pci-acpi.c                   |  3 +
 drivers/pci/pcie/Kconfig                 | 33 ---------
 drivers/pci/pcie/aspm.c                  | 86 +++++++++++++++++++-----
 include/linux/pci.h                      |  1 +
 7 files changed, 76 insertions(+), 54 deletions(-)

-- 
2.43.0

[RFC PATCH v2 1/4] pcie/aspm: Add debug logging for aspm policy config

[Thomas Falcon]

Include additional logging for ASPM and Clock PM state changes
keeping track of code flow and enabled power saving states.

Example output after enabling powersupersave policy:

[  130.179249] pcieport 0000:80:1b.0: Updating ASPM state
[  130.179256] igc 0000:81:00.0: ASPM: Disabling ASPM on this device before disabling parent
[  130.179368] pcieport 0000:80:1b.0: ASPM: Disabling ASPM before applying configuration
[  130.179373] pcieport 0000:80:1b.0: ASPM: Configure L1 substates
[  130.179391] pcieport 0000:80:1b.0: ASPM: Configure ASPM state on upstream device
[  130.179395] igc 0000:81:00.0: ASPM: Configure ASPM state on downstream device
[  130.179401] pcieport 0000:80:1b.0: ASPM: enabled states: L1 ASPM-L1.1 PCI-PM-L1.1 PCI-PM-L1.2
[  130.179416] pcieport 0000:00:06.0: Updating ASPM state
[  130.179418] nvme 0000:01:00.0: ASPM: Disabling ASPM on this device before disabling parent
[  130.179422] pcieport 0000:00:06.0: ASPM: Disabling ASPM before applying configuration
[  130.179425] pcieport 0000:00:06.0: ASPM: Configure L1 substates
[  130.179435] pcieport 0000:00:06.0: ASPM: Configure ASPM state on upstream device
[  130.179438] nvme 0000:01:00.0: ASPM: Configure ASPM state on downstream device
[  130.179442] pcieport 0000:00:06.0: ASPM: enabled states: L1 ASPM-L1.1 ASPM-L1.2 PCI-PM-L1.1 PCI-PM-L1.2

Suggested-by: David E. Box <david.e.box@linux.intel.com>
Signed-off-by: Thomas Falcon <thomas.falcon@intel.com>
---
 drivers/pci/pcie/aspm.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 925373b98dff..cd23c1462502 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -372,6 +372,9 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
 	/* Need nothing if the specified equals to current state */
 	if (link->clkpm_enabled == enable)
 		return;
+
+	pci_dbg(link->pdev, "%s Clock PM\n",
+		enable ? "Enabling" : "Disabling");
 	pcie_set_clkpm_nocheck(link, enable);
 }
 
@@ -961,11 +964,14 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 	state &= (link->aspm_capable & ~link->aspm_disable);
 
 	/* Can't enable any substates if L1 is not enabled */
-	if (!(state & PCIE_LINK_STATE_L1))
+	if (!(state & PCIE_LINK_STATE_L1)) {
+		pci_dbg(parent, "ASPM: L1 not enabled, disabling L1 substates\n");
 		state &= ~PCIE_LINK_STATE_L1SS;
+	}
 
 	/* Spec says both ports must be in D0 before enabling PCI PM substates*/
 	if (parent->current_state != PCI_D0 || child->current_state != PCI_D0) {
+		pci_dbg(parent, "ASPM: Both ports are not in D0, disable PCI PM L1 substates unless explicitly enabled\n");
 		state &= ~PCIE_LINK_STATE_L1_SS_PCIPM;
 		state |= (link->aspm_enabled & PCIE_LINK_STATE_L1_SS_PCIPM);
 	}
@@ -973,6 +979,8 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 	/* Nothing to do if the link is already in the requested state */
 	if (link->aspm_enabled == state)
 		return;
+	pci_dbg(parent, "Updating ASPM state\n");
+
 	/* Convert ASPM state to upstream/downstream ASPM register state */
 	if (state & PCIE_LINK_STATE_L0S_UP)
 		dwstream |= PCI_EXP_LNKCTL_ASPM_L0S;
@@ -997,16 +1005,34 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
 	 * Sec 7.5.3.7 also recommends programming the same ASPM Control
 	 * value for all functions of a multi-function device.
 	 */
-	list_for_each_entry(child, &linkbus->devices, bus_list)
+	list_for_each_entry(child, &linkbus->devices, bus_list) {
+		pci_dbg(child, "ASPM: Disabling ASPM on this device before disabling parent\n");
 		pcie_config_aspm_dev(child, 0);
+	}
+	pci_dbg(parent, "ASPM: Disabling ASPM before applying configuration\n");
 	pcie_config_aspm_dev(parent, 0);
 
-	if (link->aspm_capable & PCIE_LINK_STATE_L1SS)
+	if (link->aspm_capable & PCIE_LINK_STATE_L1SS) {
+		pci_dbg(parent, "ASPM: Configure L1 substates\n");
 		pcie_config_aspm_l1ss(link, state);
+	}
 
+	pci_dbg(parent, "ASPM: Configure ASPM state on upstream device\n");
 	pcie_config_aspm_dev(parent, upstream);
-	list_for_each_entry(child, &linkbus->devices, bus_list)
+	list_for_each_entry(child, &linkbus->devices, bus_list) {
+		pci_dbg(child, "ASPM: Configure ASPM state on downstream device\n");
 		pcie_config_aspm_dev(child, dwstream);
+	}
+
+	pci_dbg(parent, "ASPM: enabled states:%s%s%s%s%s%s%s%s\n",
+		FLAG(state, L0S_UP, " L0s-Upstream"),
+		FLAG(state, L0S_DW, " L0s-Downstream"),
+		FLAG(state, L1, " L1"),
+		FLAG(state, L1_1, " ASPM-L1.1"),
+		FLAG(state, L1_2, " ASPM-L1.2"),
+		FLAG(state, L1_1_PCIPM, " PCI-PM-L1.1"),
+		FLAG(state, L1_2_PCIPM, " PCI-PM-L1.2"),
+		FLAG(state, CLKPM, " ClockPM"));
 
 	link->aspm_enabled = state;
 
-- 
2.43.0

[RFC PATCH v2 2/4] pcie/aspm: Enable all power-saving states during link state initialization

[Thomas Falcon]

Setting powersave and powersupersave states at ASPM
link state initialization allows for a simpler and more
maintainable enabling flow that presumes all advertised
power states work. Restrict this behavior to systems
with a BIOS release during or after 2025.

Suggested-by: David E. Box <david.e.box@linux.intel.com>
Signed-off-by: Thomas Falcon <thomas.falcon@intel.com>
---
v2: -- pcie_aspm_legacy_config_check() returns bool instead of int
    -- only log whether aspm is configured at build or boot time once
---
 drivers/pci/pcie/aspm.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index cd23c1462502..e93b72494534 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -24,6 +24,7 @@
 #include <linux/printk.h>
 #include <linux/slab.h>
 #include <linux/time.h>
+#include <linux/dmi.h>
 
 #include "../pci.h"
 
@@ -1057,6 +1058,23 @@ static void free_link_state(struct pcie_link_state *link)
 	kfree(link);
 }
 
+static bool pcie_aspm_legacy_config_check(void)
+{
+	static bool legacy_aspm_config;
+	static bool checked;
+
+	if (checked)
+		return legacy_aspm_config;
+	if (dmi_get_bios_year() < 2025)
+		legacy_aspm_config = true;
+
+	pr_info_once("ASPM configuration is determined at %s time\n",
+		     legacy_aspm_config ? "build" : "boot");
+	checked = true;
+
+	return legacy_aspm_config;
+}
+
 static int pcie_aspm_sanity_check(struct pci_dev *pdev)
 {
 	struct pci_dev *child;
@@ -1196,8 +1214,9 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
 	 * the BIOS's expectation, we'll do so once pci_enable_device() is
 	 * called.
 	 */
-	if (aspm_policy != POLICY_POWERSAVE &&
-	    aspm_policy != POLICY_POWER_SUPERSAVE) {
+	if (!pcie_aspm_legacy_config_check() ||
+	    (aspm_policy != POLICY_POWERSAVE &&
+	     aspm_policy != POLICY_POWER_SUPERSAVE)) {
 		pcie_config_aspm_path(link);
 		pcie_set_clkpm(link, policy_to_clkpm_state(link));
 	}
@@ -1379,8 +1398,9 @@ void pcie_aspm_powersave_config_link(struct pci_dev *pdev)
 	if (aspm_disabled || !link)
 		return;
 
-	if (aspm_policy != POLICY_POWERSAVE &&
-	    aspm_policy != POLICY_POWER_SUPERSAVE)
+	if (!pcie_aspm_legacy_config_check() ||
+	    (aspm_policy != POLICY_POWERSAVE &&
+	     aspm_policy != POLICY_POWER_SUPERSAVE))
 		return;
 
 	down_read(&pci_bus_sem);
-- 
2.43.0

[RFC PATCH v2 3/4] pcie/aspm: Enable all hardware power-saving states by default

[Thomas Falcon]

For systems with a BIOS release date starting in 2025, default
ASPM policy to powersupersave if supported in the ACPI FADT.
Provide a flag, aspm_user_policy, tracking whether a user has
requested a specific power state to give those precedence.
Do not enable all states if user has chosen a specific policy
or disabled ASPM using the pcie_aspm module parameter.

Suggested-by: David E. Box <david.e.box@linux.intel.com>
Signed-off-by: Thomas Falcon <thomas.falcon@intel.com>
---
v2: -- Removed extra whitespace
    -- Fixed unbalanced brackets
    -- Replaced second dmi_bios_year_check() with pcie_aspm_legacy_config_check()
---
 drivers/pci/pci-acpi.c  |  3 +++
 drivers/pci/pcie/aspm.c | 16 ++++++++++++++++
 include/linux/pci.h     |  1 +
 3 files changed, 20 insertions(+)

diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index 4d0f2cb6c695..c73a6b06fc43 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -1523,6 +1523,9 @@ static int __init acpi_pci_init(void)
 	if (acpi_gbl_FADT.boot_flags & ACPI_FADT_NO_ASPM) {
 		pr_info("ACPI FADT declares the system doesn't support PCIe ASPM, so disable it\n");
 		pcie_no_aspm();
+	} else {
+		/* If ASPM is supported, configure the default policy here. */
+		pcie_aspm_policy_config_init();
 	}
 
 	if (acpi_pci_disabled)
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index e93b72494534..063ebe81a9cd 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -267,6 +267,8 @@ static int aspm_policy = POLICY_POWER_SUPERSAVE;
 #else
 static int aspm_policy;
 #endif
+static int aspm_default_policy = POLICY_POWER_SUPERSAVE;
+static bool aspm_user_policy;
 
 static const char *policy_str[] = {
 	[POLICY_DEFAULT] = "default",
@@ -1609,6 +1611,7 @@ static int pcie_aspm_set_policy(const char *val,
 	down_read(&pci_bus_sem);
 	mutex_lock(&aspm_lock);
 	aspm_policy = i;
+	aspm_user_policy = true;
 	list_for_each_entry(link, &link_list, sibling) {
 		pcie_config_aspm_link(link, policy_to_aspm_state(link));
 		pcie_set_clkpm(link, policy_to_clkpm_state(link));
@@ -1810,6 +1813,19 @@ static int __init pcie_aspm_disable(char *str)
 
 __setup("pcie_aspm=", pcie_aspm_disable);
 
+void __init pcie_aspm_policy_config_init(void)
+{
+	/*
+	 * Set ASPM policy here, enabling all power-saving states
+	 * unless ASPM has been disabled or the user has already
+	 * requested a policy or the systems BIOS release date
+	 * is before the year 2025. Otherwise use BIOS defaults.
+	 */
+	if (!aspm_disabled && !aspm_user_policy &&
+	    !pcie_aspm_legacy_config_check())
+		aspm_policy = aspm_default_policy;
+}
+
 void pcie_no_aspm(void)
 {
 	/*
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 2c4454583c11..36fa5579709c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1915,6 +1915,7 @@ int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
 int pci_enable_link_state(struct pci_dev *pdev, int state);
 int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
 void pcie_no_aspm(void);
+void pcie_aspm_policy_config_init(void);
 bool pcie_aspm_support_enabled(void);
 bool pcie_aspm_enabled(struct pci_dev *pdev);
 #else
-- 
2.43.0

[RFC PATCH v2 4/4] pcie/aspm: Remove CONFIG_PCIEASPM_* policy definitions

[Thomas Falcon]

ASPM policy now defaults to POLICY_POWER_SUPERSAVE for new systems
and POLICY_DEFAULT for others while allowing the user to change
policy using the existing pcie_aspm module parameter. Safely remove
CONFIG_PCIEASPM_* policy settings.

Suggested-by: David E. Box <david.e.box@linux.intel.com>
Signed-off-by: Thomas Falcon <thomas.falcon@intel.com>
---
 Documentation/arch/x86/amd-debugging.rst |  5 ++--
 arch/mips/configs/bmips_stb_defconfig    |  1 -
 arch/mips/configs/loongson2k_defconfig   |  1 -
 drivers/pci/pcie/Kconfig                 | 33 ------------------------
 drivers/pci/pcie/aspm.c                  |  8 ------
 5 files changed, 2 insertions(+), 46 deletions(-)

diff --git a/Documentation/arch/x86/amd-debugging.rst b/Documentation/arch/x86/amd-debugging.rst
index d92bf59d62c7..3f346a46357a 100644
--- a/Documentation/arch/x86/amd-debugging.rst
+++ b/Documentation/arch/x86/amd-debugging.rst
@@ -260,9 +260,8 @@ of the devices.
 ASPM
 ----
 For the best runtime power consumption, ASPM should be programmed as intended
-by the BIOS from the hardware vendor.  To accomplish this the Linux kernel
-should be compiled with ``CONFIG_PCIEASPM_DEFAULT`` set to ``y`` and the
-sysfs file ``/sys/module/pcie_aspm/parameters/policy`` should not be modified.
+by the BIOS from the hardware vendor.  To accomplish this the sysfs file
+``/sys/module/pcie_aspm/parameters/policy`` should not be modified.
 
 Most notably, if L1.2 is not configured properly for any devices, the SoC
 will not be able to enter the deepest idle state.
diff --git a/arch/mips/configs/bmips_stb_defconfig b/arch/mips/configs/bmips_stb_defconfig
index ecfa7f777efa..bb19073986a8 100644
--- a/arch/mips/configs/bmips_stb_defconfig
+++ b/arch/mips/configs/bmips_stb_defconfig
@@ -22,7 +22,6 @@ CONFIG_RD_XZ=y
 # CONFIG_RD_LZ4 is not set
 CONFIG_PCI=y
 CONFIG_PCI_MSI=y
-CONFIG_PCIEASPM_POWERSAVE=y
 CONFIG_PCIEPORTBUS=y
 CONFIG_PCIE_BRCMSTB=y
 CONFIG_CPU_FREQ=y
diff --git a/arch/mips/configs/loongson2k_defconfig b/arch/mips/configs/loongson2k_defconfig
index ca534a6b66de..0a2c123eee69 100644
--- a/arch/mips/configs/loongson2k_defconfig
+++ b/arch/mips/configs/loongson2k_defconfig
@@ -88,7 +88,6 @@ CONFIG_RFKILL=m
 CONFIG_RFKILL_INPUT=y
 CONFIG_PCIEPORTBUS=y
 CONFIG_HOTPLUG_PCI_PCIE=y
-CONFIG_PCIEASPM_PERFORMANCE=y
 CONFIG_HOTPLUG_PCI=y
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
index 207c2deae35f..069058870506 100644
--- a/drivers/pci/pcie/Kconfig
+++ b/drivers/pci/pcie/Kconfig
@@ -81,39 +81,6 @@ config PCIEASPM
 
 	  When in doubt, say Y.
 
-choice
-	prompt "Default ASPM policy"
-	default PCIEASPM_DEFAULT
-	depends on PCIEASPM
-
-config PCIEASPM_DEFAULT
-	bool "BIOS default"
-	depends on PCIEASPM
-	help
-	  Use the BIOS defaults for PCI Express ASPM.
-
-config PCIEASPM_POWERSAVE
-	bool "Powersave"
-	depends on PCIEASPM
-	help
-	  Enable PCI Express ASPM L0s and L1 where possible, even if the
-	  BIOS did not.
-
-config PCIEASPM_POWER_SUPERSAVE
-	bool "Power Supersave"
-	depends on PCIEASPM
-	help
-	  Same as PCIEASPM_POWERSAVE, except it also enables L1 substates where
-	  possible. This would result in higher power savings while staying in L1
-	  where the components support it.
-
-config PCIEASPM_PERFORMANCE
-	bool "Performance"
-	depends on PCIEASPM
-	help
-	  Disable PCI Express ASPM L0s and L1, even if the BIOS enabled them.
-endchoice
-
 config PCIE_PME
 	def_bool y
 	depends on PCIEPORTBUS && PM
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 063ebe81a9cd..4502f56f2314 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -258,15 +258,7 @@ static LIST_HEAD(link_list);
 #define POLICY_POWERSAVE 2	/* high power saving */
 #define POLICY_POWER_SUPERSAVE 3 /* possibly even more power saving */
 
-#ifdef CONFIG_PCIEASPM_PERFORMANCE
-static int aspm_policy = POLICY_PERFORMANCE;
-#elif defined CONFIG_PCIEASPM_POWERSAVE
-static int aspm_policy = POLICY_POWERSAVE;
-#elif defined CONFIG_PCIEASPM_POWER_SUPERSAVE
-static int aspm_policy = POLICY_POWER_SUPERSAVE;
-#else
 static int aspm_policy;
-#endif
 static int aspm_default_policy = POLICY_POWER_SUPERSAVE;
 static bool aspm_user_policy;
 
-- 
2.43.0

Re: [RFC PATCH v2 1/4] pcie/aspm: Add debug logging for aspm policy config

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Dead code when attempting to log Clock PM state in ASPM debug output.
- [Low] Unconditional and potentially misleading debug logging for ASPM link configuration.
--

commit 364db9458a74bb3fb257b69effddd2f95f805170
Author: Thomas Falcon <thomas.falcon@intel.com>

pcie/aspm: Add debug logging for aspm policy config

Include additional logging for ASPM and Clock PM state changes
keeping track of code flow and enabled power saving states.

> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 925373b98dff0..cd23c1462502f 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c

[ ... ]

> @@ -961,11 +964,14 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)
>  	state &= (link->aspm_capable & ~link->aspm_disable);
>  
>  	/* Can't enable any substates if L1 is not enabled */
> -	if (!(state & PCIE_LINK_STATE_L1))
> +	if (!(state & PCIE_LINK_STATE_L1)) {
> +		pci_dbg(parent, "ASPM: L1 not enabled, disabling L1 substates\n");
>  		state &= ~PCIE_LINK_STATE_L1SS;
> +	}
>  
>  	/* Spec says both ports must be in D0 before enabling PCI PM substates*/
>  	if (parent->current_state != PCI_D0 || child->current_state != PCI_D0) {
> +		pci_dbg(parent, "ASPM: Both ports are not in D0, disable PCI PM L1 substates unless explicitly enabled\n");
>  		state &= ~PCIE_LINK_STATE_L1_SS_PCIPM;
>  		state |= (link->aspm_enabled & PCIE_LINK_STATE_L1_SS_PCIPM);
>  	}

Are these debug statements being printed more often than intended?

Since these prints happen unconditionally before the early return check:

    if (link->aspm_enabled == state)
        return;

enabling dynamic debugging will output these messages even if the device 
states already match the requested state and no hardware configuration is 
actually applied.

[ ... ]

> @@ -997,16 +1005,34 @@ static void pcie_config_aspm_link(struct pcie_link_state *link, u32 state)

[ ... ]

> +	pci_dbg(parent, "ASPM: enabled states:%s%s%s%s%s%s%s%s\n",
> +		FLAG(state, L0S_UP, " L0s-Upstream"),
> +		FLAG(state, L0S_DW, " L0s-Downstream"),
> +		FLAG(state, L1, " L1"),
> +		FLAG(state, L1_1, " ASPM-L1.1"),
> +		FLAG(state, L1_2, " ASPM-L1.2"),
> +		FLAG(state, L1_1_PCIPM, " PCI-PM-L1.1"),
> +		FLAG(state, L1_2_PCIPM, " PCI-PM-L1.2"),
> +		FLAG(state, CLKPM, " ClockPM"));

Is it possible for the CLKPM flag to ever be true here?

It looks like the PCIE_LINK_STATE_CLKPM bit is masked out earlier in the
function when verifying capabilities:

    state &= (link->aspm_capable & ~link->aspm_disable);

Since the CLKPM bit is outside the 7-bit range of aspm_capable, this bit
is never present in the state mask at this point, so the macro will always
evaluate to an empty string. 

Because the Clock PM state change is already correctly logged in 
pcie_set_clkpm(), does this FLAG check need to be included here?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511210936.562622-1-thomas.falcon@intel.com?part=1

Re: [RFC PATCH v2 2/4] pcie/aspm: Enable all power-saving states during link state initialization

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Data race and missing memory barriers in lazy initialization of `pcie_aspm_legacy_config_check()`
- [Medium] Implicit reliance on `dmi_get_bios_year()` error code evaluating as a chronological year
- [Low] Misleading log message claiming ASPM configuration occurs at 'build' time, and stale code comments
--

commit 5d1b20da84d0615c569d56fd7efaa7c241b6e329
Author: Thomas Falcon <thomas.falcon@intel.com>

pcie/aspm: Enable all power-saving states during link state initialization

This commit enables setting powersave and powersupersave states during
ASPM link state initialization. It restricts this behavior to systems
with a BIOS release during or after 2025 to simplify the enabling flow.

> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index cd23c1462502f..e93b724945341 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c

[ ... ]

> @@ -1057,6 +1058,23 @@ static void free_link_state(struct pcie_link_state *link)
>  	kfree(link);
>  }
>  
> +static bool pcie_aspm_legacy_config_check(void)
> +{
> +	static bool legacy_aspm_config;
> +	static bool checked;
> +
> +	if (checked)
> +		return legacy_aspm_config;
> +	if (dmi_get_bios_year() < 2025)
> +		legacy_aspm_config = true;

When dmi_get_bios_year() returns -ENXIO (-6) on systems without DMI support
(like Device Tree based ARM64 or PowerPC), won't it evaluate as less than
2025?

Does this unintentionally classify all non-DMI platforms as legacy systems
regardless of their actual age?

> +
> +	pr_info_once("ASPM configuration is determined at %s time\n",
> +		     legacy_aspm_config ? "build" : "boot");
> +	checked = true;

Are we missing memory barriers here?

During concurrent PCIe device enumeration, if one thread sets
legacy_aspm_config = true and then checked = true:

Thread A:
    legacy_aspm_config = true;
    checked = true;

Could weak memory ordering cause Thread B to observe checked == true but
read a stale uninitialized legacy_aspm_config value of false, leading to
a regression where unsupported systems bypass the legacy ASPM delay?

Also, this isn't a bug, but does ASPM configuration really occur at "build"
time on legacy systems, or is it deferred until runtime when the device
driver calls pci_enable_device()?

The log message might be misleading for system administrators.

> +
> +	return legacy_aspm_config;
> +}
> +
>  static int pcie_aspm_sanity_check(struct pci_dev *pdev)
>  {
>  	struct pci_dev *child;

[ ... ]

> @@ -1196,8 +1214,9 @@ void pcie_aspm_init_link_state(struct pci_dev *pdev)
>  	 * the BIOS's expectation, we'll do so once pci_enable_device() is
>  	 * called.
>  	 */

This isn't a bug, but should the comment block above be updated to reflect
that POLICY_POWERSAVE configurations are now applied immediately on modern
systems instead of being deferred?

> -	if (aspm_policy != POLICY_POWERSAVE &&
> -	    aspm_policy != POLICY_POWER_SUPERSAVE) {
> +	if (!pcie_aspm_legacy_config_check() ||
> +	    (aspm_policy != POLICY_POWERSAVE &&
> +	     aspm_policy != POLICY_POWER_SUPERSAVE)) {
>  		pcie_config_aspm_path(link);
>  		pcie_set_clkpm(link, policy_to_clkpm_state(link));
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511210936.562622-1-thomas.falcon@intel.com?part=2

Re: [RFC PATCH v2 3/4] pcie/aspm: Enable all hardware power-saving states by default

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Explicit user command-line policy requests via pcie_aspm.policy are incorrectly ignored and overridden if they match the initial compile-time default.
- [High] Data race and missing memory barriers in lockless initialization of pcie_aspm_legacy_config_check().
- [Low] Missing static inline stub for pcie_aspm_policy_config_init() causes build failure when CONFIG_PCIEASPM is disabled.
--

commit 8a98442b198d4058ab31172e6d27a97c7e0f4b0f
Author: Thomas Falcon <thomas.falcon@intel.com>

pcie/aspm: Enable all hardware power-saving states by default

This commit defaults ASPM policy to powersupersave for systems with BIOS release
dates starting in 2025. It provides a flag to track user-requested policies to
give them precedence over the new default.

> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index e93b724945341..063ebe81a9cdf 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
[ ... ]
> @@ -1609,6 +1611,7 @@ static int pcie_aspm_set_policy(const char *val,
>  	down_read(&pci_bus_sem);
>  	mutex_lock(&aspm_lock);
>  	aspm_policy = i;
> +	aspm_user_policy = true;

Are explicit user command-line policy requests via pcie_aspm.policy incorrectly
ignored and overridden if they match the initial compile-time default?

If a user explicitly boots with pcie_aspm.policy=default,
pcie_aspm_set_policy() is invoked to parse the boot parameter before initcalls
run.

If the requested policy matches the initial compiled-in aspm_policy value
(e.g., POLICY_DEFAULT), the early check if (i == aspm_policy) is met, causing
the function to return 0.

Because of this early return, the aspm_user_policy = true flag is never set.

Later during boot, acpi_pci_init() unconditionally calls
pcie_aspm_policy_config_init(). Since aspm_user_policy is false, it incorrectly
assumes the user didn't specify a preference and unilaterally forces the policy
to POLICY_POWER_SUPERSAVE (on BIOS >= 2025 systems).

This logic error completely contradicts the user's explicit command-line
override and forces unsupported power states.

[ ... ]
> @@ -1810,6 +1813,19 @@ static int __init pcie_aspm_disable(char *str)
>  
>  __setup("pcie_aspm=", pcie_aspm_disable);
>  
> +void __init pcie_aspm_policy_config_init(void)
> +{
> +	/*
> +	 * Set ASPM policy here, enabling all power-saving states
> +	 * unless ASPM has been disabled or the user has already
> +	 * requested a policy or the systems BIOS release date
> +	 * is before the year 2025. Otherwise use BIOS defaults.
> +	 */
> +	if (!aspm_disabled && !aspm_user_policy &&
> +	    !pcie_aspm_legacy_config_check())

This problem wasn't introduced by this patch, but exists in
pcie_aspm_legacy_config_check(), introduced earlier in the series.

Does the lockless initialization pattern in pcie_aspm_legacy_config_check()
contain a data race?

The function implements a lockless double-checked initialization pattern using
the static variables checked and legacy_aspm_config without any memory barriers
(e.g., smp_store_release() and smp_load_acquire()).

It is called from pcie_aspm_init_link_state(), which can be executed
concurrently across multiple CPUs during PCI device enumeration (e.g., parallel
hotplug events on different root ports).

Due to compiler or CPU reordering, a CPU can observe checked == true but read
a stale false value for legacy_aspm_config.

This data race causes the function to incorrectly return false on legacy
hardware, causing the modern ASPM link configuration to be misapplied to older
hardware.

> +		aspm_policy = aspm_default_policy;
> +}
> +
>  void pcie_no_aspm(void)
>  {
>  	/*
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2c4454583c115..36fa5579709c5 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1915,6 +1915,7 @@ int pci_disable_link_state_locked(struct pci_dev *pdev, int state);
>  int pci_enable_link_state(struct pci_dev *pdev, int state);
>  int pci_enable_link_state_locked(struct pci_dev *pdev, int state);
>  void pcie_no_aspm(void);
> +void pcie_aspm_policy_config_init(void);

Does this missing static inline stub for pcie_aspm_policy_config_init() cause
a build failure when CONFIG_PCIEASPM is disabled?

The patch adds the function declaration void pcie_aspm_policy_config_init(void);
inside the #ifdef CONFIG_PCIEASPM block in include/linux/pci.h.

However, the patch forgets to provide an empty static inline stub in the
corresponding #else block.

Because drivers/pci/pci-acpi.c calls this function unconditionally and is
compiled whenever CONFIG_ACPI=y is set, building a kernel with CONFIG_ACPI=y
and CONFIG_PCIEASPM=n will result in implicit declaration compiler errors and
undefined reference linker errors.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511210936.562622-1-thomas.falcon@intel.com?part=3

Re: [RFC PATCH v2 4/4] pcie/aspm: Remove CONFIG_PCIEASPM_* policy definitions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The command-line parameter `pcie_aspm.policy=default` is silently ignored on modern systems due to an early return in the parameter parsing logic.
- [High] Removal of compile-time ASPM policy causes non-ACPI and non-DMI platforms to silently default to enabling ASPM, circumventing hardware quirk protections.
--

commit 374fa4f0c33fee54c3758b88f99192ede2552a40
Author: Thomas Falcon <thomas.falcon@intel.com>

pcie/aspm: Remove CONFIG_PCIEASPM_* policy definitions

This commit removes compile-time Kconfig options for PCIe ASPM policy.
It dynamically defaults to POLICY_POWER_SUPERSAVE for new systems and
POLICY_DEFAULT for legacy systems, while keeping the pcie_aspm module
parameter for user overrides.

> diff --git a/arch/mips/configs/loongson2k_defconfig b/arch/mips/configs/loongson2k_defconfig
> index ca534a6b66deb..0a2c123eee69b 100644
> --- a/arch/mips/configs/loongson2k_defconfig
> +++ b/arch/mips/configs/loongson2k_defconfig
> @@ -88,7 +88,6 @@ CONFIG_RFKILL=m
>  CONFIG_RFKILL_INPUT=y
>  CONFIG_PCIEPORTBUS=y
>  CONFIG_HOTPLUG_PCI_PCIE=y
> -CONFIG_PCIEASPM_PERFORMANCE=y

Could this removal introduce a regression on non-ACPI platforms like
loongson2k by circumventing hardware quirk protections?

The new dynamic assignment in pcie_aspm_policy_config_init() is exclusively
called from drivers/pci/pci-acpi.c. This means it is never executed on
non-ACPI platforms.

Even if it were called, dmi_get_bios_year() returns a negative error code
(-ENXIO) on non-DMI platforms, causing the evaluation to incorrectly flag
them as legacy and skip setting aspm_policy to POLICY_POWER_SUPERSAVE.

Consequently, aspm_policy remains permanently set to 0 (POLICY_DEFAULT)
on these platforms. On Device Tree platforms, POLICY_DEFAULT uses
link->aspm_default, where pcie_aspm_override_default_link_state()
explicitly adds PCIE_LINK_STATE_L0S and PCIE_LINK_STATE_L1 to the
default state.

Does this silently flip the hardware link state from safely disabled to
unconditionally enabled for platforms that previously relied on
CONFIG_PCIEASPM_PERFORMANCE=y to explicitly disable ASPM due to hardware bugs?

[ ... ]

> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 063ebe81a9cdf..4502f56f2314c 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -258,15 +258,7 @@ static LIST_HEAD(link_list);
>  #define POLICY_POWERSAVE 2	/* high power saving */
>  #define POLICY_POWER_SUPERSAVE 3 /* possibly even more power saving */
>  
> -#ifdef CONFIG_PCIEASPM_PERFORMANCE
> -static int aspm_policy = POLICY_PERFORMANCE;
> -#elif defined CONFIG_PCIEASPM_POWERSAVE
> -static int aspm_policy = POLICY_POWERSAVE;
> -#elif defined CONFIG_PCIEASPM_POWER_SUPERSAVE
> -static int aspm_policy = POLICY_POWER_SUPERSAVE;
> -#else
>  static int aspm_policy;

Since aspm_policy is now statically initialized to 0 (POLICY_DEFAULT),
is the command-line parameter pcie_aspm.policy=default silently ignored
on modern systems?

When a user passes pcie_aspm.policy=default during early boot,
sysfs_match_string() resolves "default" to 0. The parsing logic exits
early because the requested policy matches the initial value:

drivers/pci/pcie/aspm.c:pcie_aspm_set_policy() {
    ...
    i = sysfs_match_string(policy_str, val);
    if (i < 0)
        return i;
    if (i == aspm_policy)
        return 0;
    ...
    aspm_user_policy = true;
    ...
}

This early return prevents aspm_user_policy from being set to true.

Later in the boot process, pcie_aspm_policy_config_init() runs. Because
aspm_user_policy is still false, on modern systems the kernel unconditionally
overwrites aspm_policy:

drivers/pci/pcie/aspm.c:pcie_aspm_policy_config_init() {
    ...
    if (!aspm_disabled && !aspm_user_policy &&
        !pcie_aspm_legacy_config_check())
        aspm_policy = aspm_default_policy;
}

Does this sequence introduce a regression where the user's explicit request
to force BIOS defaults via the command line is swallowed, potentially causing
hardware hangs on boards with broken ASPM implementations?

> -#endif
>  static int aspm_default_policy = POLICY_POWER_SUPERSAVE;
>  static bool aspm_user_policy;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511210936.562622-1-thomas.falcon@intel.com?part=4