Hi Marek,
On Mon, Oct 5, 2020 at 10:00 AM Marek Vasut <marek.vasut@xxxxxxxxx> wrote:
> On 10/5/20 9:31 AM, Geert Uytterhoeven wrote:
> > 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.
Isn't that an ARM "feature"?
arch/arm/mm/fault.c-/* FSR definition */
arch/arm/mm/fault.c:#ifdef CONFIG_ARM_LPAE
arch/arm/mm/fault.c-#include "fsr-3level.c"
arch/arm/mm/fault.c-#else
arch/arm/mm/fault.c-#include "fsr-2level.c"
arch/arm/mm/fault.c-#endif
> >> 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.
if (of_find_matching_node(...))
do_the_right_stuff();
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
