Re: [PATCH RESEND] PCI: disable runtime PM for PLX switches

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Rafael, linux-pm]

On Mon, Apr 15, 2019 at 09:15:54AM -0500, Bjorn Helgaas wrote:
> This says it's a resend, but I don't see a previous posting; maybe it was
> HTML and rejected by the mailing list?
> 
> On Mon, Apr 15, 2019 at 04:59:03PM +0300, Alexander Fomichev wrote:
> > PLX switches have an issue that their internal registers become inaccessible
> > when runtime PM is enabled. Therefore PLX service tools can't communicate
> > with the hardware. A kernel option "pcie_port_pm=off" can be used as a
> > workaround. But it affects all the devices.
> > So this solution is to add PLX switch devices to the quirk list for
> > disabling runtime PM only for them.
> 
> I assume the problem is actually that the config space registers are
> inaccessible when the device is in D3hot?

Reading this again, I realize you said "internal registers".  I don't
know whether that actually means config space registers (which
*should* work even when the device is in D3hot (see the PCIe reference
below and PCI Power Management Spec r1.2, sec 5.4.1)), or MMIO
registers (which are not expected to work while in D3hot).

If the service tools read MMIO registers, presumably that goes through
some driver that should be able to manage runtime PM.  Or, if there's
no driver, I think your service tool could prevent runtime power
management by changing /sys/devices/.../power/control to "on" (see
Documentation/power/runtime_pm.txt).

Please repost this with more details.

> I think config space access is supposed to work when a device is in D3hot
> (see PCIe r4.0, sec 5.3.1.4).
> 
> If it doesn't work, wouldn't that mean that we couldn't even bring the
> device *out* of D3hot, since that requires a config write?
> 
> If this is really the problem, it'd be nice to identify this specifically
> instead of piggy-backing on the "is_hotplug_bridge" thing, which might be
> coincidentally related, but also carries other meanings.
> 
> > ---
> >  drivers/pci/quirks.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index a59ad09..8ea99aa 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -2923,6 +2923,17 @@ static void quirk_hotplug_bridge(struct pci_dev *dev)
> >  	dev->is_hotplug_bridge = 1;
> >  }
> >  DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_HINT, 0x0020, quirk_hotplug_bridge);
> > +/*
> > + * Disable runtime PM for PLX Draco (1,2), Capella (1,2) series PCIe switches
> > + * to prevent service tools from failing to access hardware registers.
> > + */
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, 0x8712, quirk_hotplug_bridge);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, 0x8733, quirk_hotplug_bridge);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, 0x8734, quirk_hotplug_bridge);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, 0x8780, quirk_hotplug_bridge);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, 0x8796, quirk_hotplug_bridge);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, 0x9781, quirk_hotplug_bridge);
> > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_PLX, 0x9797, quirk_hotplug_bridge);
> >  
> >  /*
> >   * This is a quirk for the Ricoh MMC controller found as a part of some
> > -- 
> > 2.7.4