Received-SPF: pass (google.com: domain of linux-kernel+bounces-17446-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240101181348.GA1684058@bhelgaas> <d7e7b133-d373-e850-1f5f-deee8aa86958@linux.intel.com>
In-Reply-To: <d7e7b133-d373-e850-1f5f-deee8aa86958@linux.intel.com>
From: Kai-Heng Feng <kai.heng.feng@canonical.com>
Date: Fri, 5 Jan 2024 11:14:53 +0800
Message-ID: <CAAd53p7yASMMcRTUR6ybJzj8KUP_+7h4nvE7wQ7cm0eQR6nEBQ@mail.gmail.com>
Subject: Re: [Regression] [PCI/ASPM] [ASUS PN51] Reboot on resume attempt
 (bisect done; commit found)
To: =?UTF-8?Q?Ilpo_J=C3=A4rvinen?= <ilpo.jarvinen@linux.intel.com>
Cc: Bjorn Helgaas <helgaas@kernel.org>, Michael Schaller <michael@5challer.de>, bhelgaas@google.com, 
	linux-pci@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>, 
	regressions@lists.linux.dev, macro@orcam.me.uk, ajayagarwal@google.com, 
	sathyanarayanan.kuppuswamy@linux.intel.com, 
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>, hkallweit1@gmail.com, 
	michael.a.bottini@linux.intel.com, johan+linaro@kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 3, 2024 at 11:41=E2=80=AFPM Ilpo J=C3=A4rvinen
<ilpo.jarvinen@linux.intel.com> wrote:
>
> On Mon, 1 Jan 2024, Bjorn Helgaas wrote:
>
> > On Mon, Dec 25, 2023 at 07:29:02PM +0100, Michael Schaller wrote:
> > > Issue:
> > > On resume from suspend to RAM there is no output for about 12 seconds=
, then
> > > shortly a blinking cursor is visible in the upper left corner on an
> > > otherwise black screen which is followed by a reboot.
> > >
> > > Setup:
> > > * Machine: ASUS mini PC PN51-BB757MDE1 (DMI model: MINIPC PN51-E1)
> > > * Firmware: 0508 (latest; also tested previous 0505)
> > > * OS: Ubuntu 23.10 (except kernel)
> > > * Kernel: 6.6.8 (also tested 6.7-rc7; config attached)
> > >
> > > Debugging summary:
> > > * Kernel 5.10.205 isn=E2=80=99t affected.
> > > * Bisect identified commit 08d0cc5f34265d1a1e3031f319f594bd1970976c a=
s
> > > cause.
> > > * PCI device 0000:03:00.0 (Intel 8265 Wifi) causes resume issues as l=
ong as
> > > ASPM is enabled (default).
> > > * The commit message indicates that a quirk could be written to mitig=
ate the
> > > issue but I don=E2=80=99t know how to write such a quirk.
> > >
> > > Confirmed workarounds:
> > > * Connect a USB flash drive (no clue why; maybe this causes a delay t=
hat
> > > lets the resume succeed)
> > > * Revert commit 08d0cc5f34265d1a1e3031f319f594bd1970976c (commit seem=
ed
> > > intentional; a quirk seems to be the preferred solution)
> > > * pcie_aspm=3Doff
> > > * pcie_aspm.policy=3Dperformance
> > > * echo 0 | sudo tee /sys/bus/pci/devices/0000:03:00.0/link/l1_aspm
> > >
> > > Debugging details:
> > > * The resume trigger (power button, keyboard, mouse) doesn=E2=80=99t =
seem to make
> > > any difference.
> > > * Double checked that the kernel is configured to *not* reboot on pan=
ic.
> > > * Double checked that there still isn't any kernel output without qui=
et and
> > > splash.
> > > * The issue doesn=E2=80=99t happen if a USB flash drive is connected.=
 The content of
> > > the flash drive doesn=E2=80=99t appear to matter. The USB port doesn=
=E2=80=99t appear to
> > > matter.
> > > * No information in any logs after the reboot. I suspect the resume f=
rom
> > > suspend to RAM isn=E2=80=99t getting far enough as that logs could be=
 written.
> > > * Kernel 5.10.205 isn=E2=80=99t affected. Kernel 5.15.145, 6.6.8 and =
6.7-rc7 are
> > > affected.
> > > * A kernel bisect has revealed the following commit as cause:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/comm=
it/?id=3D08d0cc5f34265d1a1e3031f319f594bd1970976c
> > > * The commit was part of kernel 5.20 and has been backported to 5.15.
> > > * The commit mentions that a device-specific quirk could be added in =
case of
> > > new issues.
> > > * According to sysfs and lspci only device 0000:03:00.0 (Intel 8265 W=
ifi)
> > > has ASPM enabled by default.
> > > * Disabling ASPM for device 0000:03:00.0 lets the resume from suspend=
 to RAM
> > > succeed.
> > > * Enabling ASPM for all devices except 0000:03:00.0 lets the resume f=
rom
> > > suspend to RAM succeed.
> > > * This would indicate that a quirk is missing for the device 0000:03:=
00.0
> > > (Intel 8265 Wifi) but I have no clue how to write such a quirk or how=
 to get
> > > the specifics for such a quirk.
> > > * I still have no clue how a USB flash drive plays into all this. May=
be some
> > > kind of a timing issue where the connected USB flash drive delays som=
ething
> > > long enough so that the resume succeeds. Maybe the code removed by co=
mmit
> > > 08d0cc5f34265d1a1e3031f319f594bd1970976c caused a similar delay. =C2=
=AF\_(=E3=83=84)_/=C2=AF
> >
> > Hmmm.  08d0cc5f3426 ("PCI/ASPM: Remove pcie_aspm_pm_state_change()")
> > appeared in v6.0, released Oct 2, 2022, so it's been there a while.
> >
> > But I think the best option is to revert it until this issue is
> > resolved.  Per the commit log, 08d0cc5f3426 solved two problems:
> >
> >   1) ASPM config changes done via sysfs are lost if the device power
> >      state is changed, e.g., typically set to D3hot in .suspend() and
> >      D0 in .resume().
> >
> >   2) If L1SS is restored during system resume, that restored state
> >      would be overwritten.
> >
> > Problem 2) relates to a patch that is currently reverted (a7152be79b62
> > ("Revert "PCI/ASPM: Save L1 PM Substates Capability for
> > suspend/resume""), so I don't think reverting 08d0cc5f3426 will make
> > this problem worse.
> >
> > Reverting 08d0cc5f3426 will make 1) a problem again.  But my guess is
> > ASPM changes via sysfs are fairly unusual and the device probably
> > remains functional even though it may use more power because the ASPM
> > configuration was lost.
> >
> > So unless somebody has a counter-argument, I plan to queue a revert of
> > 08d0cc5f3426 ("PCI/ASPM: Remove pcie_aspm_pm_state_change()") for
> > v6.7.
>
> Hi,
>
> I cannot understand how 1) even occurs. AFAICT, nothing
> pcie_aspm_pm_state_change() calls into overwrites link->aspm_disable that
> is the variable storing user inputs via sysfs. So how the changes via
> sysfs are lost?

Because it's states being enabled via sysfs get overwritten, not the
disabled ones.

Kai-Heng

>
> --
>  i.