On Fri, Jun 08, 2018 at 05:27:12PM +0530, Subrahmanya Lingappa wrote: > Bjorn, > > On Thu, Jun 7, 2018 at 7:29 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote: > > On Thu, Jun 07, 2018 at 04:20:10PM +0530, poza@xxxxxxxxxxxxxx wrote: > >> On 2018-06-07 15:45, Subrahmanya Lingappa wrote: > >> > Bjorn, > >> > > >> > On Wed, Jun 6, 2018 at 6:20 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> > >> > wrote: > >> > > Hi Subrahmanya, > >> > > > >> > > On Wed, Jun 06, 2018 at 05:57:17PM +0530, Subrahmanya Lingappa wrote: > >> > > > Hi, > >> > > > according to https://github.com/torvalds/linux/blob/master/Documentation/PCI/pci-error-recovery.txt > >> > > > > >> > > > as part of AER handling, struct pci_error_handlers{} is > >> > > > implemented by > >> > > > endpoint drivers to handle device specific recovery steps for "struct > >> > > > pci_driver". > >> > > > > >> > > > But we have a platform_driver which implements "struct > >> > > > pci_host_bridge" which also supports AER capability how can we > >> > > > support > >> > > > pci_error_handlers() for the host bridge drivers ? > >> > > > >> > > I assume you're referring to Mobiveil. Can you explain more of the > >> > > topology here? Can you also include "sudo lspci -vv" output? > >> > > > >> > Yes, it is for Mobiveil's Host bridge driver : > >> > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/pci/host/pcie-mobiveil.c > >> > lspci output is now is not available, I'll try to get it sooner. > >> > > >> > we have an endpoint connected directly to Rootport as follows > >> > RC Rootport ----->BUS------> EP > >> > > >> > > The AER capability is an optional capability of PCIe device functions. > >> > > A host bridge is not itself a PCIe function; it's a bridge between a > >> > > platform-specific host bus and the PCIe bus. > >> > > > >> > > Sometimes there is a PCI function that corresponds to the host bridge, > >> > > but that's not required by the PCI specs and there is no generic > >> > > programming model for it. > >> > > > >> > > If you have an PCIe function corresponding to the Mobiveil host > >> > > bridge, and it has an AER capability, what would you want the error > >> > > handlers to do? This function would not normally be a Root Port or > >> > > other type 1 PCI-to-PCI bridge device, so it's not clear how its AER > >> > > would integrate with the PCIe hierarchy. > >> > > > >> > Yes we do have a PCI function with AER capability, after an AER > >> > reported by EP, AER driver initiates an hot_reset on subordinate > >> > bus, which happens to be downstream port for RC. So we get a > >> > downstream port link down happens in this case RC driver needs to > >> > follow a specific register restore sequence, which is most of the > >> > HW specific initialization done in probe function of the driver > >> > to recover properly. > > > > I'm still not quite getting your point. Let's try a concrete example. > > Assume this topology (this is what you described above): > > > > 00:00.0 Root Port to [bus 01] > > 01:00.0 Endpoint > > > > Then we have this situation: > > > > - AER driver binds to Root Port 00:00.0 > > - Endpoint 01:00.0 detects an error and sends ERR_* message upstream > > - Root Port 00:00.0 receives ERR_* message > > - Root Port 00:00.0 generates AER interrupt > > - AER driver handles the interrupt and does a secondary bus reset on > > Root Port 00:00.0. This resets the Endpoint (01:00.0) but not the > > Root Port itself. > > > > It sounds like you need to do an RC register restore sequence at this > > point, i.e., something like mobiveil_host_init()? That sounds like a > > hardware defect. Doing a secondary bus reset via the Root Port's > > bridge control register should have no effect on the RC itself. > > > > What am I missing? > Yes, in this situation PCI config registers are fine, but due to a bug > RC, config space registers will get reset > in the SW we need to call the mobiveil_host_init() again to recover in > case of directly connected EP to the Root-port. > > We do get an local interrupt when this problem occurs, so this can be > handled in Driver's ISR; > or is there a better way to handle it ? I don't see a better way to handle it. If the PCI core were doing something wrong here, e.g., missing some restore required by hardware that follows the spec, we would of course want a nice way to fix it in the core. But this sounds like hardware that does not follow the spec, so the workaround should be in the hardware-specific driver. Note that this may be an issue if/when you want to support ACPI with this hardware, because in that case we use the generic ACPI host bridge driver (drivers/acpi/pci_root.c) and there is less opportunity for device-specific workarounds like this. Bjorn
