Re: [PATCH 1/3] PCI/portdrv: Flag services when IRQ is shared with PME

[Kai-Heng Feng]

On Thu, Sep 29, 2022 at 5:32 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
>
> On Wed, Jul 27, 2022 at 09:32:50AM +0800, Kai-Heng Feng wrote:
> > After commit cb1f65c1e142 ("PM: s2idle: ACPI: Fix wakeup interrupts
> > handling"), there's a system that always gets woken up by spurious PME
> > event when one of the root port is put to D3cold.
> >
> > '/sys/power/pm_wakeup_irq' shows 122, which is an IRQ shared between
> > PME, AER and DPC:
> > pcieport 0000:00:01.0: PME: Signaling with IRQ 122
> > pcieport 0000:00:01.0: AER: enabled with IRQ 122
> > pcieport 0000:00:01.0: DPC: enabled with IRQ 122
> >
> > Disabling services one by one and the issue goes away when
> > PCIE_PORT_SERVICE_AER is not enabled. Following the lead, more info can
> > be found on resume when pci_aer_clear_status() is removed from
> > pci_restore_state() to print out what happened:
> > pcieport 0000:00:01.0: AER: Corrected error received: 0000:00:01.0
> > pcieport 0000:00:01.0: PCIe Bus Error: severity=Corrected, type=Physical Layer, (Receiver ID)
> > pcieport 0000:00:01.0:   device [8086:4c01] error status/mask=00000001/00002000
> > pcieport 0000:00:01.0:    [ 0] RxErr
> >
> > Since the corrected AER error happens at physical layer when the root
> > port is transitioning to D3cold, making system be able to suspend is
> > more important than reporting issues like this.
> >
> > So introduce a new flag to indicate when IRQ is shared with PME,
> > therefore AER and DPC can be suspended to prevent any spurious wakeup.
> > HP already has its own suspend routine so it doesn't need to use this
> > flag.
>
> I think it probably does make sense to disable AER and DPC interrupts
> during suspend.  I'm not sure it makes sense to do that conditionally
> based on whether the interrupt is shared.  I think I'd rather disable
> them always, whether the interrupt is shared or not, because then we
> would do the same thing on all machines.  What do you think?

Sorry for the belated response.

I think that sounds reasonable.
I'll send a new version that unconditionally disable AER and DPC IRQ on suspend.

Kai-Heng

>
> Bjorn