linux-kernel - Re: [PATCH] PCI: pciehp: Do not turn off slot if presence comes up after link

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: <20190209115849.244u67h7wmn3eb7o@wunner.de>
Date:   Sat, 9 Feb 2019 12:58:49 +0100
From:   Lukas Wunner <lukas@...ner.de>
To:     Alexandru Gagniuc <mr.nuke.me@...il.com>
Cc:     bhelgaas@...gle.com, austin_bolen@...l.com,
        alex_gagniuc@...lteam.com, keith.busch@...el.com,
        Shyam_Iyer@...l.com,
        "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: pciehp: Do not turn off slot if presence comes up
 after link

On Tue, Feb 05, 2019 at 03:06:56PM -0600, Alexandru Gagniuc wrote:
> According to PCIe 3.0, the presence detect state is a logical OR of
> in-band and out-of-band presence.

Since Bjorn asked for a spec reference:

PCIe r4.0 sec 7.8.11: "Presence Detect State - This bit indicates the
presence of an adapter in the slot, reflected by the logical OR of the
Physical Layer in-band presence detect mechanism and, if present, any
out-of-band presence detect mechanism defined for the slot's
corresponding form factor."

Table 4-14 in sec 4.2.6 is also important because it shows the difference
between Link Up and in-band presence.

> With this, we'd expect the presence
> state to always be asserted when the link comes up.
> 
> Not all hardware follows this, and it is possible for the presence to
> come up after the link. In this case, the PCIe device would be
> erroneously disabled and re-probed. It is possible to distinguish
> between a delayed presence and a card swap by looking at the DLL state
> changed bit -- The link has to come down if the card is removed.

So in reality, PDC and DLLSC events rarely come in simultaneously and
we do allow some leeway in pcie_wait_for_link(), it's just not enough
in your particular case due to the way presence is detected by the
platform.

I think in this case instead of changing the behavior for everyone,
it's more appropriate to add a quirk for your hardware.  Two ideas:

* Amend pcie_init() to set slot_cap |= PCI_EXP_SLTCAP_ABP.  Insert
  this as a quirk right below the existing Thunderbolt quirk and
  use PCI vendor/device IDs and/or DMI checks to identify your
  particular hardware.  If the ABP bit is set, PDC events are not
  enabled by pcie_enable_notification(), so you will solely rely on
  DLLSC events.  Problem solved.  (Hopefully.)

* Alternatively, add a DECLARE_PCI_FIXUP_FINAL() quirk which sets
  pdev->link_active_reporting = false.  This causes pcie_wait_for_link()
  to wait 1100 msec before the hot-added device's config space is
  accessed for the first time.

Would this work for you?

Thanks,

Lukas