On 10/5/20 9:31 AM, Geert Uytterhoeven wrote: > Hi Marek, Hi, > On Sun, Oct 4, 2020 at 4:16 PM Marek Vasut <marek.vasut@xxxxxxxxx> wrote: >> On 9/28/20 11:35 AM, Geert Uytterhoeven wrote: >> [...] >>>> +static int __init rcar_pcie_init(void) >>>> +{ >>>> +#ifdef CONFIG_ARM_LPAE >>>> + hook_fault_code(17, rcar_pcie_aarch32_abort_handler, SIGBUS, 0, >>>> + "asynchronous external abort"); >>>> +#else >>>> + hook_fault_code(22, rcar_pcie_aarch32_abort_handler, SIGBUS, 0, >>>> + "imprecise external abort"); >>>> +#endif >>> >>> As there can be only a single handler, this may interfere with a handler >>> for another platform in a multi-platform kernel. >>> Hence I think this should not be done unconditionally, but be moved to >>> the driver's .probe() callback. >> >> Why is nobody doing this in the probe code then ? It seems all the other > > drivers/pci/controller/dwc/pci-keystone.c is: > > ks_pcie_probe() > ks_pcie_add_pcie_port() > dw_pcie_host_init() > pp->ops->host_init(pp) = ks_pcie_host_init() > hook_fault_code() Well that one is interesting. I wonder whether that driver has the same LPAE bug (different fault code for LPAE and non-LPAE configuration) we found here too, since it is used on CA15 TI SoCs. >> drivers which hook fault code do it in init as well. I can imagine that > > Probably nobody bothered exercising the external abort handler on > multi-platform kernels? > >> something might trip the fault handler even before probe is called, e.g. >> some PM handling or simply user accessing that PCIe area using setpci. > > If that is the case, it must indeed by done earlier, but still > conditional on the presence of the actual PCIe controller. I am open to suggestions how to do that part.