Messages in this thread | | | Subject | Re: [PATCH v3 1/1] PCI/ERR: Fix reset logic in pcie_do_recovery() call | From | "Kuppuswamy, Sathyanarayanan" <> | Date | Mon, 28 Sep 2020 10:15:44 -0700 |
| |
On 9/28/20 4:17 AM, Sinan Kaya wrote: > On 9/27/2020 10:43 PM, Kuppuswamy, Sathyanarayanan wrote: >> FATAL + no-hotplug - In this case, link will still be reseted. But >> currently driver state is not properly restored. So I attempted >> to restore it using pci_reset_bus(). >> >> status = reset_link(dev); >> - if (status != PCI_ERS_RESULT_RECOVERED) { >> + if (status == PCI_ERS_RESULT_RECOVERED) { >> + status = PCI_ERS_RESULT_NEED_RESET; >> >> ... >> >> if (status == PCI_ERS_RESULT_NEED_RESET) { >> /* >> - * TODO: Should call platform-specific >> - * functions to reset slot before calling >> - * drivers' slot_reset callbacks? >> + * TODO: Optimize the call to pci_reset_bus() >> + * >> + * There are two components to pci_reset_bus(). >> + * >> + * 1. Do platform specific slot/bus reset. >> + * 2. Save/Restore all devices in the bus. >> + * >> + * For hotplug capable devices and fatal errors, >> + * device is already in reset state due to link >> + * reset. So repeating platform specific slot/bus >> + * reset via pci_reset_bus() call is redundant. So >> + * can optimize this logic and conditionally call >> + * pci_reset_bus(). >> */ >> + pci_reset_bus(dev); > > I think we have to go to remove/rescan for this case as you also > mentioned above. There is no state to save. All BAR assignments > are gone. Entire device programming is also lost. > > I don't think pci_reset_bus() can recover from this situation safely. > It will make things worse by saving/restoring the hardware default > state. > > This should remove/rescan logic should be inside DPC's slot_reset() > function BTW. Not here. Since there is no state restoration for FATAL errors, I am wondering whether calls to ->error_detected(), ->mmio_enabled() and ->slot_reset() are required?
Let me know your comments about following pseudo code.
if (fatal error & hotplug_supported) do nothing // if fatal triggered by DPC, clear DPC state.
if (fatal error & no-hotplug) perform slot_reset and renumerate affected devices.
>
-- Sathyanarayanan Kuppuswamy Linux Kernel Developer
| |