| Message ID | 20210411185030.8818-1-marek.vasut@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | [V5] PCI: rcar: Add L1 link state fix into data abort hook | expand |
Hi Marek, On Sun, Apr 11, 2021 at 8:50 PM <marek.vasut@gmail.com> wrote: > From: Marek Vasut <marek.vasut+renesas@gmail.com> > > The R-Car PCIe controller is capable of handling L0s/L1 link states. > While the controller can enter and exit L0s link state, and exit L1 > link state, without any additional action from the driver, to enter > L1 link state, the driver must complete the link state transition by > issuing additional commands to the controller. > > The problem is, this transition is not atomic. The controller sets > PMEL1RX bit in PMSR register upon reception of PM_ENTER_L1 DLLP from > the PCIe card, but then the controller enters some sort of inbetween > state. The driver must detect this condition and complete the link > state transition, by setting L1IATN bit in PMCTLR and waiting for > the link state transition to complete. > > If a PCIe access happens inside this window, where the controller > is between L0 and L1 link states, the access generates a fault and > the ARM 'imprecise external abort' handler is invoked. > > Just like other PCI controller drivers, here we hook the fault handler, > perform the fixup to help the controller enter L1 link state, and then > restart the instruction which triggered the fault. Since the controller > is in L1 link state now, the link can exit from L1 link state to L0 and > successfully complete the access. > > While it was suggested to disable L1 link state support completely on > the controller level, this would not prevent the L1 link state entry > initiated by the link partner. This happens e.g. in case a PCIe card > enters D3Hot state, which could be initiated from pci_set_power_state() > if the card indicates D3Hot support, which in turn means link must enter > L1 state. So instead, fix up the L1 link state after all. > > Note that this fixup is applicable only to Aarch32 R-Car controllers, > the Aarch64 R-Car perform the same fixup in TFA, see TFA commit [1] > 0969397f2 ("rcar_gen3: plat: Prevent PCIe hang during L1X config access") > [1] https://github.com/ARM-software/arm-trusted-firmware/commit/0969397f295621aa26b3d14b76dd397d22be58bf > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > V5: - Add mutex around rcar_pcie_aarch32_abort_handler() spinlock (used as a mutex ;-) > - Update commit message again to point out issues with L1/D3Hot states Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org 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
On Sun, Apr 11, 2021 at 08:50:30PM +0200, marek.vasut@gmail.com wrote: [...] > +#ifdef CONFIG_ARM > +static DEFINE_SPINLOCK(pmsr_lock); > +static int rcar_pcie_aarch32_abort_handler(unsigned long addr, > + unsigned int fsr, struct pt_regs *regs) > +{ > + unsigned long flags; > + int ret = 1; I think we should return 1 only if the condition that triggered the fault can't be fixed. If it is fixed on another core we should not return 1 so ret should be set according to the PMSR register state IIUC. Lorenzo > + u32 pmsr; > + > + spin_lock_irqsave(&pmsr_lock, flags); > + > + if (!pcie_base || !__clk_is_enabled(pcie_bus_clk)) > + goto unlock_exit; > + > + pmsr = readl(pcie_base + PMSR); > + > + /* > + * Test if the PCIe controller received PM_ENTER_L1 DLLP and > + * the PCIe controller is not in L1 link state. If true, apply > + * fix, which will put the controller into L1 link state, from > + * which it can return to L0s/L0 on its own. > + */ > + if ((pmsr & PMEL1RX) && ((pmsr & PMSTATE) != PMSTATE_L1)) { > + writel(L1IATN, pcie_base + PMCTLR); > + while (!(readl(pcie_base + PMSR) & L1FAEG)) > + ; > + writel(L1FAEG | PMEL1RX, pcie_base + PMSR); > + ret = 0; > + } > + > +unlock_exit: > + spin_unlock_irqrestore(&pmsr_lock, flags); > + return ret; > +} > + > +static const struct of_device_id rcar_pcie_abort_handler_of_match[] __initconst = { > + { .compatible = "renesas,pcie-r8a7779" }, > + { .compatible = "renesas,pcie-r8a7790" }, > + { .compatible = "renesas,pcie-r8a7791" }, > + { .compatible = "renesas,pcie-rcar-gen2" }, > + {}, > +}; > + > +static int __init rcar_pcie_init(void) > +{ > + if (of_find_matching_node(NULL, rcar_pcie_abort_handler_of_match)) { > +#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 > + } > + > + return platform_driver_register(&rcar_pcie_driver); > +} > +device_initcall(rcar_pcie_init); > +#else > builtin_platform_driver(rcar_pcie_driver); > +#endif > diff --git a/drivers/pci/controller/pcie-rcar.h b/drivers/pci/controller/pcie-rcar.h > index d4c698b5f821..9bb125db85c6 100644 > --- a/drivers/pci/controller/pcie-rcar.h > +++ b/drivers/pci/controller/pcie-rcar.h > @@ -85,6 +85,13 @@ > #define LTSMDIS BIT(31) > #define MACCTLR_INIT_VAL (LTSMDIS | MACCTLR_NFTS_MASK) > #define PMSR 0x01105c > +#define L1FAEG BIT(31) > +#define PMEL1RX BIT(23) > +#define PMSTATE GENMASK(18, 16) > +#define PMSTATE_L1 (3 << 16) > +#define PMCTLR 0x011060 > +#define L1IATN BIT(31) > + > #define MACS2R 0x011078 > #define MACCGSPSETR 0x011084 > #define SPCNGRSN BIT(31) > -- > 2.30.2 >
diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c index 765cf2b45e24..993ffdea3131 100644 --- a/drivers/pci/controller/pcie-rcar-host.c +++ b/drivers/pci/controller/pcie-rcar-host.c @@ -13,6 +13,7 @@ #include <linux/bitops.h> #include <linux/clk.h> +#include <linux/clk-provider.h> #include <linux/delay.h> #include <linux/interrupt.h> #include <linux/irq.h> @@ -41,6 +42,21 @@ struct rcar_msi { int irq2; }; +#ifdef CONFIG_ARM +/* + * Here we keep a static copy of the remapped PCIe controller address. + * This is only used on aarch32 systems, all of which have one single + * PCIe controller, to provide quick access to the PCIe controller in + * the L1 link state fixup function, called from the ARM fault handler. + */ +static void __iomem *pcie_base; +/* + * Static copy of bus clock pointer, so we can check whether the clock + * is enabled or not. + */ +static struct clk *pcie_bus_clk; +#endif + /* Structure representing the PCIe interface */ struct rcar_pcie_host { struct rcar_pcie pcie; @@ -776,6 +792,12 @@ static int rcar_pcie_get_resources(struct rcar_pcie_host *host) } host->msi.irq2 = i; +#ifdef CONFIG_ARM + /* Cache static copy for L1 link state fixup hook on aarch32 */ + pcie_base = pcie->base; + pcie_bus_clk = host->bus_clk; +#endif + return 0; err_irq2: @@ -1031,4 +1053,65 @@ static struct platform_driver rcar_pcie_driver = { }, .probe = rcar_pcie_probe, }; + +#ifdef CONFIG_ARM +static DEFINE_SPINLOCK(pmsr_lock); +static int rcar_pcie_aarch32_abort_handler(unsigned long addr, + unsigned int fsr, struct pt_regs *regs) +{ + unsigned long flags; + int ret = 1; + u32 pmsr; + + spin_lock_irqsave(&pmsr_lock, flags); + + if (!pcie_base || !__clk_is_enabled(pcie_bus_clk)) + goto unlock_exit; + + pmsr = readl(pcie_base + PMSR); + + /* + * Test if the PCIe controller received PM_ENTER_L1 DLLP and + * the PCIe controller is not in L1 link state. If true, apply + * fix, which will put the controller into L1 link state, from + * which it can return to L0s/L0 on its own. + */ + if ((pmsr & PMEL1RX) && ((pmsr & PMSTATE) != PMSTATE_L1)) { + writel(L1IATN, pcie_base + PMCTLR); + while (!(readl(pcie_base + PMSR) & L1FAEG)) + ; + writel(L1FAEG | PMEL1RX, pcie_base + PMSR); + ret = 0; + } + +unlock_exit: + spin_unlock_irqrestore(&pmsr_lock, flags); + return ret; +} + +static const struct of_device_id rcar_pcie_abort_handler_of_match[] __initconst = { + { .compatible = "renesas,pcie-r8a7779" }, + { .compatible = "renesas,pcie-r8a7790" }, + { .compatible = "renesas,pcie-r8a7791" }, + { .compatible = "renesas,pcie-rcar-gen2" }, + {}, +}; + +static int __init rcar_pcie_init(void) +{ + if (of_find_matching_node(NULL, rcar_pcie_abort_handler_of_match)) { +#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 + } + + return platform_driver_register(&rcar_pcie_driver); +} +device_initcall(rcar_pcie_init); +#else builtin_platform_driver(rcar_pcie_driver); +#endif diff --git a/drivers/pci/controller/pcie-rcar.h b/drivers/pci/controller/pcie-rcar.h index d4c698b5f821..9bb125db85c6 100644 --- a/drivers/pci/controller/pcie-rcar.h +++ b/drivers/pci/controller/pcie-rcar.h @@ -85,6 +85,13 @@ #define LTSMDIS BIT(31) #define MACCTLR_INIT_VAL (LTSMDIS | MACCTLR_NFTS_MASK) #define PMSR 0x01105c +#define L1FAEG BIT(31) +#define PMEL1RX BIT(23) +#define PMSTATE GENMASK(18, 16) +#define PMSTATE_L1 (3 << 16) +#define PMCTLR 0x011060 +#define L1IATN BIT(31) + #define MACS2R 0x011078 #define MACCGSPSETR 0x011084 #define SPCNGRSN BIT(31)