Re: [PATCH] PCI/PM: Put devices to low power state on shutdown

[Mario Limonciello]

On 9/12/2024 02:02, Kai-Heng Feng wrote:
> On Thu, Sep 12, 2024 at 3:38 AM Mario Limonciello
> <mario.limonciello@xxxxxxx> wrote:
> > On 9/11/2024 14:16, Mario Limonciello wrote:
> > > On 9/11/2024 14:05, Bjorn Helgaas wrote:
> > > > On Fri, Jul 12, 2024 at 02:24:11PM +0800, Kai-Heng Feng wrote:
> > > > > Some laptops wake up after poweroff when HP Thunderbolt Dock G4 is
> > > > > connected.
> > > > >
> > > > > The following error message can be found during shutdown:
> > > > > pcieport 0000:00:1d.0: AER: Correctable error message received from
> > > > > 0000:09:04.0
> > > > > pcieport 0000:09:04.0: PCIe Bus Error: severity=Correctable,
> > > > > type=Data Link Layer, (Receiver ID)
> > > > > pcieport 0000:09:04.0:   device [8086:0b26] error
> > > > > status/mask=00000080/00002000
> > > > > pcieport 0000:09:04.0:    [ 7] BadDLLP
> > > > >
> > > > > Calling aer_remove() during shutdown can quiesce the error message,
> > > > > however the spurious wakeup still happens.
> > > > >
> > > > > The issue won't happen if the device is in D3 before system shutdown, so
> > > > > putting device to low power state before shutdown to solve the issue.
> > > > >
> > > > > I don't have a sniffer so this is purely guesswork, however I believe
> > > > > putting device to low power state it's the right thing to do.
> > > >
> > > > My objection here is that we don't have an explanation of why this
> > > > should matter or a pointer to any spec language about this situation,
> > > > so it feels a little bit random.
> > > >
> > > > I suppose the problem wouldn't happen if AER interrupts were disabled?
> > > > We already do disable them in aer_suspend(), but maybe that's not used
> > > > in the shutdown path?
> > > >
> > > > My understanding is that .shutdown() should turn off device interrupts
> > > > and stop DMA.  So maybe we need an aer_shutdown() that disables
> > > > interrupts?
> > >
> > > IMO I see this commit as two problems with the same solution.
> > >
> > > I don't doubt that cleaning up AER interrupts in the shutdown path would
> > > help AER messages, but you really don't "want" devices to be in D0 when
> > > the system is "off" because even if the system is off some rails are
> > > still active and the device might still be powered.
> > >
> > > A powered device could cause interrupts (IE a spurious wakeup).
> >
> > It's a bit of a stretch, but ACPI 7.4.2.5 and 7.4.2.6 are the closest
> > corollary to a spec I can find.
> >
> > "Devices states are compatible with the current Power Resource states.
> > In other words, all devices are in the D3 state when the system state is
> > S4."
>
> In addition to that, vendor collected the wave form from the device,
> Windows put the device to D3 while Linux kept the device in D0, and
> that asserted one of the PCIe interrupt line to cause system wakeup.

I did the same collection and confirmed the state as well of many PCI 
devices on the system, elsewhere in this thread.

https://gist.github.com/superm1/f8f81e52f5b1d55b64493fdaec38e31c