| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAKd_mfSgzb5Zwb-_hjXMuiFfOw=ZoZn0GrC37=s8DraQTXVRhw@mail.gmail.com>
Date: Fri, 10 Feb 2023 21:39:56 +0800
From: Mark Enriquez <enriquezmark36@...il.com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Vidya Sagar <vidyas@...dia.com>, bhelgaas@...gle.com,
sathyanarayanan.kuppuswamy@...ux.intel.com,
rafael.j.wysocki@...el.com, kai.heng.feng@...onical.com,
tasev.stefanoska@...net.be, linux-pci@...r.kernel.org,
linux-kernel@...r.kernel.org, treding@...dia.com,
jonathanh@...dia.com, kthota@...dia.com, mmaddireddy@...dia.com,
sagar.tv@...il.com, Thomas Witt <kernel@...t.link>
Subject: Re: [PATCH V2] PCI/ASPM: Skip L1SS save/restore if not already enabled
Hi,
Resending this in plaintext mode.
I apologize for the duplicate mail.
Sorry,
Mark Francis
----------------------
Hello,
I tried the test patch with the "ASPM: can't restore L1SS while L1
enabled" message on the v6.1 tag.
I tried setting the ASPM policy to default rather than powersupersave.
Tested twice.
The result is I get to see the messages in the kernel log. The system
resumed successfully in all tests.
[ 330.438136] ACPI: PM: Waking up from system sleep state S3
[ 330.445959] ACPI: EC: interrupt unblocked
[ 330.446174] pcieport 0000:00:1c.0: ASPM: can't restore L1SS while
L1 enabled (0x0042)
[ 330.446177] pcieport 0000:00:1c.6: ASPM: can't restore L1SS while
L1 enabled (0x0002)
[ 330.448354] r8169 0000:03:00.0: ASPM: can't restore L1SS while L1
enabled (0x0142)
[ 330.448368] sdhci-pci 0000:04:00.0: ASPM: can't restore L1SS while
L1 enabled (0x0102)
[ 330.448672] pcieport 0000:00:06.0: ASPM: can't restore L1SS while
L1 enabled (0x0042)
[ 330.448965] nvme 0000:02:00.0: ASPM: can't restore L1SS while L1
enabled (0x0042)
[ 330.449814] pcieport 0000:00:01.0: ASPM: can't restore L1SS while
L1 enabled (0x0042)
[ 330.577111] pci 0000:01:00.0: ASPM: can't restore L1SS while L1
enabled (0x0142)
[ 330.580820] ACPI: EC: event unblocked
[ 330.581066] sd 0:0:0:0: [sda] Starting disk
I also noticed that these messages also pop out when activating a
userspace powersave tool (i.e., tlp).
(I was restoring my machine after the test, that is, re-enabling
services like tlp.
Then, I accidentally knocked off the wall plug with my foot causing
tlp to activate its battery profile)
[ 4065.786154] pcieport 0000:00:1c.0: ASPM: can't restore L1SS while
L1 enabled (0x0042)
[ 4065.799553] r8169 0000:03:00.0: ASPM: can't restore L1SS while L1
enabled (0x0142)
[ 4065.969703] r8169 0000:03:00.0 enp3s0: Link is Down
I really wish I could also try and speculate other solutions but I am
ignorant with respect to the PCIe specifications.
Nevertheless, Hope this helps.
Let me know if I also need to test the case where the commits are reverted.
Thanks,
On Fri, Feb 10, 2023 at 9:35 PM Mark Enriquez <enriquezmark36@...il.com> wrote:
>
> Hello,
>
> I tried the test patch with the "ASPM: can't restore L1SS while L1 enabled" message on the v6.1 tag.
>
> I tried setting the ASPM policy to default rather than powersupersave. Tested twice.
> The result is I get to see the messages in the kernel log. The system resumed successfully in all tests.
> [ 330.438136] ACPI: PM: Waking up from system sleep state S3
> [ 330.445959] ACPI: EC: interrupt unblocked
> [ 330.446174] pcieport 0000:00:1c.0: ASPM: can't restore L1SS while L1 enabled (0x0042)
> [ 330.446177] pcieport 0000:00:1c.6: ASPM: can't restore L1SS while L1 enabled (0x0002)
> [ 330.448354] r8169 0000:03:00.0: ASPM: can't restore L1SS while L1 enabled (0x0142)
> [ 330.448368] sdhci-pci 0000:04:00.0: ASPM: can't restore L1SS while L1 enabled (0x0102)
> [ 330.448672] pcieport 0000:00:06.0: ASPM: can't restore L1SS while L1 enabled (0x0042)
> [ 330.448965] nvme 0000:02:00.0: ASPM: can't restore L1SS while L1 enabled (0x0042)
> [ 330.449814] pcieport 0000:00:01.0: ASPM: can't restore L1SS while L1 enabled (0x0042)
> [ 330.577111] pci 0000:01:00.0: ASPM: can't restore L1SS while L1 enabled (0x0142)
> [ 330.580820] ACPI: EC: event unblocked
> [ 330.581066] sd 0:0:0:0: [sda] Starting disk
>
> I also noticed that these messages also pop out when activating a userspace powersave tool (i.e., tlp).
> (I was restoring my machine after the test, that is, re-enabling services like tlp.
> Then, I accidentally knocked off the wall plug with my foot causing tlp to activate its battery profile)
> [ 4065.786154] pcieport 0000:00:1c.0: ASPM: can't restore L1SS while L1 enabled (0x0042)
> [ 4065.799553] r8169 0000:03:00.0: ASPM: can't restore L1SS while L1 enabled (0x0142)
> [ 4065.969703] r8169 0000:03:00.0 enp3s0: Link is Down
>
> I really wish I could also try and speculate other solutions but I am ignorant with respect to the PCIe specifications.
>
> Nevertheless, Hope this helps.
> Let me know if I also need to test the case where the commits are reverted.
>
> Thanks,
>
> On Fri, Feb 10, 2023 at 8:18 AM Bjorn Helgaas <helgaas@...nel.org> wrote:
>>
>> [+cc Thomas]
>>
>> On Wed, Feb 08, 2023 at 05:42:29PM -0600, Bjorn Helgaas wrote:
>> > On Fri, Jan 20, 2023 at 02:45:40PM +0530, Vidya Sagar wrote:
>> > > Skip save and restore of ASPM L1 Sub-States specific registers if they
>> > > are not already enabled in the system. This is to avoid issues observed
>> > > on certain platforms during restoration process, particularly when
>> > > restoring the L1SS registers contents.
>> > >
>> > > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=216782
>> > > Signed-off-by: Vidya Sagar <vidyas@...dia.com>
>> > > ---
>> > > v2:
>> > > * Address review comments from Kai-Heng Feng and Rafael
>> > >
>> > > drivers/pci/pcie/aspm.c | 17 ++++++++++++++++-
>> > > include/linux/pci.h | 1 +
>> > > 2 files changed, 17 insertions(+), 1 deletion(-)
>> > >
>> > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>> > > index 53a1fa306e1e..bd2a922081bd 100644
>> > > --- a/drivers/pci/pcie/aspm.c
>> > > +++ b/drivers/pci/pcie/aspm.c
>> > > @@ -761,11 +761,23 @@ void pci_save_aspm_l1ss_state(struct pci_dev *dev)
>> > > {
>> > > struct pci_cap_saved_state *save_state;
>> > > u16 l1ss = dev->l1ss;
>> > > - u32 *cap;
>> > > + u32 *cap, val;
>> > >
>> > > if (!l1ss)
>> > > return;
>> > >
>> > > + /*
>> > > + * Skip save and restore of L1 Sub-States registers if they are not
>> > > + * already enabled in the system
>> > > + */
>> > > + pci_read_config_dword(dev, l1ss + PCI_L1SS_CTL1, &val);
>> > > + if (!(val & PCI_L1SS_CTL1_L1SS_MASK)) {
>> > > + dev->skip_l1ss_restore = true;
>> > > + return;
>> > > + }
>> >
>> > I think this fix is still problematic. PCIe r6.0, sec 5.5.4, requires
>> > that
>> >
>> > If setting either or both of the enable bits for ASPM L1 PM
>> > Substates, both ports must be configured as described in this
>> > section while ASPM L1 is disabled.
>> >
>> > The current Linux code does not observe this because ASPM L1 is
>> > enabled by PCI_EXP_LNKCTL (in the PCIe Capability Link Control
>> > register), while ASPM L1 PM Substate configuration is in PCI_L1SS_CTL1
>> > (in the L1 PM Substates Capability), and these two things are not
>> > integrated:
>> >
>> > pci_restore_state
>> > pci_restore_aspm_l1ss_state
>> > aspm_program_l1ss
>> > pci_write_config_dword(PCI_L1SS_CTL1, ctl1) # L1SS restore
>> > pci_restore_pcie_state
>> > pcie_capability_write_word(PCI_EXP_LNKCTL, cap[i++]) # L1 restore
>> >
>> > So I suspect the problem is that we're writing PCI_L1SS_CTL1 while
>> > ASPM L1 is enabled, and the device gets confused somehow.
>> >
>> > I think it would be better change this restore flow to follow that
>> > spec requirement instead of skipping the save/restore like this.
>>
>> A revert of 4ff116d0d5fd ("PCI/ASPM: Save L1 PM Substates Capability
>> for suspend/resume") has been in linux-next starting with Feb 6.
>>
>> I originally reverted 5e85eba6f50d ("PCI/ASPM: Refactor L1 PM
>> Substates Control Register programming") because it broke
>> suspend/resume differently [1].
>>
>> I had to revert 4ff116d0d5fd at the same time because 5e85eba6f50d
>> added aspm_program_l1ss(), which was used by 4ff116d0d5fd.
>>
>> I don't think Tasev or Mark have directly tested reverting
>> 4ff116d0d5fd to see if it resolves the problem *they* are seeing. But
>> that would be good to know so I can update the commit logs.
>>
>> Bjorn
>>
>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=216877
Powered by blists - more mailing lists