linux-kernel - Re: [Regression in 6.14-rc1] System suspend/resume broken by PCI commit 1db806ec06b7c

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0i=yBSFW82E=s=mx7ztVzmnoUwbxkDRnYyDztAWK9VcsQ@mail.gmail.com>
Date: Mon, 3 Feb 2025 21:31:23 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Bjorn Helgaas <bhelgaas@...gle.com>, Jian-Hong Pan <jhp@...lessos.org>
Cc: Linux PCI <linux-pci@...r.kernel.org>, Linux PM <linux-pm@...r.kernel.org>, 
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>, 
	Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, 
	"Thorsten Leemhuis (regressions address)" <regressions@...mhuis.info>, 
	Linux regressions mailing list <regressions@...ts.linux.dev>
Subject: Re: [Regression in 6.14-rc1] System suspend/resume broken by PCI
 commit 1db806ec06b7c

On Mon, Feb 3, 2025 at 9:12 PM Rafael J. Wysocki <rafael@...nel.org> wrote:
>
> Hi,
>
> The following commit:
>
> commit 1db806ec06b7c6e08e8af57088da067963ddf117
> Author: Jian-Hong Pan <jhp@...lessos.org>
> Date:   Fri Nov 15 15:22:02 2024 +0800
>
>    PCI/ASPM: Save parent L1SS config in pci_save_aspm_l1ss_state()
>
>    After 17423360a27a ("PCI/ASPM: Save L1 PM Substates Capability for
>    suspend/resume"), pci_save_aspm_l1ss_state(dev) saves the L1SS state for
>    "dev", and pci_restore_aspm_l1ss_state(dev) restores the state for both
>    "dev" and its parent.
>
>    The problem is that unless pci_save_state() has been used in some other
>    path and has already saved the parent L1SS state, we will restore junk to
>    the parent, which means the L1 Substates likely won't work correctly.
>
>    Save the L1SS config for both the device and its parent in
>    pci_save_aspm_l1ss_state().  When restoring, we need both because L1SS must
>    be enabled at the parent (the Downstream Port) before being enabled at the
>    child (the Upstream Port).
>
>    Link: https://lore.kernel.org/r/20241115072200.37509-3-jhp@endlessos.org
>    Fixes: 17423360a27a ("PCI/ASPM: Save L1 PM Substates Capability for
> suspend/resume")
>    Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218394
>    Suggested-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
>    Signed-off-by: Jian-Hong Pan <jhp@...lessos.org>
>    [bhelgaas: parallel save/restore structure, simplify commit log, patch at
>    https://lore.kernel.org/r/20241212230340.GA3267194@bhelgaas]
>    Signed-off-by: Bjorn Helgaas <bhelgaas@...gle.com>
>    Tested-by: Jian-Hong Pan <jhp@...lessos.org> # Asus B1400CEAE
>
> broke system suspend/resume on my Dell XPS13 9360.  It doesn't even
> pass suspend/resume testing after "echo devices > /sys/power/pm_test".
>
> It looks like PCIe links are all down during resume after the above
> commit, but it is rather hard to collect any data in that state.
>
> Reverting the above commit on top of 6.14-rc1 makes things work again,
> no problem.
>
> I'm unsure what exactly the problem is ATM, but I'm going to check a
> couple of theories.

The attached change makes it work again, FWIW, but moving the
parent->l1ss check alone below the pdev l1ss saving doesn't help.

So it is either the parent check against NULL or the
pcie_downstream_port(pdev) one that breaks it.  I guess the former,
but I'll test it tomorrow.

View attachment "pci-l1ss-save.patch" of type "text/x-patch" (794 bytes)