[PATCH] PCI/ASPM: Fix L1 sub-state handling in aspm_attr_store_common

[PATCH] PCI/ASPM: Fix L1 sub-state handling in aspm_attr_store_common

[Heiner Kallweit]

So far we may see an unexpected behavior regard L1 sub-states.
Following scenario:

Write 0 to "l1_aspm" to disable L1
Write 1 to "l1_1_aspm" to enable L1.1

Intention of step 1 is to implicitly disable also L1.1 and L1.2.
However after step 2 L1.2 is unexpectedly enabled.

Fix this by explicitly disabling L1 sub-states when disabling L1.

Fixes: 72ea91afbfb0 ("PCI/ASPM: Add sysfs attributes for controlling ASPM link states")
Link: https://lore.kernel.org/linux-pci/20231002151452.GA560499@bhelgaas/
Cc: stable@vger.kernel.org
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
Splitted-out part of original patch according to linked discussion.
---
 drivers/pci/pcie/aspm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 530c3bb5708c..fc18e42f0a6e 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1248,6 +1248,8 @@ static ssize_t aspm_attr_store_common(struct device *dev,
 			link->aspm_disable &= ~ASPM_STATE_L1;
 	} else {
 		link->aspm_disable |= state;
+		if (state & ASPM_STATE_L1)
+			link->aspm_disable |= ASPM_STATE_L1SS;
 	}
 
 	pcie_config_aspm_link(link, policy_to_aspm_state(link));
-- 
2.42.0

Re: [PATCH] PCI/ASPM: Fix L1 sub-state handling in aspm_attr_store_common

[Bjorn Helgaas]

On Wed, Oct 11, 2023 at 09:46:45AM +0200, Heiner Kallweit wrote:
> So far we may see an unexpected behavior regard L1 sub-states.
> Following scenario:
> 
> Write 0 to "l1_aspm" to disable L1
> Write 1 to "l1_1_aspm" to enable L1.1
> 
> Intention of step 1 is to implicitly disable also L1.1 and L1.2.
> However after step 2 L1.2 is unexpectedly enabled.
> 
> Fix this by explicitly disabling L1 sub-states when disabling L1.
> 
> Fixes: 72ea91afbfb0 ("PCI/ASPM: Add sysfs attributes for controlling ASPM link states")
> Link: https://lore.kernel.org/linux-pci/20231002151452.GA560499@bhelgaas/
> Cc: stable@vger.kernel.org
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>

Applied to pci/aspm for v6.7, thanks again.

> ---
> Splitted-out part of original patch according to linked discussion.
> ---
>  drivers/pci/pcie/aspm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 530c3bb5708c..fc18e42f0a6e 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -1248,6 +1248,8 @@ static ssize_t aspm_attr_store_common(struct device *dev,
>  			link->aspm_disable &= ~ASPM_STATE_L1;
>  	} else {
>  		link->aspm_disable |= state;
> +		if (state & ASPM_STATE_L1)
> +			link->aspm_disable |= ASPM_STATE_L1SS;
>  	}
>  
>  	pcie_config_aspm_link(link, policy_to_aspm_state(link));
> -- 
> 2.42.0
>