Re: [PATCH 2/2] PCI/ASPM: Clear ASPM Control on links without common ASPM support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci/aspm: cached saved state not updated after clearing ASPMC
--

commit d667f086616b3d0999815c8db89e04e4d40aea30
Author: Stefan Roese <stefan.roese@mailbox.org>

PCI/ASPM: Clear ASPM Control on links without common ASPM support

This commit ensures ASPM Control is cleared for all functions on a link that
lacks common ASPM support, processing downstream components first. This
prevents devices that enable ASPM by default from keeping it enabled when
connected to a port that does not support it.

> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index f8e64971c6c3d..13d7a6d3f6ad3 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c

[ ... ]

> @@ -947,11 +959,17 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
>  
>  	/*
>  	 * If ASPM not supported, don't mess with the clocks and link,
> -	 * bail out now.
> +	 * bail out now. A device may still come out of reset with ASPM
> +	 * Control set, which the rest of this code never touches for
> +	 * such a link. Clear it, downstream component first.
>  	 */
>  	if (!(parent->aspm_l0s_support && child->aspm_l0s_support) &&
> -	    !(parent->aspm_l1_support && child->aspm_l1_support))
> +	    !(parent->aspm_l1_support && child->aspm_l1_support)) {
> +		list_for_each_entry(fn, &linkbus->devices, bus_list)
> +			pcie_aspm_clear_aspmc(fn);
> +		pcie_aspm_clear_aspmc(parent);

[Severity: High]
Does this bypass updating the parent's cached saved state?

By clearing the ASPM Control in hardware but returning immediately without
updating the cache (e.g., via pci_update_aspm_saved_state()), it appears the
parent bridge's saved_config_space will retain the stale ASPMC enabled value.

If a non-ASPM-capable device is hot-added, and the system undergoes an AER or
DPC reset before any power management event forces a state save, could
pci_restore_state() use this stale cache to erroneously re-enable ASPM on the
parent port?

This might lead to completion timeouts or link hangs.

>  		return;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930144650.3701516-1-stefan.roese@mailbox.org?part=2