[PATCH] PCI: qcom: Block accesses to downstream devices on link down

[PATCH] PCI: qcom: Block accesses to downstream devices on link down

[Qiang Yu]

After a PCIe link goes down, software may still access the BAR (MMIO)
space or configuration space of devices behind that link before recovery
has run. As the link is down, these accesses never complete, resulting in
a storm of Completion Timeout AERs.

Use the controller's ECAM blocker to drop these accesses to the PCIe
address space as soon as the link-down interrupt fires, so that Completion
Timeout AERs are reduced. The blocked range covers the entire address
space (base 0x0, all-ones limit), since the Root Port's own DBI/iATU
register space remains accessible regardless.

The range is programmed once in the host init path, since the range
registers are wiped by BCR reset. This leaves only the ECAM_BLOCKER_EN
bit to be flipped from the link-down IRQ handler, so the blocker can be
armed with a single fast register write for immediate effect.

The subsequent Root Port reset re-initialises the controller, which clears
the enable bit and re-programs the range for the fresh link.

Signed-off-by: Qiang Yu <qiang.yu@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 54 ++++++++++++++++++++++++++++++++++
 1 file changed, 54 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index b58a607b713f..7005d989f6ff 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -73,6 +73,23 @@
 #define PARF_BDF_TO_SID_TABLE_N			0x2000
 #define PARF_BDF_TO_SID_CFG			0x2c00
 
+/*
+ * ECAM blocker range registers. The blocked range has a write pair
+ * (WR_BASE/WR_LIMIT) and a read pair (RD_BASE/RD_LIMIT); each address is
+ * split into a low (32-bit) and a HI (upper 32-bit) register.
+ */
+#define PARF_BLOCK_SLV_AXI_WR_BASE		0x360
+#define PARF_BLOCK_SLV_AXI_WR_BASE_HI		0x364
+#define PARF_BLOCK_SLV_AXI_WR_LIMIT		0x368
+#define PARF_BLOCK_SLV_AXI_WR_LIMIT_HI		0x36c
+#define PARF_BLOCK_SLV_AXI_RD_BASE		0x370
+#define PARF_BLOCK_SLV_AXI_RD_BASE_HI		0x374
+#define PARF_BLOCK_SLV_AXI_RD_LIMIT		0x378
+#define PARF_BLOCK_SLV_AXI_RD_LIMIT_HI		0x37c
+
+#define PARF_ECAM_BASE				0x380
+#define PARF_ECAM_BASE_HI			0x384
+
 /* ELBI registers */
 #define ELBI_SYS_CTRL				0x04
 #define ELBI_SYS_STTS				0x08
@@ -90,6 +107,7 @@
 
 /* PARF_SYS_CTRL register fields */
 #define MAC_PHY_POWERDOWN_IN_P2_D_MUX_EN	BIT(29)
+#define ECAM_BLOCKER_EN				BIT(26)
 #define MST_WAKEUP_EN				BIT(13)
 #define SLV_WAKEUP_EN				BIT(12)
 #define MSTR_ACLK_CGC_DIS			BIT(10)
@@ -1382,6 +1400,37 @@ static void qcom_pcie_configure_ports(struct qcom_pcie *pcie)
 		dw_pcie_program_t_power_on(pcie->pci, port->l1ss_t_power_on);
 }
 
+static void qcom_pcie_init_ecam_blocker(struct qcom_pcie *pcie)
+{
+	struct dw_pcie *pci = pcie->pci;
+
+	/* ECAM base must match the DBI base address */
+	writel(lower_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE);
+	writel(upper_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE_HI);
+
+	writel(0, pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE);
+	writel(0, pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE_HI);
+	writel(U32_MAX, pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT);
+	writel(U32_MAX, pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT_HI);
+
+	writel(0, pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE);
+	writel(0, pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE_HI);
+	writel(U32_MAX, pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT);
+	writel(U32_MAX, pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT_HI);
+}
+
+static void qcom_pcie_enable_ecam_blocker(struct qcom_pcie *pcie)
+{
+	u32 sys_ctrl;
+
+	sys_ctrl = readl(pcie->parf + PARF_SYS_CTRL);
+	sys_ctrl |= ECAM_BLOCKER_EN;
+	writel(sys_ctrl, pcie->parf + PARF_SYS_CTRL);
+
+	/* Flush the write so the blocker is enabled before this function returns */
+	readl(pcie->parf + PARF_SYS_CTRL);
+}
+
 static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -1422,6 +1471,8 @@ static int qcom_pcie_host_init(struct dw_pcie_rp *pp)
 
 	qcom_pcie_configure_ports(pcie);
 
+	qcom_pcie_init_ecam_blocker(pcie);
+
 	qcom_pcie_perst_deassert(pcie);
 
 	if (pcie->cfg->ops->config_sid) {
@@ -1878,6 +1929,9 @@ static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
 
 	if (test_and_clear_bit(INT_ALL_LINK_DOWN, &status)) {
 		dev_dbg(dev, "Received Link down event\n");
+
+		qcom_pcie_enable_ecam_blocker(pcie);
+
 		for_each_pci_bridge(port, pp->bridge->bus) {
 			if (pci_pcie_type(port) == PCI_EXP_TYPE_ROOT_PORT)
 				pci_host_handle_link_down(port);

---
base-commit: 3de6add8d15430cde96c615fd156216676762bf9
change-id: 20260810-ecam_blocker-f801b3a02bcd

Best regards,
--  
Qiang Yu <qiang.yu@oss.qualcomm.com>

Re: [PATCH] PCI: qcom: Block accesses to downstream devices on link down

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The ECAM blocker enable bit (`ECAM_BLOCKER_EN`) is not explicitly cleared during host initialization, causing downstream devices to remain permanently blocked after a link recovery on platforms lacking a full hardware reset.
- [High] V2-specific ECAM blocker registers are unconditionally programmed on all IP revisions, causing system crashes or state corruption on older hardware.
- [High] Unsynchronized read-modify-write of the `PARF_SYS_CTRL` register between the link-down IRQ thread and the concurrent AER recovery workqueue leads to hardware state corruption.
--

--- Patch [1]: [PATCH] PCI: qcom: Block accesses to downstream devices on link down ---
commit 17695f1fbc53b125cd81eddfebc8a383d0abdbd8
Author: Qiang Yu <qiang.yu@oss.qualcomm.com>

PCI: qcom: Block accesses to downstream devices on link down

This patch introduces an ECAM blocker to drop accesses to the PCIe address
space when a link goes down. The blocker range is programmed during host
initialization, and the blocker is enabled in the link-down IRQ handler.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index b58a607b713f5..7005d989f6ff2 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
[ ... ]
> @@ -1382,6 +1400,37 @@ static void qcom_pcie_configure_ports(struct qcom_pcie *pcie)
>  		dw_pcie_program_t_power_on(pcie->pci, port->l1ss_t_power_on);
>  }
>  
> +static void qcom_pcie_init_ecam_blocker(struct qcom_pcie *pcie)
> +{
> +	struct dw_pcie *pci = pcie->pci;
> +
> +	/* ECAM base must match the DBI base address */
> +	writel(lower_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE);
> +	writel(upper_32_bits(pci->dbi_phys_addr), pcie->parf + PARF_ECAM_BASE_HI);
> +
> +	writel(0, pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE);
> +	writel(0, pcie->parf + PARF_BLOCK_SLV_AXI_WR_BASE_HI);

[Severity: High]
Does this code unconditionally write to V2-specific PARF offsets on all IP
revisions? On older IPs (like v1.0.0 and v2.3.2), these registers might not
exist, as their PARF_DBI_BASE_ADDR is at 0x168 instead. Could writing to these
unmapped or undocumented registers trigger a Synchronous External Abort
and cause a kernel panic during boot on older hardware?

[Severity: High]
Is the ECAM_BLOCKER_EN bit explicitly cleared during this host initialization?
If the controller lacks a full hardware reset on some platforms (like v2.3.2)
where it only cycles clocks and regulators, the bit might remain set from a
prior link-down event.

Could subsequent read-modify-write operations on PARF_SYS_CTRL, such as:

drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_post_init_2_3_2() {
    ...
	val = readl(pcie->parf + PARF_SYS_CTRL);
	val &= ~MAC_PHY_POWERDOWN_IN_P2_D_MUX_EN;
	writel(val, pcie->parf + PARF_SYS_CTRL);
    ...
}

silently preserve this bit and permanently block downstream devices after a
link recovery?

> +	writel(U32_MAX, pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT);
> +	writel(U32_MAX, pcie->parf + PARF_BLOCK_SLV_AXI_WR_LIMIT_HI);
> +
> +	writel(0, pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE);
> +	writel(0, pcie->parf + PARF_BLOCK_SLV_AXI_RD_BASE_HI);
> +	writel(U32_MAX, pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT);
> +	writel(U32_MAX, pcie->parf + PARF_BLOCK_SLV_AXI_RD_LIMIT_HI);
> +}
> +
> +static void qcom_pcie_enable_ecam_blocker(struct qcom_pcie *pcie)
> +{
> +	u32 sys_ctrl;
> +
> +	sys_ctrl = readl(pcie->parf + PARF_SYS_CTRL);
> +	sys_ctrl |= ECAM_BLOCKER_EN;
> +	writel(sys_ctrl, pcie->parf + PARF_SYS_CTRL);

[Severity: High]
Can this unprotected read-modify-write of PARF_SYS_CTRL race with a
concurrent AER recovery workqueue?

If a fatal AER error triggers a Root Port reset, the AER workqueue might
execute initialization functions like qcom_pcie_post_init_2_3_2() locklessly:

drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_post_init_2_3_2() {
    ...
	val = readl(pcie->parf + PARF_SYS_CTRL);
	val &= ~MAC_PHY_POWERDOWN_IN_P2_D_MUX_EN;
	writel(val, pcie->parf + PARF_SYS_CTRL);
    ...
}

Since qcom_pcie_enable_ecam_blocker() is called from the link-down IRQ thread,
could this race silently overwrite vital initialization bits and corrupt
the hardware link state?

> +
> +	/* Flush the write so the blocker is enabled before this function returns */
> +	readl(pcie->parf + PARF_SYS_CTRL);
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810-ecam_blocker-v1-1-e588e07f68d4@oss.qualcomm.com?part=1