[PATCH v4 0/2] PCI: qcom: Implement shutdown() to avoid SMMU/NoC errors on reboot

[PATCH v4 0/2] PCI: qcom: Implement shutdown() to avoid SMMU/NoC errors on reboot

[Krishna Chaitanya Chundru]

During system shutdown/reboot, power/clocks to the PCIe controller get
removed regardless of link state. If the link is still up when that
happens, it can trigger SMMU or NoC errors.

This series adds a shutdown() callback to the Qualcomm PCIe host driver
that forces the link into L2/D3cold before shutdown proceeds, reusing
the existing suspend_noirq() path.

Patch 1 makes dw_pcie_suspend_noirq() skip the D3cold capability check
during shutdown/reboot, since that check can fail and leave the link up
if any endpoint hasn't suspended yet.

Patch 2 adds qcom_pcie_shutdown() and wires it up as .shutdown.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v4:
- removed goto d3cold, (Konrad & Sashiko for PME error)
- Disabling MSI IRQ's (Sashiko)
- Link to v3: https://patch.msgid.link/20260824-shutdown-v3-0-81c14bb7a1af@oss.qualcomm.com

Changes in v3:
- Added null point check and use pm_runtime_put_sync (Sashiko).
- Link to v2: https://patch.msgid.link/20260822-shutdown-v2-0-520a68f1b4a5@oss.qualcomm.com

Changes in v2:
1) don't remove the endpoint pci dev's only keep link in D3cold.
Link to v1: https://lore.kernel.org/all/20250401-shutdown-v1-1-f699859403ae@oss.qualcomm.com/

---
Krishna Chaitanya Chundru (1):
      PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check

Manivannan Sadhasivam (1):
      PCI: qcom: Implement shutdown() callback

 drivers/pci/controller/dwc/pcie-designware-host.c | 16 ++++++-
 drivers/pci/controller/dwc/pcie-qcom.c            | 53 +++++++++++++++++++++++
 2 files changed, 67 insertions(+), 2 deletions(-)
---
base-commit: 570f7e331f5febb30f1384817463c7e42b65ca7d
change-id: 20260822-shutdown-fe8139dff2b7

Best regards,
--  
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

[PATCH v4 1/2] PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check

[Krishna Chaitanya Chundru]

dw_pcie_suspend_noirq() normally calls pci_host_common_d3cold_possible()
to check whether every downstream endpoint can be put into D3cold before
bothering to move the link to L2. If no endpoint supports it, the
function returns early and leaves the link up.

Querying D3cold support during shutdown is actively harmful, not just
slow: pci_host_common_d3cold_possible() requires every active endpoint
to already be in PCI_D3hot, and returns false otherwise. If any endpoint
is still in D0 -- which is common, since endpoint drivers aren't
guaranteed to have suspended by the time the host's shutdown path runs
the check fails and dw_pcie_suspend_noirq() returns early without ever
moving the link to L2, leaving it up right up to the point where the
system cuts power/clocks to the controller.

Detect the shutdown/reboot case via system_state. Still call
pci_host_common_d3cold_possible() unconditionally, since it's also how
"pme_capable" gets set, but ignore its return value when shutting down
and force L2 entry regardless.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index f5a38e6fd8d7..5d07a742faa1 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -13,6 +13,7 @@
 #include <linux/irqchip/chained_irq.h>
 #include <linux/irqchip/irq-msi-lib.h>
 #include <linux/irqdomain.h>
+#include <linux/kernel.h>
 #include <linux/msi.h>
 #include <linux/of_address.h>
 #include <linux/of_pci.h>
@@ -1224,14 +1225,25 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
 
 int dw_pcie_suspend_noirq(struct dw_pcie *pci)
 {
-	bool pme_capable = false;
+	bool shutdown = system_state == SYSTEM_HALT ||
+			system_state == SYSTEM_POWER_OFF ||
+			system_state == SYSTEM_RESTART;
+	bool d3cold, pme_capable = false;
 	int ret = 0;
 	u32 val;
 
 	if (!dw_pcie_link_up(pci))
 		goto stop_link;
 
-	if (!pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
+	/*
+	 * During reboot/halt/poweroff the link is going away regardless, so
+	 * force L2 entry without checking whether endpoints have transitioned
+	 * to D3hot -- there's no point walking the bus to find out. Call
+	 * pci_host_common_d3cold_possible() anyway (ignoring its return value
+	 * in the shutdown case) since it's also how "pme_capable" gets set.
+	 */
+	d3cold = pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable);
+	if (!d3cold && !shutdown)
 		return 0;
 
 	if (pci->pp.ops->pme_turn_off) {

-- 
2.34.1

Re: [PATCH v4 1/2] PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incomplete evaluation of `pme_capable` during shutdown breaks Wake-on-LAN from S5 by improperly cutting auxiliary power.
--

commit d9876255ad827b6e1f2d6b4bca95ab65c2ea8123
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check

This commit modifies dw_pcie_suspend_noirq() to detect the shutdown/reboot
case and forces L2 entry without checking if endpoints have transitioned
to D3hot. It unconditionally calls pci_host_common_d3cold_possible() to set
pme_capable but ignores its return value.

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index f5a38e6fd8d79..5d07a742faa16 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
[ ... ]
> @@ -1224,14 +1225,25 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
>  
>  int dw_pcie_suspend_noirq(struct dw_pcie *pci)
>  {
> -	bool pme_capable = false;
> +	bool shutdown = system_state == SYSTEM_HALT ||
> +			system_state == SYSTEM_POWER_OFF ||
> +			system_state == SYSTEM_RESTART;
> +	bool d3cold, pme_capable = false;
>  	int ret = 0;
>  	u32 val;
>  
>  	if (!dw_pcie_link_up(pci))
>  		goto stop_link;
>  
> -	if (!pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable))
> +	/*
> +	 * During reboot/halt/poweroff the link is going away regardless, so
> +	 * force L2 entry without checking whether endpoints have transitioned
> +	 * to D3hot -- there's no point walking the bus to find out. Call
> +	 * pci_host_common_d3cold_possible() anyway (ignoring its return value
> +	 * in the shutdown case) since it's also how "pme_capable" gets set.
> +	 */
> +	d3cold = pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable);
> +	if (!d3cold && !shutdown)
>  		return 0;
>  
>  	if (pci->pp.ops->pme_turn_off) {

[Severity: High]
The commit message notes that endpoints are frequently left in D0 during
shutdown. If an endpoint is left in D0, won't pci_walk_bus() abort early
during the pci_host_common_d3cold_possible() call?

Looking at __pci_host_common_d3cold_possible() in
drivers/pci/controller/pci-host-common.c, it returns -EOPNOTSUPP if it
encounters a device not in PCI_D3hot:

drivers/pci/controller/pci-host-common.c:__pci_host_common_d3cold_possible() {
    ...
	if (pdev->current_state != PCI_D3hot)
		goto exit;
    ...
exit:
	*flags &= ~PCI_HOST_D3COLD_ALLOWED;

	return -EOPNOTSUPP;
}

Because pci_walk_bus() aborts immediately on a non-zero return, downstream
PME-capable devices won't be evaluated, leaving pme_capable as false.

This would cause the driver to unconditionally cut auxiliary power. Does this
incomplete evaluation of pme_capable break Wake-on-LAN and other PME wakeup
features from the S5 (poweroff) state when devices are left in D0?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-shutdown-v4-0-eb5fe9d454ae@oss.qualcomm.com?part=1

[PATCH v4 2/2] PCI: qcom: Implement shutdown() callback

[Krishna Chaitanya Chundru]

From: Manivannan Sadhasivam <mani@kernel.org>

PCIe host controllers should bring the link down cleanly before system
shutdown/reboot proceeds to remove power/clocks from the controller.
Without this, the link may still be up and endpoints still have
transactions in flight when power/clocks are cut, which can trip SMMU
translation faults or NoC protocol errors.

Reuse dw_pcie_suspend_noirq() in the shutdown path to force the link
into L2, putting it into D3cold.

device_shutdown() runs with interrupts enabled, unlike suspend_noirq().
Mask the chained MSI IRQ(s) and the Global IRQ before tearing down the
link and clocks/PHY, since a late/spurious interrupt could otherwise
reach a handler that touches now-unclocked PARF/DBI registers.

Firmware-managed (ECAM) Root Complexes own their own link teardown and
clock/PHY shutdown; for those, only mask the MSI IRQs.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-qcom.c | 53 ++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index b58a607b713f..fad2f4e2d6be 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -1924,6 +1924,9 @@ static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
 	pp->use_imsi_rx = true;
 	dw_pcie_msi_init(pp);
 
+	/* Stash pci so qcom_pcie_shutdown() can mask the MSI IRQ(s) later */
+	platform_set_drvdata(to_platform_device(dev), pci);
+
 	return devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
 }
 
@@ -2337,6 +2340,55 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	return ret;
 }
 
+static void qcom_pcie_mask_msi_irqs(struct dw_pcie_rp *pp)
+{
+	u32 ctrl;
+
+	/*
+	 * Mask the chained MSI IRQ(s) before tearing down the link and
+	 * clocks/PHY. Unlike suspend_noirq(), device_shutdown() runs with
+	 * interrupts enabled, so a late/spurious MSI could otherwise hit
+	 * dw_chained_msi_isr() and touch DBI registers after the controller
+	 * is powered off.
+	 */
+	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
+		if (pp->msi_irq[ctrl] > 0)
+			disable_irq(pp->msi_irq[ctrl]);
+	}
+}
+
+static void qcom_pcie_shutdown(struct platform_device *pdev)
+{
+	const struct qcom_pcie_cfg *pcie_cfg = of_device_get_match_data(&pdev->dev);
+	struct qcom_pcie *pcie;
+
+	if (pcie_cfg && pcie_cfg->firmware_managed) {
+		/*
+		 * Firmware owns the link teardown and clock/PHY shutdown in
+		 * this mode; Linux only owns the chained MSI IRQ(s), which
+		 * still need to be masked off before shutdown proceeds.
+		 */
+		struct dw_pcie *pci = platform_get_drvdata(pdev);
+
+		if (pci)
+			qcom_pcie_mask_msi_irqs(&pci->pp);
+		return;
+	}
+
+	pcie = platform_get_drvdata(pdev);
+	if (pcie) {
+		qcom_pcie_mask_msi_irqs(&pcie->pci->pp);
+
+		if (pcie->global_irq)
+			disable_irq(pcie->global_irq);
+
+		dw_pcie_suspend_noirq(pcie->pci);
+	}
+
+	pm_runtime_put_sync(&pdev->dev);
+	pm_runtime_disable(&pdev->dev);
+}
+
 static int qcom_pcie_suspend_noirq(struct device *dev)
 {
 	struct qcom_pcie *pcie;
@@ -2519,5 +2571,6 @@ static struct platform_driver qcom_pcie_driver = {
 		.pm = &qcom_pcie_pm_ops,
 		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
 	},
+	.shutdown = qcom_pcie_shutdown,
 };
 builtin_platform_driver(qcom_pcie_driver);

-- 
2.34.1

Re: [PATCH v4 2/2] PCI: qcom: Implement shutdown() callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Critical] The newly added platform_set_drvdata() in qcom_pcie_ecam_host_init() overwrites drvdata with a struct dw_pcie* for firmware-managed platforms, causing catastrophic type confusion and invalid memory dereferences in PM callbacks that expect a struct qcom_pcie*.
- [High] Using disable_irq() on chained MSI interrupts in qcom_pcie_shutdown() fails to synchronize with currently executing handlers, leaving a race condition that can crash the system.

Pre-existing issues:
- [High] Unconditional dereference of optional elbi_base in qcom_pcie_host_pme_turn_off() triggers a crash on platforms where ELBI is not present.
--

commit cb528be41a06ef2211c8efca75a786c37e95604e
Author: Manivannan Sadhasivam <mani@kernel.org>
Date:   Wed Aug 26 07:16:31 2026 +0000

    PCI: qcom: Implement shutdown() callback

This commit implements the shutdown() callback for the Qualcomm PCIe host
controller driver. It tears down the link by reusing the suspend_noirq logic,
forcing the link into L2. It also masks chained MSI IRQs to prevent
late or spurious interrupts after hardware clocks and power are removed.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index b58a607b713f5..fad2f4e2d6be5 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1924,6 +1924,9 @@ static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
>  	pp->use_imsi_rx = true;
>  	dw_pcie_msi_init(pp);
>  
> +	/* Stash pci so qcom_pcie_shutdown() can mask the MSI IRQ(s) later */
> +	platform_set_drvdata(to_platform_device(dev), pci);

[Severity: Critical]
Will this overwrite the platform driver data with a divergent structure type?

PM callbacks like qcom_pcie_suspend_noirq() unconditionally cast the fetched
drvdata to a struct qcom_pcie pointer. Since a struct dw_pcie pointer is stashed
here instead for firmware-managed platforms, won't accessing pcie->pci in the
suspend path resolve to the first field of struct dw_pcie (which is a struct
device pointer)? 

Could this cause an invalid pointer dereference when that corrupted pointer
is passed into dw_pcie_suspend_noirq()?

>  
>  	return devm_add_action_or_reset(dev, qcom_pci_free_msi, pp);
>  }
>  
[ ... ]
> +static void qcom_pcie_mask_msi_irqs(struct dw_pcie_rp *pp)
> +{
> +	u32 ctrl;
> +
> +	/*
> +	 * Mask the chained MSI IRQ(s) before tearing down the link and
> +	 * clocks/PHY. Unlike suspend_noirq(), device_shutdown() runs with
> +	 * interrupts enabled, so a late/spurious MSI could otherwise hit
> +	 * dw_chained_msi_isr() and touch DBI registers after the controller
> +	 * is powered off.
> +	 */
> +	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
> +		if (pp->msi_irq[ctrl] > 0)
> +			disable_irq(pp->msi_irq[ctrl]);

[Severity: High]
Does disable_irq() successfully block for chained handlers here?

Chained IRQ handlers (like dw_chained_msi_isr) act as flow handlers and do not
have the IRQD_IRQ_INPROGRESS flag set on the parent IRQ descriptor. Because of
this, won't the synchronize_irq() call hidden inside disable_irq() return
immediately without waiting for the chained handler to finish?

If an MSI interrupt fires concurrently on another CPU exactly as the system
initiates shutdown, couldn't this leave a race condition where the shutdown
sequence cuts the DBI clocks and power while the chained handler is still
actively reading registers?

> +	}
> +}
> +
> +static void qcom_pcie_shutdown(struct platform_device *pdev)
> +{
[ ... ]
> +	pcie = platform_get_drvdata(pdev);
> +	if (pcie) {
> +		qcom_pcie_mask_msi_irqs(&pcie->pci->pp);
> +
> +		if (pcie->global_irq)
> +			disable_irq(pcie->global_irq);
> +
> +		dw_pcie_suspend_noirq(pcie->pci);

[Severity: High]
This is a pre-existing issue, but does calling the core suspend logic here
unconditionally trigger a crash on platforms without an ELBI region?

In qcom_pcie_host_pme_turn_off(), which is called during the suspend sequence,
there is an unconditional write to elbi_base:

drivers/pci/controller/dwc/pcie-qcom.c:qcom_pcie_host_pme_turn_off() {
    ...
    writel(ELBI_SYS_CTRL_PME_TURNOFF_MSG, pci->elbi_base + ELBI_SYS_CTRL);
    ...
}

Because the new shutdown path unconditionally executes this suspend callback
on all platform resets, won't this guarantee a NULL pointer dereference on
platforms where ELBI is not present (such as Qcom IP rev 2.1.0 APQ8064/IPQ8064)
when they attempt to write to offset 0x04 of a NULL pointer?

> +	}
> +
> +	pm_runtime_put_sync(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826-shutdown-v4-0-eb5fe9d454ae@oss.qualcomm.com?part=2