| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2fa15dbbe2452a08ca4a02b4179844829fb4884f.camel@wdc.com>
Date: Wed, 30 Apr 2025 07:59:05 +0000
From: Wilfred Mallawa <wilfred.mallawa@....com>
To: "robh@...nel.org" <robh@...nel.org>, "kw@...ux.com" <kw@...ux.com>,
"bhelgaas@...gle.com" <bhelgaas@...gle.com>,
"manivannan.sadhasivam@...aro.org" <manivannan.sadhasivam@...aro.org>,
"heiko@...ech.de" <heiko@...ech.de>, "lpieralisi@...nel.org"
<lpieralisi@...nel.org>, "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>
CC: "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "alistair@...stair23.me"
<alistair@...stair23.me>, "linux-pci@...r.kernel.org"
<linux-pci@...r.kernel.org>, "cassel@...nel.org" <cassel@...nel.org>,
"dlemoal@...nel.org" <dlemoal@...nel.org>, "linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>, "linux-rockchip@...ts.infradead.org"
<linux-rockchip@...ts.infradead.org>
Subject: Re: [PATCH] PCI: dwc: Add support for slot reset on link down event
On Wed, 2025-04-30 at 17:43 +1000, Wilfred Mallawa wrote:
> From: Wilfred Mallawa <wilfred.mallawa@....com>
>
> The PCIe link may go down in cases like firmware crashes or unstable
> connections. When this occurs, the PCIe slot must be reset to restore
> functionality. However, the current driver lacks link down handling,
> forcing users to reboot the system to recover.
>
> This patch implements the `reset_slot` callback for link down
> handling
> for DWC PCIe host controller. In which, the RC is reset, reconfigured
> and link training initiated to recover from the link down event.
>
> This patch by extension fixes issues with sysfs initiated bus resets.
> In that, currently, when a sysfs initiated bus reset is issued, the
> endpoint device is non-functional after (may link up with downgraded
> link
> status). With this patch adding support for link down recovery, a
> sysfs
> initiated bus reset works as intended. Testing conducted on a ROCK5B
> board
> with an M.2 NVMe drive.
>
> Signed-off-by: Wilfred Mallawa <wilfred.mallawa@....com>
> ---
> Hey all,
>
> This patch builds ontop of [1] to extend the reset slot support for
> the
> DWC PCIe host controller. Which implements link down recovery support
> for the DesignWare PCIe host controller by adding a `reset_slot`
> callback.
> This allows the system to recover from PCIe link failures without
> requiring a reboot.
>
> This patch by extension improves the behavior of sysfs-initiated bus
> resets.
> Previously, a `reset` issued via sysfs could leave the endpoint in a
> non-functional state or with downgraded link parameters. With the
> added
> link down recovery logic, sysfs resets now result in a properly
> reinitialized
> and fully functional endpoint device. This issue was discovered on a
> Rock5B board, and thus testing was also conducted on the same
> platform
> with a known good M.2 NVMe drive.
>
> Thanks!
>
> [1]
> https://lore.kernel.org/all/20250417-pcie-reset-slot-v3-0-59a10811c962@linaro.org/
> ---
> drivers/pci/controller/dwc/Kconfig | 1 +
> drivers/pci/controller/dwc/pcie-dw-rockchip.c | 89
> ++++++++++++++++++++++++++-
> 2 files changed, 88 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/Kconfig
> b/drivers/pci/controller/dwc/Kconfig
> index
> d9f0386396edf66ad0e514a0f545ed24d89fcb6c..878c52de0842e32ca50dfcc4b66
> 231a73ef436c4 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -347,6 +347,7 @@ config PCIE_ROCKCHIP_DW_HOST
> depends on OF
> select PCIE_DW_HOST
> select PCIE_ROCKCHIP_DW
> + select PCI_HOST_COMMON
> help
> Enables support for the DesignWare PCIe controller in the
> Rockchip SoC (except RK3399) to work in host mode.
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index
> 3c6ab71c996ec1246954f52a9454c8ae67956a54..4c2c683d170f19266e1dfe0efde
> 76d6feb23bf7a 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -23,6 +23,8 @@
> #include <linux/reset.h>
>
> #include "pcie-designware.h"
> +#include "../../pci.h"
> +#include "../pci-host-common.h"
>
> /*
> * The upper 16 bits of PCIE_CLIENT_CONFIG are a write
> @@ -83,6 +85,9 @@ struct rockchip_pcie_of_data {
> const struct pci_epc_features *epc_features;
> };
>
> +static int rockchip_pcie_rc_reset_slot(struct pci_host_bridge
> *bridge,
> + struct pci_dev *pdev);
> +
> static int rockchip_pcie_readl_apb(struct rockchip_pcie *rockchip,
> u32 reg)
> {
> return readl_relaxed(rockchip->apb_base + reg);
> @@ -256,6 +261,7 @@ static int rockchip_pcie_host_init(struct
> dw_pcie_rp *pp)
> rockchip);
>
> rockchip_pcie_enable_l0s(pci);
> + pp->bridge->reset_slot = rockchip_pcie_rc_reset_slot;
>
> return 0;
> }
> @@ -455,6 +461,11 @@ static irqreturn_t
> rockchip_pcie_rc_sys_irq_thread(int irq, void *arg)
> dev_dbg(dev, "PCIE_CLIENT_INTR_STATUS_MISC: %#x\n", reg);
> dev_dbg(dev, "LTSSM_STATUS: %#x\n",
> rockchip_pcie_get_ltssm(rockchip));
>
> + if (reg & PCIE_LINK_REQ_RST_NOT_INT) {
> + dev_dbg(dev, "hot reset or link-down reset\n");
> + pci_host_handle_link_down(pp->bridge);
> + }
> +
> if (reg & PCIE_RDLH_LINK_UP_CHGED) {
> if (rockchip_pcie_link_up(pci)) {
> dev_dbg(dev, "Received Link up event.
> Starting enumeration!\n");
> @@ -536,8 +547,8 @@ static int rockchip_pcie_configure_rc(struct
> platform_device *pdev,
> return ret;
> }
>
> - /* unmask DLL up/down indicator */
> - val = HIWORD_UPDATE(PCIE_RDLH_LINK_UP_CHGED, 0);
> + /* unmask DLL up/down indicator and hot reset/link-down
> reset irq */
> + val = HIWORD_UPDATE(PCIE_RDLH_LINK_UP_CHGED |
> PCIE_LINK_REQ_RST_NOT_INT, 0);
> rockchip_pcie_writel_apb(rockchip, val,
> PCIE_CLIENT_INTR_MASK_MISC);
>
> return ret;
> @@ -688,6 +699,80 @@ static int rockchip_pcie_probe(struct
> platform_device *pdev)
> return ret;
> }
>
> +static int rockchip_pcie_rc_reset_slot(struct pci_host_bridge
> *bridge,
> + struct pci_dev *pdev)
oops, will fixup the alignment here for V2 :)
Wilfred
> +{
> + struct pci_bus *bus = bridge->bus;
> + struct dw_pcie_rp *pp = bus->sysdata;
> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> + struct rockchip_pcie *rockchip = to_rockchip_pcie(pci);
> + struct device *dev = rockchip->pci.dev;
> + u32 val;
> + int ret;
> +
> + dw_pcie_stop_link(pci);
> + rockchip_pcie_phy_deinit(rockchip);
> +
> + ret = reset_control_assert(rockchip->rst);
> + if (ret)
> + return ret;
> +
> + ret = rockchip_pcie_phy_init(rockchip);
> + if (ret)
> + goto disable_regulator;
> +
> + ret = reset_control_deassert(rockchip->rst);
> + if (ret)
> + goto deinit_phy;
> +
> + ret = rockchip_pcie_clk_init(rockchip);
> + if (ret)
> + goto deinit_phy;
> +
> + ret = pp->ops->init(pp);
> + if (ret) {
> + dev_err(dev, "Host init failed: %d\n", ret);
> + goto deinit_clk;
> + }
> +
> + /* LTSSM enable control mode */
> + val = HIWORD_UPDATE_BIT(PCIE_LTSSM_ENABLE_ENHANCE);
> + rockchip_pcie_writel_apb(rockchip, val,
> PCIE_CLIENT_HOT_RESET_CTRL);
> +
> + rockchip_pcie_writel_apb(rockchip, PCIE_CLIENT_RC_MODE,
> PCIE_CLIENT_GENERAL_CON);
> +
> + ret = dw_pcie_setup_rc(pp);
> + if (ret) {
> + dev_err(dev, "Failed to setup RC: %d\n", ret);
> + goto deinit_clk;
> + }
> +
> + /* unmask DLL up/down indicator and hot reset/link-down
> reset irq */
> + val = HIWORD_UPDATE(PCIE_RDLH_LINK_UP_CHGED |
> PCIE_LINK_REQ_RST_NOT_INT, 0);
> + rockchip_pcie_writel_apb(rockchip, val,
> PCIE_CLIENT_INTR_MASK_MISC);
> +
> + ret = dw_pcie_start_link(pci);
> + if (ret)
> + return ret;
> +
> + ret = dw_pcie_wait_for_link(pci);
> + if (ret)
> + return ret;
> +
> + dev_dbg(dev, "Slot reset completed\n");
> + return ret;
> +
> +deinit_clk:
> + clk_bulk_disable_unprepare(rockchip->clk_cnt, rockchip-
> >clks);
> +deinit_phy:
> + rockchip_pcie_phy_deinit(rockchip);
> +disable_regulator:
> + if (rockchip->vpcie3v3)
> + regulator_disable(rockchip->vpcie3v3);
> +
> + return ret;
> +}
> +
> static const struct rockchip_pcie_of_data
> rockchip_pcie_rc_of_data_rk3568 = {
> .mode = DW_PCIE_RC_TYPE,
> };
>
> ---
> base-commit: 08733088b566b58283f0f12fb73f5db6a9a9de30
> change-id: 20250430-b4-pci_dwc_reset_support-d720dbafb7ea
> prerequisite-change-id: 20250404-pcie-reset-slot-730bfa71a202:v3
> prerequisite-patch-id: 2dad85eb26838d89569b12c19d70f392fa592667
> prerequisite-patch-id: 6238a682bd8e9476e5911b7a59263c3fc618d63e
> prerequisite-patch-id: a01300083e94a67ea7c8bfcde320081d90b384d4
> prerequisite-patch-id: ff711f65cf9926374646b76cd38bdd823d576764
> prerequisite-patch-id: a5ee9d4b728b80d32844c5108a5b453eaa4f653f
>
> Best regards,
Powered by blists - more mailing lists