Re: [PATCH v2] PCI: pciehp: Don't enable PME on runtime suspend

[Rafael J. Wysocki]

On Monday, February 06, 2017 10:20:41 PM Lukas Wunner wrote:
> On Mon, Feb 06, 2017 at 11:54:05AM -0600, Bjorn Helgaas wrote:
> > On Mon, Feb 06, 2017 at 06:54:37AM +0100, Lukas Wunner wrote:
> > > Since commit 68db9bc81436 ("PCI: pciehp: Add runtime PM support for PCIe
> > > hotplug ports") we runtime suspend a hotplug port to D3hot when all its
> > > children are runtime suspended or none are present.
> > > 
> > > When runtime suspending the port the PCI core automatically enables PME:
> > >     pci_pm_runtime_suspend()
> > >         pci_finish_runtime_suspend()
> > >             __pci_enable_wake()
> > > 
> > > According to the PCI Express Base Specification, section 6.7.3.4:
> > >    "Note that PME and Hot-Plug Event interrupts (when both are
> > >     implemented) always share the same MSI or MSI-X vector [...]
> > >     If wake generation is required by the associated form factor
> > >     specification, a hot-plug capable Downstream Port must support
> > >     generation of a wakeup event (using the PME mechanism) on hotplug
> > >     events that occur when the system is in a sleep state or the Port
> > >     is in device state D1, D2, or D3Hot."
> > > 
> > > Thus, if the port is runtime suspended even though it is still occupied,
> > > it may immediately be woken by a PME interrupt.
> > 
> > The spec goes on to say that a wakeup event should be generated when
> > all three of these conditions occur:
> > 
> >   - status register for an enabled [hotplug] event transitions from
> >     not set to set
> > 
> >   - Port is in D1, D2, or D3hot,
> > 
> >   - PME_En is set
> > 
> > I think you're saying that if we put a hotplug-capable port that
> > controls an occupied slot into D3hot, the port may immediately
> > generate a wakeup PME.
> > 
> > What is the hotplug event that causes generation of this wakeup event?
> 
> If you had read all e-mails in this thread or looked at the bugzilla
> entry I've created, you wouldn't have to ask this question.
> 
> I think it's disappointing that you're asking me to jump through
> various hoops like creating a bugzilla entry, as well as threatening
> to revert my patch, but are unwilling to even look at the bugzilla
> entry or read the entire thread.  It is equally disappointing that
> the reporter of the regression was unwilling or unable to provide
> dmesg output for both machines so that we've got no real idea what
> we're dealing with.
> 
> It's a Link Up event.
> 
> Which doesn't make sense of course because per the spec PME is only
> supposed to be signaled when the Link Status *changes* from 0 to 1,
> which is not supposed to happen for a slot that has been manually
> disabled.  It's a quirk of this (unknown) hardware.
> 
> 
> > > PME is in fact unnecessary on hotplug ports:  Hotplug can be signaled
> > > even in D3hot,
> > 
> > How are hotplug events signaled in D3hot without using PME?  I'm
> > looking at the PCI PM spec, r1.2, table 5-4 (p 49 in my copy), which
> > says a function in D3hot can only generate PME.
> 
> The table doesn't seem to be in congruence with reality.  The hotplug
> ports on Thunderbolt chips emit hotplug interrupts in D3hot regardless
> of PME.  The Broadcom wireless cards I submitted an early quirk for
> last year likewise emitted interrupts in D3hot.

Which is not spec-compliant, so quirks are the way to go here IMO, for endponts.

Ports are a bit different, because the PCIe spec isn't particularly clear about
what events can be generated by ports in D3hot.  In fact, it is not particuarly
clear what D3hot really means for root ports for that matter.

Thanks,
Rafael