[PATCH v2] PCI/ASPM: Mask ASPM states based on Devicetree properties

[PATCH v2] PCI/ASPM: Mask ASPM states based on Devicetree properties

[Krishna Chaitanya Chundru]

Some platforms require selectively disabling specific ASPM states on a
given PCIe link to avoid link instability or functional failures caused
by board-level connectivity constraints such as PCB routing, connectors,
slots, or external cabling.

Devicetree initially supported disabling ASPM L0s via the 'aspm-no-l0s'
property, and has since been extended to also allow disabling ASPM L1 and
L1 PM Substates using the 'aspm-no-l1' [1] and 'aspm-no-l1ss' [2]
properties. However, the ASPM driver does not currently account for these
properties when determining the default ASPM link state.

Update ASPM link initialization to check for these Devicetree properties
on either end of the link and mask the corresponding ASPM states from
link->aspm_support before applying the default ASPM policy.

Link [1]: https://github.com/devicetree-org/dt-schema/pull/188
Link [2]: https://github.com/devicetree-org/dt-schema/pull/190

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v2:
- Disable L1ss when L1 is disabled as pointed by sashiko.
- Disable L1ss if bootloader enables them but we are disabling via
  devicetree pointed by sashiko.
- Link to v1: https://patch.msgid.link/20260511-aspm-v1-1-b4a9fe955cf9@oss.qualcomm.com
---
 drivers/pci/pcie/aspm.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 925373b98dff..a79426b5bff0 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -27,6 +27,8 @@
 
 #include "../pci.h"
 
+static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state);
+
 void pci_save_ltr_state(struct pci_dev *dev)
 {
 	int ltr;
@@ -799,6 +801,13 @@ static void aspm_l1ss_init(struct pcie_link_state *link)
 
 #define FLAG(x, y, d)	(((x) & (PCIE_LINK_STATE_##y)) ? d : "")
 
+static bool pcie_link_has_aspm_override(const struct pcie_link_state *link,
+					const char *aspm)
+{
+	return (device_property_present(&link->pdev->dev, aspm) ||
+		device_property_present(&link->downstream->dev, aspm));
+}
+
 static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
 {
 	struct pci_dev *pdev = link->downstream;
@@ -806,6 +815,24 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
 
 	/* For devicetree platforms, enable L0s and L1 by default */
 	if (of_have_populated_dt()) {
+		if (pcie_link_has_aspm_override(link, "aspm-no-l0s"))
+			link->aspm_support &= ~PCIE_LINK_STATE_L0S;
+
+		if (pcie_link_has_aspm_override(link, "aspm-no-l1"))
+			link->aspm_support &= ~(PCIE_LINK_STATE_L1 | PCIE_LINK_STATE_L1SS);
+
+		if (pcie_link_has_aspm_override(link, "aspm-no-l1ss")) {
+			/*
+			 * Clear L1SS in hardware before updating aspm_support. Once
+			 * aspm_capable is derived from aspm_support, pcie_config_aspm_link()
+			 * skips pcie_config_aspm_l1ss() entirely via the aspm_capable guard,
+			 * leaving firmware-enabled L1SS substates active in hardware.
+			 */
+			if (link->aspm_enabled & PCIE_LINK_STATE_L1SS)
+				pcie_config_aspm_l1ss(link, 0);
+			link->aspm_support &= ~PCIE_LINK_STATE_L1SS;
+		}
+
 		if (link->aspm_support & PCIE_LINK_STATE_L0S)
 			link->aspm_default |= PCIE_LINK_STATE_L0S;
 		if (link->aspm_support & PCIE_LINK_STATE_L1)

---
base-commit: 840ef6c78e6a2f694b578ecb9063241c992aaa9e
change-id: 20260429-aspm-da55a54279e4

Best regards,
--  
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

Re: [PATCH v2] PCI/ASPM: Mask ASPM states based on Devicetree properties

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Modifying ASPM L1 PM Substates without first disabling ASPM L1 violates PCIe specifications.
- [Medium] The 'aspm-no-l1' override removes L1SS software support but fails to clear firmware-enabled L1SS hardware state.
--

commit 0343a07bd629cd03e625683fa27781cd434b46f3
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

PCI/ASPM: Mask ASPM states based on Devicetree properties

This commit updates ASPM link initialization to check for Devicetree
properties like 'aspm-no-l0s', 'aspm-no-l1', and 'aspm-no-l1ss'. It masks
the corresponding ASPM states from the link support mask before applying
the default ASPM policy.

Link [1]: https://github.com/devicetree-org/dt-schema/pull/188
Link [2]: https://github.com/devicetree-org/dt-schema/pull/190

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

[ ... ]

> @@ -806,6 +815,24 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
>  
>  	/* For devicetree platforms, enable L0s and L1 by default */
>  	if (of_have_populated_dt()) {
> +		if (pcie_link_has_aspm_override(link, "aspm-no-l0s"))
> +			link->aspm_support &= ~PCIE_LINK_STATE_L0S;
> +
> +		if (pcie_link_has_aspm_override(link, "aspm-no-l1"))
> +			link->aspm_support &= ~(PCIE_LINK_STATE_L1 | PCIE_LINK_STATE_L1SS);

[Severity: Medium]
If the devicetree has 'aspm-no-l1' but not 'aspm-no-l1ss', this clears
PCIE_LINK_STATE_L1SS from the link's aspm_support. Will this cause
pcie_config_aspm_link() to skip updating the L1SS registers because
aspm_capable lacks L1SS?

Should this branch also explicitly clear the hardware state via
pcie_config_aspm_l1ss(link, 0) in the same way the 'aspm-no-l1ss' branch
does, to prevent leaving a firmware-enabled L1SS configuration active in
hardware?

> +
> +		if (pcie_link_has_aspm_override(link, "aspm-no-l1ss")) {
> +			/*
> +			 * Clear L1SS in hardware before updating aspm_support. Once
> +			 * aspm_capable is derived from aspm_support, pcie_config_aspm_link()
> +			 * skips pcie_config_aspm_l1ss() entirely via the aspm_capable guard,
> +			 * leaving firmware-enabled L1SS substates active in hardware.
> +			 */
> +			if (link->aspm_enabled & PCIE_LINK_STATE_L1SS)
> +				pcie_config_aspm_l1ss(link, 0);

[Severity: High]
Does modifying the L1SS registers here violate PCIe specifications by
changing L1 PM Substates while ASPM L1 is still enabled?

Looking at the caller pcie_aspm_cap_init(), the original PCI_EXP_LNKCTL
values are restored right before calling pcie_aspm_override_default_link_state(),
which can re-enable ASPM L1 if the firmware had it enabled:

pcie_aspm_cap_init() {
    ...
    if (FIELD_GET(PCI_EXP_LNKCTL_ASPMC, child_lnkctl) ||
        FIELD_GET(PCI_EXP_LNKCTL_ASPMC, parent_lnkctl)) {
        pcie_capability_write_word(parent, PCI_EXP_LNKCTL, parent_lnkctl);
        pcie_capability_write_word(child, PCI_EXP_LNKCTL, child_lnkctl);
    }
    ...
    pcie_aspm_override_default_link_state(link);
}

Since PCIe r6.2 sec 5.5.4 requires disabling ASPM L1 first, is there
a risk of undefined hardware behavior or link drops here?

> +			link->aspm_support &= ~PCIE_LINK_STATE_L1SS;
> +		}
> +
>  		if (link->aspm_support & PCIE_LINK_STATE_L0S)
>  			link->aspm_default |= PCIE_LINK_STATE_L0S;
>  		if (link->aspm_support & PCIE_LINK_STATE_L1)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-aspm-v2-1-800a4151ba3a@oss.qualcomm.com?part=1

Re: [PATCH v2] PCI/ASPM: Mask ASPM states based on Devicetree properties

[kernel test robot]

Hi Krishna,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 840ef6c78e6a2f694b578ecb9063241c992aaa9e]

url:    https://github.com/intel-lab-lkp/linux/commits/Krishna-Chaitanya-Chundru/PCI-ASPM-Mask-ASPM-states-based-on-Devicetree-properties/20260624-181628
base:   840ef6c78e6a2f694b578ecb9063241c992aaa9e
patch link:    https://lore.kernel.org/r/20260624-aspm-v2-1-800a4151ba3a%40oss.qualcomm.com
patch subject: [PATCH v2] PCI/ASPM: Mask ASPM states based on Devicetree properties
config: x86_64-buildonly-randconfig-006-20260624 (https://download.01.org/0day-ci/archive/20260624/202606242330.Ab9bKFmk-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260624/202606242330.Ab9bKFmk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202606242330.Ab9bKFmk-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/pci/pcie/aspm.c:30:13: warning: 'pcie_config_aspm_l1ss' declared 'static' but never defined [-Wunused-function]
      30 | static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state);
         |             ^~~~~~~~~~~~~~~~~~~~~


vim +30 drivers/pci/pcie/aspm.c

    29	
  > 30	static void pcie_config_aspm_l1ss(struct pcie_link_state *link, u32 state);
    31	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki