This is a note to let you know that I've just added the patch titled
Revert "PCI/ASPM: Disable only ASPM_STATE_L1 when driver, disables L1"
to the 6.5-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
revert-pci-aspm-disable-only-aspm_state_l1-when-driver-disables-l1.patch
and it can be found in the queue-6.5 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From 3cb4f534bac010258b2688395c2f13459a932be9 Mon Sep 17 00:00:00 2001
From: Heiner Kallweit <hkallweit1@xxxxxxxxx>
Date: Wed, 11 Oct 2023 09:36:40 +0200
Subject: Revert "PCI/ASPM: Disable only ASPM_STATE_L1 when driver, disables L1"
From: Heiner Kallweit <hkallweit1@xxxxxxxxx>
commit 3cb4f534bac010258b2688395c2f13459a932be9 upstream.
This reverts commit fb097dcd5a28c0a2325632405c76a66777a6bed9.
After fb097dcd5a28 ("PCI/ASPM: Disable only ASPM_STATE_L1 when driver
disables L1"), disabling L1 via pci_disable_link_state(PCIE_LINK_STATE_L1),
then enabling one substate, e.g., L1.1, via sysfs actually enables *all*
the substates.
For example, r8169 disables L1 because of hardware issues on a number of
systems, which implicitly disables the L1.1 and L1.2 substates.
On some systems, L1 and L1.1 work fine, but L1.2 causes missed rx packets.
Enabling L1.1 via the sysfs "aspm_l1_1" attribute unexpectedly enables L1.2
as well as L1.1.
After fb097dcd5a28, pci_disable_link_state(PCIE_LINK_STATE_L1) adds only
ASPM_L1 (but not any of the L1.x substates) to the "aspm_disable" mask:
--- Before fb097dcd5a28
+++ After fb097dcd5a28
# r8169 disables L1:
pci_disable_link_state(PCIE_LINK_STATE_L1)
- disable |= ASPM_L1 | ASPM_L1_1 | ASPM_L1_2 | ... # disable L1, L1.x
+ disable |= ASPM_L1 # disable L1 only
# write "1" to sysfs "aspm_l1_1" attribute:
l1_1_aspm
aspm_attr_store_common(state = ASPM_L1_1)
disable &= ~ASPM_L1_1 # enable L1.1
if (state & (ASPM_L1_1 | ...)) # if enabling any substate
disable &= ~ASPM_L1 # enable L1
# final state:
- disable = ASPM_L1_2 | ... # L1, L1.1 enabled; L1.2 disabled
+ disable = 0 # L1, L1.1, L1.2 all enabled
Enabling an L1.x substate removes the substate and L1 from the
"aspm_disable" mask. After fb097dcd5a28, the substates were not added to
the mask when disabling L1, so enabling one substate implicitly enables all
of them.
Revert fb097dcd5a28 so enabling one substate doesn't enable the others.
Link: https://lore.kernel.org/r/c75931ac-7208-4200-9ca1-821629cf5e28@xxxxxxxxx
Signed-off-by: Heiner Kallweit <hkallweit1@xxxxxxxxx>
[bhelgaas: work through example in commit log]
Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
drivers/pci/pcie/aspm.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -1059,7 +1059,8 @@ static int __pci_disable_link_state(stru
if (state & PCIE_LINK_STATE_L0S)
link->aspm_disable |= ASPM_STATE_L0S;
if (state & PCIE_LINK_STATE_L1)
- link->aspm_disable |= ASPM_STATE_L1;
+ /* L1 PM substates require L1 */
+ link->aspm_disable |= ASPM_STATE_L1 | ASPM_STATE_L1SS;
if (state & PCIE_LINK_STATE_L1_1)
link->aspm_disable |= ASPM_STATE_L1_1;
if (state & PCIE_LINK_STATE_L1_2)
Patches currently in stable-queue which might be from hkallweit1@xxxxxxxxx are
queue-6.5/r8169-respect-userspace-disabling-iff_multicast.patch
queue-6.5/revert-pci-aspm-disable-only-aspm_state_l1-when-driver-disables-l1.patch
queue-6.5/r8169-fix-rare-issue-with-broken-rx-after-link-down-.patch
queue-6.5/net-r8169-disable-multicast-filter-for-rtl8168h-and-.patch
