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

[Lukas Wunner <lukas@wunner.de>]

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