From patchwork Sat Mar 12 21:23:48 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Marek Vasut X-Patchwork-Id: 12778941 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DB823C433FE for ; Sat, 12 Mar 2022 21:24:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231661AbiCLVZa (ORCPT ); Sat, 12 Mar 2022 16:25:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54518 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229803AbiCLVZ3 (ORCPT ); Sat, 12 Mar 2022 16:25:29 -0500 Received: from phobos.denx.de (phobos.denx.de [IPv6:2a01:238:438b:c500:173d:9f52:ddab:ee01]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E37CF2B278; Sat, 12 Mar 2022 13:24:22 -0800 (PST) Received: from tr.lan (ip-89-176-112-137.net.upcbroadband.cz [89.176.112.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 8AFDE81F9A; Sat, 12 Mar 2022 22:24:19 +0100 (CET) From: marek.vasut@gmail.com To: linux-pci@vger.kernel.org Cc: Marek Vasut , Geert Uytterhoeven , Arnd Bergmann , Bjorn Helgaas , =?utf-8?q?Krzysztof_Wilczy=C5=84ski?= , Lorenzo Pieralisi , Wolfram Sang , Yoshihiro Shimoda , linux-renesas-soc@vger.kernel.org Subject: [PATCH v6 1/2] PCI: rcar: Finish transition to L1 state in rcar_pcie_config_access() Date: Sat, 12 Mar 2022 22:23:48 +0100 Message-Id: <20220312212349.781799-1-marek.vasut@gmail.com> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org From: Marek Vasut In case the controller is transitioning to L1 in rcar_pcie_config_access(), any read/write access to PCIECDR triggers asynchronous external abort. This is because the transition to L1 link state must be manually finished by the driver. The PCIe IP can transition back from L1 state to L0 on its own. Avoid triggering the abort in rcar_pcie_config_access() by checking whether the controller is in the transition state, and if so, finish the transition right away. This prevents a lot of unnecessary exceptions, although not all of them. Reviewed-by: Geert Uytterhoeven Tested-by: Geert Uytterhoeven Signed-off-by: Marek Vasut Cc: Arnd Bergmann Cc: Bjorn Helgaas Cc: Geert Uytterhoeven Cc: Krzysztof Wilczyński Cc: Lorenzo Pieralisi Cc: Wolfram Sang Cc: Yoshihiro Shimoda Cc: linux-renesas-soc@vger.kernel.org --- V2: Pull DEFINE_SPINLOCK(pmsr_lock) and rcar_pcie_wakeup() out of ifdef(CONFIG_ARM), since this change is applicable even on arm64 V3: - Convert non-zero return value from rcar_pcie_wakeup() in either PCIBIOS_SET_FAILED in rcar_pcie_config_access(), or, 1 in rcar_pcie_aarch32_abort_handler(). - Set error response using PCI_SET_ERROR_RESPONSE() in rcar_pcie_config_access() - Fix double spinlock unlock in rcar_pcie_aarch32_abort_handler(). V4: No change V5: No change V6: Add RB/TB from Geert --- drivers/pci/controller/pcie-rcar-host.c | 76 +++++++++++++++---------- 1 file changed, 45 insertions(+), 31 deletions(-) diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c index dfca59c4ae34..0966cf63d9b3 100644 --- a/drivers/pci/controller/pcie-rcar-host.c +++ b/drivers/pci/controller/pcie-rcar-host.c @@ -65,6 +65,42 @@ struct rcar_pcie_host { int (*phy_init_fn)(struct rcar_pcie_host *host); }; +static DEFINE_SPINLOCK(pmsr_lock); + +static int rcar_pcie_wakeup(struct device *pcie_dev, void __iomem *pcie_base) +{ + unsigned long flags; + u32 pmsr, val; + int ret = 0; + + spin_lock_irqsave(&pmsr_lock, flags); + + if (!pcie_base || pm_runtime_suspended(pcie_dev)) { + ret = -EINVAL; + 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); + ret = readl_poll_timeout_atomic(pcie_base + PMSR, val, + val & L1FAEG, 10, 1000); + WARN(ret, "Timeout waiting for L1 link state, ret=%d\n", ret); + writel(L1FAEG | PMEL1RX, pcie_base + PMSR); + } + +unlock_exit: + spin_unlock_irqrestore(&pmsr_lock, flags); + return ret; +} + static struct rcar_pcie_host *msi_to_host(struct rcar_msi *msi) { return container_of(msi, struct rcar_pcie_host, msi); @@ -85,6 +121,14 @@ static int rcar_pcie_config_access(struct rcar_pcie_host *host, { struct rcar_pcie *pcie = &host->pcie; unsigned int dev, func, reg, index; + int ret; + + /* Wake the bus up in case it is in L1 state. */ + ret = rcar_pcie_wakeup(pcie->dev, pcie->base); + if (ret) { + PCI_SET_ERROR_RESPONSE(data); + return PCIBIOS_SET_FAILED; + } dev = PCI_SLOT(devfn); func = PCI_FUNC(devfn); @@ -1050,40 +1094,10 @@ static struct platform_driver rcar_pcie_driver = { }; #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; - u32 pmsr, val; - int ret = 0; - - spin_lock_irqsave(&pmsr_lock, flags); - - if (!pcie_base || pm_runtime_suspended(pcie_dev)) { - ret = 1; - 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); - ret = readl_poll_timeout_atomic(pcie_base + PMSR, val, - val & L1FAEG, 10, 1000); - WARN(ret, "Timeout waiting for L1 link state, ret=%d\n", ret); - writel(L1FAEG | PMEL1RX, pcie_base + PMSR); - } - -unlock_exit: - spin_unlock_irqrestore(&pmsr_lock, flags); - return ret; + return !!rcar_pcie_wakeup(pcie_dev, pcie_base); } static const struct of_device_id rcar_pcie_abort_handler_of_match[] __initconst = { From patchwork Sat Mar 12 21:23:49 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Marek Vasut X-Patchwork-Id: 12778942 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BAF7FC4332F for ; Sat, 12 Mar 2022 21:24:26 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232726AbiCLVZa (ORCPT ); Sat, 12 Mar 2022 16:25:30 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54516 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231661AbiCLVZ3 (ORCPT ); Sat, 12 Mar 2022 16:25:29 -0500 Received: from phobos.denx.de (phobos.denx.de [IPv6:2a01:238:438b:c500:173d:9f52:ddab:ee01]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E333C2B26F; Sat, 12 Mar 2022 13:24:22 -0800 (PST) Received: from tr.lan (ip-89-176-112-137.net.upcbroadband.cz [89.176.112.137]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: marex@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 7E8B4820A0; Sat, 12 Mar 2022 22:24:20 +0100 (CET) From: marek.vasut@gmail.com To: linux-pci@vger.kernel.org Cc: Marek Vasut , Arnd Bergmann , Geert Uytterhoeven , Bjorn Helgaas , =?utf-8?q?Krzysztof_Wilczy=C5=84ski?= , Lorenzo Pieralisi , Wolfram Sang , Yoshihiro Shimoda , linux-renesas-soc@vger.kernel.org Subject: [PATCH v6 2/2] PCI: rcar: Use PCI_SET_ERROR_RESPONSE after read which triggered an exception Date: Sat, 12 Mar 2022 22:23:49 +0100 Message-Id: <20220312212349.781799-2-marek.vasut@gmail.com> X-Mailer: git-send-email 2.35.1 In-Reply-To: <20220312212349.781799-1-marek.vasut@gmail.com> References: <20220312212349.781799-1-marek.vasut@gmail.com> MIME-Version: 1.0 X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org From: Marek Vasut In case the controller is transitioning to L1 in rcar_pcie_config_access(), any read/write access to PCIECDR triggers asynchronous external abort. This is because the transition to L1 link state must be manually finished by the driver. The PCIe IP can transition back from L1 state to L0 on its own. The current asynchronous external abort hook implementation restarts the instruction which finally triggered the fault, which can be a different instruction than the read/write instruction which started the faulting access. Usually the instruction which finally triggers the fault is one which has some data dependency on the result of the read/write. In case of read, the read value after fixup is undefined, while a read value of faulting read should be PCI_ERROR_RESPONSE. It is possible to enforce the fault using 'isb' instruction placed right after the read/write instruction which started the faulting access. Add custom register accessors which perform the read/write followed immediately by 'isb'. This way, the fault always happens on the 'isb' and in case of read, which is located one instruction before the 'isb', it is now possible to fix up the return value of the read in the asynchronous external abort hook and make that read return PCI_ERROR_RESPONSE. Reviewed-by: Arnd Bergmann Reviewed-by: Geert Uytterhoeven Tested-by: Geert Uytterhoeven Signed-off-by: Marek Vasut Cc: Arnd Bergmann Cc: Bjorn Helgaas Cc: Geert Uytterhoeven Cc: Krzysztof Wilczyński Cc: Lorenzo Pieralisi Cc: Wolfram Sang Cc: Yoshihiro Shimoda Cc: linux-renesas-soc@vger.kernel.org --- V2: Rebase on 1/2 V3: - Add .text.fixup on all three ldr/str/isb instructions and call fixup_exception() in the abort handler to trigger the fixup. - Propagate return value from read/write accessors, in case the access fails, return PCIBIOS_SET_FAILED, else PCIBIOS_SUCCESSFUL. V4: - Cover both ldr/str and isb with the fixup - Add RB from Arnd - Use PCI_SET_ERROR_RESPONSE instead of val = 0xffffffff - Update commit message V5: - Replace all Fs with PCI_ERROR_RESPONSE in commit message - Make rcar_pci_{read,write}_reg_workaround() static V6: - Add RB/TB from Geert - Add .arch armv7-a to __rcar_pci_rw_reg_workaround() to fix clang build as suggested by Arnd --- drivers/pci/controller/pcie-rcar-host.c | 56 +++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c index 0966cf63d9b3..997c4df6a1e7 100644 --- a/drivers/pci/controller/pcie-rcar-host.c +++ b/drivers/pci/controller/pcie-rcar-host.c @@ -114,6 +114,54 @@ static u32 rcar_read_conf(struct rcar_pcie *pcie, int where) return val >> shift; } +#ifdef CONFIG_ARM +#define __rcar_pci_rw_reg_workaround(instr) \ + " .arch armv7-a\n" \ + "1: " instr " %1, [%2]\n" \ + "2: isb\n" \ + "3: .pushsection .text.fixup,\"ax\"\n" \ + " .align 2\n" \ + "4: mov %0, #" __stringify(PCIBIOS_SET_FAILED) "\n" \ + " b 3b\n" \ + " .popsection\n" \ + " .pushsection __ex_table,\"a\"\n" \ + " .align 3\n" \ + " .long 1b, 4b\n" \ + " .long 2b, 4b\n" \ + " .popsection\n" +#endif + +static int rcar_pci_write_reg_workaround(struct rcar_pcie *pcie, u32 val, + unsigned int reg) +{ + int error = PCIBIOS_SUCCESSFUL; +#ifdef CONFIG_ARM + asm volatile( + __rcar_pci_rw_reg_workaround("str") + : "+r"(error):"r"(val), "r"(pcie->base + reg) : "memory"); +#else + rcar_pci_write_reg(pcie, val, reg); +#endif + return error; +} + +static int rcar_pci_read_reg_workaround(struct rcar_pcie *pcie, u32 *val, + unsigned int reg) +{ + int error = PCIBIOS_SUCCESSFUL; +#ifdef CONFIG_ARM + asm volatile( + __rcar_pci_rw_reg_workaround("ldr") + : "+r"(error), "=r"(*val) : "r"(pcie->base + reg) : "memory"); + + if (error != PCIBIOS_SUCCESSFUL) + PCI_SET_ERROR_RESPONSE(val); +#else + *val = rcar_pci_read_reg(pcie, reg); +#endif + return error; +} + /* Serialization is provided by 'pci_lock' in drivers/pci/access.c */ static int rcar_pcie_config_access(struct rcar_pcie_host *host, unsigned char access_type, struct pci_bus *bus, @@ -185,14 +233,14 @@ static int rcar_pcie_config_access(struct rcar_pcie_host *host, return PCIBIOS_DEVICE_NOT_FOUND; if (access_type == RCAR_PCI_ACCESS_READ) - *data = rcar_pci_read_reg(pcie, PCIECDR); + ret = rcar_pci_read_reg_workaround(pcie, data, PCIECDR); else - rcar_pci_write_reg(pcie, *data, PCIECDR); + ret = rcar_pci_write_reg_workaround(pcie, *data, PCIECDR); /* Disable the configuration access */ rcar_pci_write_reg(pcie, 0, PCIECCTLR); - return PCIBIOS_SUCCESSFUL; + return ret; } static int rcar_pcie_read_conf(struct pci_bus *bus, unsigned int devfn, @@ -1097,7 +1145,7 @@ static struct platform_driver rcar_pcie_driver = { static int rcar_pcie_aarch32_abort_handler(unsigned long addr, unsigned int fsr, struct pt_regs *regs) { - return !!rcar_pcie_wakeup(pcie_dev, pcie_base); + return !fixup_exception(regs); } static const struct of_device_id rcar_pcie_abort_handler_of_match[] __initconst = {