linux-kernel - Re: [PATCH v2 2/2] PCI: pciehp: Disable Data Link Layer State Changed event on suspend

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: <20190215093649.GJ2322@lahna.fi.intel.com>
Date:   Fri, 15 Feb 2019 11:36:49 +0200
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Lukas Wunner <lukas@...ner.de>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Sinan Kaya <okaya@...eaurora.org>,
        Keith Busch <keith.busch@...el.com>,
        Oza Pawandeep <poza@...eaurora.org>, linux-pci@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] PCI: pciehp: Disable Data Link Layer State
 Changed event on suspend

On Thu, Feb 14, 2019 at 03:23:38PM -0600, Bjorn Helgaas wrote:
> On Thu, Jan 31, 2019 at 08:07:46PM +0300, Mika Westerberg wrote:
> > Commit 0e157e528604 ("PCI/PME: Implement runtime PM callbacks") tried to
> > solve an issue where the hierarchy immediately wakes up when it is
> > transitioned into D3cold. However, it turns out to prevent PME
> > propagation on some systems that do not support D3cold.
> > 
> > I looked more closely what might cause the immediate wakeup. It happens
> > when the ACPI power resource of the root port is turned off. The AML
> > code associated with the _OFF() method of the ACPI power resource
> > executes PCIe L2/3 ready transition and waits for it to complete. Right
> > after the L2/3 ready transition is started the root port receives PME
> > from the downstream port.
> > 
> > The simplest hierarchy where this happens looks like this:
> > 
> >   00:1d.0 PCIe Root port
> >     ^
> >     |
> >     v
> >     05:00.0 PCIe switch #1 upstream port
> >       06:01.0 PCIe switch #1 downstream hotplug port
> >         ^
> >         |
> >         v
> >         08:00.0 Pcie switch #2 upstream port
> > 
> > It seems that the PCIe link between the two switches, before
> > PME_Turn_Off/PME_TO_Ack is complete for the whole hierarchy, goes
> > inactive and triggers PME towards the root port bringing it back to D0.
> > The L2/3 ready sequence is described in PCIe r4.0 spec sections 5.2 and
> > 5.3.3 but unfortunately they do not state what happens if DLLSCE is
> > enabled during the sequence.
> 
> The hotplug (and, I guess, the DLLSCE interrupt) and PME interrupts
> are the same INTx wire or MSI vector.  But I guess we know the
> interrupt here must really be a PME because pcie_pme_irq() reads
> PCI_EXP_RTSTA_PME and does nothing unless it is set.
> 
> > Disabling Data Link Layer State Changed event (DLLSCE) seems to prevent
> > the issue and still allows the downstream hotplug port to notice when a
> > device is plugged/unplugged.
> 
> Interesting that Hot-Plug Interrupt Enable by itself doesn't seem to
> gate the DLLSCE notification.  Sec 6.7.3.4 says Hot-Plug Interrupt
> Enable is a master enable/disable bit for "all hot-plug events".  It's
> not completely explicit about what "all hot-plug events" includes, but
> DLLSCE is definitely included in the list in sec 6.7.3.

Indeed the spec is quite vague at places and leaves too much for the
imagination of the reader.

Sec 6.7.3.4 says:

  Software enables a hot-plug event to generate a wakeup event by
  enabling software notification of the event as described in Section
  6.7.3.1. Note that in order for software to disable interrupt
  generation while keeping wakeup generation enabled, the Hot-Plug
  Interrupt Enable bit must be cleared. For form factors that support
  wake generation, a wakeup event must be generated if all
  three of the following conditions occur:
  * The status register for an enabled event transitions from Clearnot
    set to Set
  * The Port is in device state D1, D2, or D3 Hot , and
  * The PME_En bit in the Port’s Power Management Control/Status register is Set

Which to me means that PMEs are generated based on which events are
enabled regardless whether Hot-Plug Interrupt is enabled or not.

> I don't think the bugzilla from Heiner below is actually related to
> *this* patch.  Heiner's system doesn't have the topology above.
> 
> If you have a report for a system where the immediate wakeup happens,
> i.e., something with the topology above, I'd be interested in
> including that report here.  Ideally it would have complete lspci
> info.  I'm just wondering if this is actually a defect in the switch.

I filed a new bugzilla about this here:

  https://bugzilla.kernel.org/show_bug.cgi?id=202593

Let me know if I should re-send with the link corrected.