On 2018-08-16 03:26, Benjamin Herrenschmidt wrote:
(resent using my proper email address)
On Tue, 2018-08-14 at 14:52 +0530, poza@xxxxxxxxxxxxxx wrote:
> > if (result == PCI_ERS_RESULT_RECOVERED) {
> > if (pcie_wait_for_link(udev, true))
> > pci_rescan_bus(udev->bus);
> > - pci_info(dev, "Device recovery from fatal error successful\n");
> > + /* find the pci_dev after rescanning the bus */
> > + dev = pci_get_domain_bus_and_slot(domain, bus, devfn);
>
> one of the motivations was to remove and re-enumerate rather then
> going thorugh driver's recovery sequence
> was; it might be possible that hotplug capable bridge, the device
> might have changed.
> hence this check will fail
Under what circumstances do you actually "unplug" the device ? We are
trying to cleanup/fix some of the PowerPC EEH code which is in a way
similar to AER, and we found that this unplug/replug, which we do if
the driver doesn't have recovery callbacks only, is causing more
problems than it solves.
We are moving toward instead unbinding the driver, resetting the
device, then re-binding the driver instead of unplug/replug.
Also why would you ever bypass the driver callbacks if the driver has
some ? The whole point is to keep the driver bound while resetting the
device (provided it has the right callbacks) so we don't lose the
linkage between stroage devices and mounted filesystems.
there are two different paths we are taking, one is for ERR_FATAL and
the other is for ERR_NONFATAL.
while AER and DPC converge into ERR_FATAL path; ERR_NONFATAL path is the
one where AER and driver callbacks would come into picture.
since DPC does hw recovery of the link, and handles only ERR_FATAL, we
do remove devices and re-enumerate them.
but if the error is no fatal, we will fall back to driver callbacks to
initiate link error handling and recovery process.
under normal circumstances I do not see the some downstream devices
would have been replaced in case of FATAL errors, but code might not
want to assume that
same device is there, since we are removing downstream devices
completely. so taking reference of old BDF and keeping it for later
reference is not good idea.
although I think there are some parts of pcie_do_fatal_recovery need a
fix, which Thomas is fixing is anyway.
so yes, I agree with you and we are talking about same thing e.g.
"
We are moving toward instead unbinding the driver, resetting the
device, then re-binding the driver instead of unplug/replug.
"
driver callbacks are very much there, please have a look at
pcie_do_nonfatal_recovery().
refering Sepc: 6.2.2.2.1 Fatal Errors, where link is unreliable and it
might need AER style reset of link or DPC style HW recovery
In both cases, the shutdown callbacks are expected to be called,
e.g. some driver handle errors ERR_NONFATAL or FATAL in similar ways
e.g.
ioat_pcie_error_detected(); calls ioat_shutdown(); in case of
ERR_NONFATAL
otherwise ioat_shutdown() in case of ERR_FATAL.
Cheers,
Ben.
