Messages in this thread | | | Date | Thu, 11 May 2023 08:56:36 +0200 | From | Lukas Wunner <> | 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
| |