Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 28 Sep 2022 16:32:09 -0500
From:   Bjorn Helgaas <helgaas@kernel.org>
To:     Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc:     bhelgaas@google.com, mika.westerberg@linux.intel.com,
        koba.ko@canonical.com, sathyanarayanan.kuppuswamy@linux.intel.com,
        Lukas Wunner <lukas@wunner.de>,
        Jan Kiszka <jan.kiszka@siemens.com>,
        Stuart Hayes <stuart.w.hayes@gmail.com>,
        linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/3] PCI/portdrv: Flag services when IRQ is shared with
 PME
Message-ID: <20220928213209.GA1839792@bhelgaas>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220727013255.269815-1-kai.heng.feng@canonical.com>
Precedence: bulk

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?

Bjorn