[PATCH 0/2] PCI: qcom: Add support for firmware-managed PCIe controller recovery

[PATCH 0/2] PCI: qcom: Add support for firmware-managed PCIe controller recovery

[Mrinmay Sarkar]

This series adds recovery support for firmware-managed Qualcomm PCIe
controllers.

For these controllers, firmware manages resources such as PHY, clocks,
resets and LTSSM through the power-domain framework.

Patch 1 adds Root Port reset support by power-cycling the controller
through Runtime PM, allowing firmware to reset and reinitialize the
hardware.

Patch 2 restores the DesignWare MSI configuration after resume, as MSI
register contents are lost across the controller power cycle.

Signed-off-by: Mrinmay Sarkar <mrinmay.sarkar@oss.qualcomm.com>
---
Mrinmay Sarkar (2):
      PCI: qcom: Implement .reset_root_port() for firmware-managed controllers
      PCI: qcom: Reinitialize MSI on resume for firmware-managed controllers

 drivers/pci/controller/dwc/pcie-qcom.c | 41 +++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)
---
base-commit: b810335a0699d1810683796b0e3ffe8827ddfe6c
change-id: 20261016-pcie-controller-reset-8938e4bac060

Best regards,
--  
Mrinmay Sarkar <mrinmay.sarkar@oss.qualcomm.com>

[PATCH 1/2] PCI: qcom: Implement .reset_root_port() for firmware-managed controllers

[Mrinmay Sarkar]

During PCIe recovery events, such as a fatal AER event or other
platform-specific PCIe failures, the controller may enter a state that
requires a full reset before normal link operation can be restored.

On firmware-managed PCIe controllers, Linux does not control the PHY,
clocks, resets, or LTSSM state directly. Instead, these resources are
managed by firmware through the power-domain framework.

Implement the reset_root_port() callback by performing a Runtime PM
power cycle of the controller. This allows firmware to reset and
reinitialize the hardware before bringing the controller back online.

Since controller state is lost across the power cycle, reinitialize the
DWC MSI configuration after resume.

This enables Root Port reset and recovery support for firmware-managed
controllers.

Signed-off-by: Mrinmay Sarkar <mrinmay.sarkar@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index d903f10211c2..744455ae90d4 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1855,6 +1855,35 @@ static int qcom_pcie_set_max_opp(struct device *dev)
 	return ret;
 }
 
+/*
+ * Reset for firmware-managed controllers: power-cycle via runtime PM and
+ * reprogram DWC MSI registers on resume.
+ */
+static int qcom_pcie_fw_reset_root_port(struct pci_host_bridge *bridge,
+					struct pci_dev *pdev)
+{
+	struct pci_config_window *cfg = bridge->sysdata;
+	struct device *dev = bridge->dev.parent;
+	struct dw_pcie_rp *pp = cfg->priv;
+	int ret;
+
+	ret = pm_runtime_force_suspend(dev);
+	if (ret) {
+		dev_err(dev, "Failed to force runtime suspend for reset: %d\n", ret);
+		return ret;
+	}
+
+	ret = pm_runtime_force_resume(dev);
+	if (ret) {
+		dev_err(dev, "Failed to force runtime resume after reset: %d\n", ret);
+		return ret;
+	}
+
+	dw_pcie_msi_init(pp);
+
+	return 0;
+}
+
 /*
  * Qcom PCIe controllers only support one Root Port per controller instance. So
  * this function ignores the 'pci_dev' associated with the Root Port and just
@@ -2031,6 +2060,8 @@ static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
 
 	dw_pcie_msi_init(pp);
 
+	cfg->priv = pp;
+
 	return devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
 }
 
@@ -2292,6 +2323,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		bridge->sysdata = cfg;
 		bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
 		bridge->msi_domain = true;
+		bridge->reset_root_port = qcom_pcie_fw_reset_root_port;
 
 		ret = pci_host_probe(bridge);
 		if (ret)

-- 
2.43.0

Re: [PATCH 1/2] PCI: qcom: Implement .reset_root_port() for firmware-managed controllers

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] pci: qcom_pcie_fw_reset_root_port fails unconditionally if CONFIG_PM_SLEEP=n
- [High] pci: qcom: MMIO crash due to PM state corruption via pm_runtime_force_suspend()
- [High] pci/ecam: memory corruption due to cfg->win assignment on 32-bit
- [Medium] pm: async pm_runtime_put() races with pm_runtime_disable() on probe error path
- [Medium] pci: dwc: lockless dw_pcie_msi_init() races with IRQ mask updates
--

commit 758a2a2a75785b89801fabff2d094acd8d02d3de
Author: Mrinmay Sarkar <mrinmay.sarkar@oss.qualcomm.com>

PCI: qcom: Implement .reset_root_port() for firmware-managed controllers

During PCIe recovery events, such as a fatal AER event or other
platform-specific PCIe failures, the controller may enter a state that
requires a full reset before normal link operation can be restored.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index d903f10211c23..744455ae90d46 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1855,6 +1855,35 @@ static int qcom_pcie_set_max_opp(struct device *dev)
>  	return ret;
>  }
>  
> +/*
> + * Reset for firmware-managed controllers: power-cycle via runtime PM and
> + * reprogram DWC MSI registers on resume.
> + */
> +static int qcom_pcie_fw_reset_root_port(struct pci_host_bridge *bridge,
> +					struct pci_dev *pdev)
> +{
> +	struct pci_config_window *cfg = bridge->sysdata;
> +	struct device *dev = bridge->dev.parent;
> +	struct dw_pcie_rp *pp = cfg->priv;
> +	int ret;
> +
> +	ret = pm_runtime_force_suspend(dev);

[Severity: High]
Is there a risk of an SError or kernel panic here due to unclocked or
unpowered MMIO access?

Calling pm_runtime_force_suspend() physically powers off the PM domain.
However, because the controller has active children (PCI devices),
pm_runtime_need_not_resume() evaluates to false, leaving the device's
runtime_status as RPM_ACTIVE while disable_depth is > 0.

If a concurrent thread (like unprivileged userspace reading sysfs) calls
pci_config_pm_runtime_get(), rpm_resume() will see disable_depth > 0
and runtime_status == RPM_ACTIVE. It will incorrectly return success
without powering on the device, and the subsequent MMIO access will hit
powered-off hardware.

> +	if (ret) {
> +		dev_err(dev, "Failed to force runtime suspend for reset: %d\n", ret);
> +		return ret;
> +	}
> +
> +	ret = pm_runtime_force_resume(dev);

[Severity: High]
Does this fail unconditionally on systems where CONFIG_PM_SLEEP is disabled?

In include/linux/pm_runtime.h, if CONFIG_PM_SLEEP is disabled,
pm_runtime_force_resume() is a static inline function that
unconditionally returns -ENXIO.

This guarantees that PCIe root port resets, such as during AER recovery,
will permanently fail and leave the hardware powered off.

> +	if (ret) {
> +		dev_err(dev, "Failed to force runtime resume after reset: %d\n", ret);
> +		return ret;
> +	}
> +
> +	dw_pcie_msi_init(pp);

[Severity: High]
Could this lead to memory corruption on 32-bit architectures?

On 32-bit systems, pci_ecam.c sets per_bus_mapping to true and allocates
cfg->winp as a heap pointer array using kcalloc(). Because cfg->win
and cfg->winp are union members in struct pci_config_window, cfg->win
ends up holding the virtual address of the heap-allocated pointer array.

qcom_pcie_ecam_host_init() unconditionally assigns pci->dbi_base =
cfg->win. When this new reset path calls dw_pcie_msi_init(pp), it uses
pci->dbi_base to write MSI configuration values, which will overwrite
the heap array instead of MMIO registers.

[Severity: Medium]
Is it safe to call dw_pcie_msi_init() locklessly from a concurrent AER
recovery context?

dw_pcie_msi_init() reads pp->irq_mask[ctrl] locklessly and writes it
directly to the hardware register:

drivers/pci/controller/dwc/pcie-designware-host.c:dw_pcie_msi_init() {
    ...
    dw_pcie_writel_dbi(pci, PCIE_MSI_INTR0_MASK + ..., pp->irq_mask[ctrl]);
    ...
}

During AER recovery, this executes from a normal workqueue. If an IRQ
affinity change or driver unmask operation concurrently invokes
dw_pci_bottom_mask() on another CPU, it updates the mask under
pp->lock. The lockless read here could overwrite the hardware register
with a stale mask, desynchronizing the hardware and software states.

> +
> +	return 0;
> +}
> +

[ ... ]

> @@ -2292,6 +2323,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  		bridge->sysdata = cfg;
>  		bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
>  		bridge->msi_domain = true;
> +		bridge->reset_root_port = qcom_pcie_fw_reset_root_port;
>  
>  		ret = pci_host_probe(bridge);
>  		if (ret)

[Severity: Medium]
Does the error path for pci_host_probe() race with pm_runtime_disable()?

When pci_host_probe() fails, the code jumps to the err_pm_runtime_put
label:

err_pm_runtime_put:
    pm_runtime_put(dev);
    pm_runtime_disable(dev);

Since pm_runtime_put() is asynchronous, it queues idle/suspend work
that can be immediately canceled by the subsequent pm_runtime_disable()
before the device is powered down. This leaves the hardware active
and leaks power. Should this use pm_runtime_put_sync() instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-pcie-controller-reset-v1-0-51779736cf12@oss.qualcomm.com?part=1

[PATCH 2/2] PCI: qcom: Reinitialize MSI on resume for firmware-managed controllers

[Mrinmay Sarkar]

On firmware-managed PCIe controllers, hardware context is lost across
system suspend. While firmware reinitializes the controller through the
power domain framework during resume, the DesignWare MSI registers are
not preserved across the power cycle and must be reprogrammed by the
driver.

Retrieve the DesignWare PCIe context and reinitialize MSI state during
the resume path for firmware-managed controllers to restore MSI
functionality after resume.

Signed-off-by: Mrinmay Sarkar <mrinmay.sarkar@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index 744455ae90d4..116622843d2a 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -2324,6 +2324,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 		bridge->ops = (struct pci_ops *)&pci_qcom_ecam_ops.pci_ops;
 		bridge->msi_domain = true;
 		bridge->reset_root_port = qcom_pcie_fw_reset_root_port;
+		dev_set_drvdata(dev, cfg);
 
 		ret = pci_host_probe(bridge);
 		if (ret)
@@ -2551,8 +2552,14 @@ static int qcom_pcie_resume_noirq(struct device *dev)
 	int ret;
 	const struct qcom_pcie_cfg *pcie_cfg = of_device_get_match_data(dev);
 
-	if (pcie_cfg && pcie_cfg->firmware_managed)
+	if (pcie_cfg && pcie_cfg->firmware_managed) {
+		struct pci_config_window *cfg = dev_get_drvdata(dev);
+
+		if (cfg && cfg->priv)
+			dw_pcie_msi_init(cfg->priv);
+
 		return 0;
+	}
 
 	pcie = dev_get_drvdata(dev);
 	if (!pcie)

-- 
2.43.0

Re: [PATCH 2/2] PCI: qcom: Reinitialize MSI on resume for firmware-managed controllers

[sashiko-bot]

> On firmware-managed PCIe controllers, hardware context is lost across
> system suspend. While firmware reinitializes the controller through the
> power domain framework during resume, the DesignWare MSI registers are
> not preserved across the power cycle and must be reprogrammed by the
> driver.
> 
> Retrieve the DesignWare PCIe context and reinitialize MSI state during
> the resume path for firmware-managed controllers to restore MSI
> functionality after resume.
> 
> Signed-off-by: Mrinmay Sarkar <mrinmay.sarkar@oss.qualcomm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-pcie-controller-reset-v1-0-51779736cf12@oss.qualcomm.com?part=2