linux-kernel - Re: [PATCH v2 1/2] PCI: pciehp: Add support for async hotplug with native AER and DPC/EDR

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20230511065636.GA2478@wunner.de>
Date:   Thu, 11 May 2023 08:56:36 +0200
From:   Lukas Wunner <lukas@...ner.de>
To:     Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
Cc:     Sathyanarayanan Kuppuswamy 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        Bjorn Helgaas <bhelgaas@...gle.com>, oohall@...il.com,
        Mahesh J Salgaonkar <mahesh@...ux.ibm.com>,
        Yazen Ghannam <yazen.ghannam@....com>,
        Fontenot Nathan <Nathan.Fontenot@....com>
Subject: Re: [PATCH v2 1/2] PCI: pciehp: Add support for async hotplug with
 native AER and DPC/EDR

On Wed, May 10, 2023 at 02:42:13PM -0700, Smita Koralahalli wrote:
> As far as I can see, async removal solely with DPC is not handled properly
> in Linux.

The dpc driver can react to a DPC event, attempt reset recovery.

But it doesn't do de-enumeration or re-enumeration of subordinate devices.
It also doesn't do slot handling (enable/disable Power Controller etc).
That's only implemented in the hotplug driver.

PCIe r6.0.1 contains appendix I.2 which basically suggests to "use DPC"
for async hot-plug but that doesn't really seem to make sense.

> On AMD systems, PDSC is triggered along with DPC on a async remove. And this
> PDSC event (hotplug handler) will unconfigure and uninitialize the driver
> and device.
> This is one thing which I wanted clarity on as per my question in v1.
> Whether all systems
> trigger PDSC on a async remove along with DPC?

In principle, yes.  Actually the hotplug driver will see both a DLLSC
*and* a PDC event and will react to whichever comes first.  Experience
has shown that the two events may occur in arbitrary order and with
significant delays in-between.

There are systems which erroneously hardwire Presence Detect to zero.
The hotplug driver works even with those.  It solely relies on the
DLLSC event then, see commit 80696f991424 ("PCI: pciehp: Tolerate
Presence Detect hardwired to zero").

> I feel there are two approaches going forward. Since, hotplug handler is
> also
> triggered with PDSC, rely on it to bring down the device and prevent calling
> the
> error_recovery process in dpc handler as its not a true error event. I have
> taken this
> approach.
> 
> Or, don't call the hotplug handler at all and rely on DPC solely to bring
> down the device
> but here, there should be additional callbacks to unconfigure and
> uninitialize the pcie
> driver and currently I only see report_slot_reset() being called from
> error_recovery()
> and I don't think it unconfigures the driver/device.

The latter approach doesn't really make sense to me because we'd have to
duplicate all the slot handling and device de-/re-enumeration in the dpc
driver.

Let's try masking Surprise Down Errors first and see how that goes.

Thanks,

Lukas