Hi Marek: > -----Original Message----- > From: Marek Vasut [mailto:marex@xxxxxxx] > Sent: Wednesday, October 30, 2013 10:55 PM > To: Bjorn Helgaas > Cc: Pratyush Anand; Tim Harvey; linux-pci@xxxxxxxxxxxxxxx; linux-arm- > kernel@xxxxxxxxxxxxxxxxxxx; Sascha Hauer; Zhu Richard-R65037; Shawn Guo; > Jingoo Han; Pratyush Anand; Troy Kisky > Subject: Re: [PATCH] PCI: imx6: fix imprecise abort handler > > Dear Bjorn Helgaas, > > > On Mon, Oct 21, 2013 at 2:16 AM, Pratyush Anand > > > > <pratyush.anand@xxxxxxxxx> wrote: > > > Hi, > > > > > > On Sat, Oct 19, 2013 at 7:03 AM, Marek Vasut <marex@xxxxxxx> wrote: > > >> Dear Tim Harvey, > > >> > > >> > An imprecise abort is triggered when a port behind a switch is > > >> > accessed and no device is present. At enumeration, imprecise > > >> > aborts are not enabled > > > > > > I do not know the complete history. But I do not understand, why > > > imprecise aborts are not enabled during kernel enumeration. > > > What happens when PCIe switch is already connected before boot? > > > pci_common_init will try to read vendor/product ID for a > > > non-existing function during kernel enumeration itself, which should > > > result in imprecise external abort. In fact, I had observed with my > > > system that if a switch is connected to PCIe RC, then abort occurs > > > during kernel initialization itself. So a proper handling was > > > needed. We called hook_fault_code from a subsys_init pcie_init (or > > > similar) function, which insured that abort handler is called > > > whenever, software tries to read vendor/product id of a non-existing > > > function > > > > > >> > thus this ends up getting deferred until the kernel has completed > > >> > init. At that point we must not adjust PC - the handler must do > > >> > nothing, but a handler must exist. > > >> > > > >> > This fixes random crashes that occur right after freeing init. > > >> > This is against linux-pci/host-imx6. > > >> > > > >> > Acked-by: Marek Vasut <marex@xxxxxxx> > > >> > Tested-by: Marek Vasut <marex@xxxxxxx> > > >> > Signed-off-by: Tim Harvey <tharvey@xxxxxxxxxxxxx> > > >> > > >> Expanding CC a bit, let's have more eyes on this. > > >> > > >> > --- > > >> > > > >> > drivers/pci/host/pci-imx6.c | 6 ------ > > >> > 1 file changed, 6 deletions(-) > > >> > > > >> > diff --git a/drivers/pci/host/pci-imx6.c > > >> > b/drivers/pci/host/pci-imx6.c index 966bac6..90fce05 100644 > > >> > --- a/drivers/pci/host/pci-imx6.c > > >> > +++ b/drivers/pci/host/pci-imx6.c > > >> > @@ -200,12 +200,6 @@ static int pcie_phy_write(void __iomem > > >> > *dbi_base, int addr, int data) static int > > >> > imx6q_pcie_abort_handler(unsigned long addr, > > >> > > > >> > unsigned int fsr, struct pt_regs *regs) > > >> > > > >> > { > > >> > > > >> > - /* > > >> > - * If it was an imprecise abort, then we need to correct the > > >> > - * return address to be _after_ the instruction. > > >> > - */ > > >> > - if (fsr & (1 << 10)) > > >> > - regs->ARM_pc += 4; > > > > > > Just for knowledge, is there any case other than read of IDs which > > > might cause imprecise abort? If not, then this handler can be > > > modified a bit to insure that it does not handle any unintended > > > imprecise abort. > > > > > > Regards > > > Pratyush > > > > > >> > return 0; > > >> > > > >> > } > > > > I'm sort of dubious about imx6q_pcie_abort_handler() to begin with -- > > it seems like it assumes that imprecise aborts are either enabled or > > disabled. Isn't there some way it can *check* whether that's the > > case, so it won't break if those aborts are enabled at a different > > point in the future? > > The best way here would be to find a way to prevent the DWC controller from > generating data abort on missing device _at_all_ and just make the read from > that portion of config space be 0xffffffff or zeroes . I didn't find any > configuration bit that would be able to do it though :( Richard, can you maybe > check with FSL if such an option exists on the controller? > [Richard] Unfortunately, discussed with IC guys, there is no such kind of mechanism. > > For now, I'll apply this to my pci/host-imx6 branch for v3.13, but let > > me know if you want to tweak it somehow. > > > > Bjorn -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
