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

[PATCH v5 0/3] 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 fixes pci_host_common_d3cold_possible()'s underlying bus walk,
which aborts as soon as it finds a device outside D3hot and can
therefore miss a later PME-capable device -- something that becomes
common once patch 3 starts forcing D3cold entry during shutdown while
endpoints may still be in D0.

Patch 2 adds a "force_d3cold" flag to struct dw_pcie_rp that callers can
set to make dw_pcie_suspend_noirq() force L2 entry during shutdown/
reboot, skipping the D3cold capability check that can otherwise leave
the link up if any endpoint hasn't suspended yet.

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

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
Changes in v5:
- use a flag/parameter Instead of poking the system states (Mani)
- Fix pme_wakeup issue by walking through entire bus (Sashiko)
- Fix Irq free issue by using dwc free msi API (Sashiko)
- Link to v4: https://patch.msgid.link/20260826-shutdown-v4-0-eb5fe9d454ae@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 (2):
      PCI: host-common: Fix early bus-walk exit in d3cold_possible()
      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 |  5 +--
 drivers/pci/controller/dwc/pcie-designware.h      |  1 +
 drivers/pci/controller/dwc/pcie-qcom.c            | 38 +++++++++++++++++++++++
 drivers/pci/controller/pci-host-common.c          | 17 +++++-----
 4 files changed, 49 insertions(+), 12 deletions(-)
---
base-commit: 39ee38a9fa2eaeff030a6c865abb244597c091eb
change-id: 20260822-shutdown-fe8139dff2b7

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

[PATCH v5 1/3] PCI: host-common: Fix early bus-walk exit in d3cold_possible()

[Krishna Chaitanya Chundru]

__pci_host_common_d3cold_possible() returns -EOPNOTSUPP for the first
downstream device it finds outside PCI_D3hot, and pci_walk_bus() aborts
the walk as soon as its callback returns nonzero. Any device enumerated
after the disqualifying one -- including a wakeup-enabled, PME-from-D3cold
capable endpoint -- is then never visited, so pme_capable can come back
false even though such a device exists on the bus.

Since pci_host_common_d3cold_possible() already returns false whenever
any device disqualifies D3cold, aborting the walk buys nothing for the
plain suspend path: the overall bool result is unaffected. But it
silently drops pme_capable detection for any device ordered after the
disqualifying one.

This matters for the upcoming shutdown path in particular: unlike plain
suspend, shutdown forces the link into L2/D3cold regardless of whether
pci_host_common_d3cold_possible() itself allows it (see the following
"force_d3cold" changes), so at shutdown time it's common for an
endpoint to still be in D0 and disqualify D3cold while a later,
PME-capable device is never visited. dw_pcie_suspend_noirq() still
uses "pme_capable" to set pci->pp.skip_pwrctrl_off, so an inaccurate
result here can cause Vaux/wakeup support to be dropped for a device
that actually supports PME from D3cold.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/pci-host-common.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index a23907a875e5..78bc4c8c9656 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -274,22 +274,19 @@ static int __pci_host_common_d3cold_possible(struct pci_dev *pdev,
 	if (!pdev->dev.driver && !pci_is_enabled(pdev))
 		return 0;
 
-	if (pdev->current_state != PCI_D3hot)
-		goto exit;
+	if (pdev->current_state != PCI_D3hot) {
+		*flags &= ~PCI_HOST_D3COLD_ALLOWED;
+		return 0;
+	}
 
 	if (device_may_wakeup(&pdev->dev)) {
-		if (!pci_pme_capable(pdev, PCI_D3cold))
-			goto exit;
-		else
+		if (pci_pme_capable(pdev, PCI_D3cold))
 			*flags |= PCI_HOST_PME_D3COLD_CAPABLE;
+		else
+			*flags &= ~PCI_HOST_D3COLD_ALLOWED;
 	}
 
 	return 0;
-
-exit:
-	*flags &= ~PCI_HOST_D3COLD_ALLOWED;
-
-	return -EOPNOTSUPP;
 }
 
 /**

-- 
2.34.1

Re: [PATCH v5 1/3] PCI: host-common: Fix early bus-walk exit in d3cold_possible()

[sashiko-bot]

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

Pre-existing issues:
- [High] PME capability detection is still skipped for devices not in D3hot, leaving the fix incomplete for endpoints in D0.
--

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

PCI: host-common: Fix early bus-walk exit in d3cold_possible()

This commit addresses an issue in __pci_host_common_d3cold_possible() where 
the bus walk was aborted when a downstream device was found outside of 
PCI_D3hot. By changing the return value to 0 instead of -EOPNOTSUPP, it allows 
the bus walk to continue and check subsequent devices for PME-from-D3cold 
capabilities, which is especially important during shutdown when devices might 
still be in D0.

> diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
> index a23907a875e57..78bc4c8c96564 100644
> --- a/drivers/pci/controller/pci-host-common.c
> +++ b/drivers/pci/controller/pci-host-common.c
> @@ -274,22 +274,19 @@ static int __pci_host_common_d3cold_possible(struct pci_dev *pdev,
>  	if (!pdev->dev.driver && !pci_is_enabled(pdev))
>  		return 0;
>  
> -	if (pdev->current_state != PCI_D3hot)
> -		goto exit;
> +	if (pdev->current_state != PCI_D3hot) {
> +		*flags &= ~PCI_HOST_D3COLD_ALLOWED;
> +		return 0;
> +	}

[Severity: High]
This is a pre-existing issue, but doesn't this early return still skip PME 
capability detection for endpoints in D0?

The commit message notes it is common during shutdown for an endpoint to still
be in D0, and this patch aims to ensure later devices on the bus are visited.
However, for the very device in D0, __pci_host_common_d3cold_possible() will
clear PCI_HOST_D3COLD_ALLOWED and return 0 before it checks device_may_wakeup().

Does this mean a PCIe endpoint in D0 that supports Wakeup/PME from D3cold will
fail to have its PME capabilities recorded, causing its auxiliary power (Vaux)
to be incorrectly turned off during shutdown?

>  
>  	if (device_may_wakeup(&pdev->dev)) {
> -		if (!pci_pme_capable(pdev, PCI_D3cold))
> -			goto exit;
> -		else
> +		if (pci_pme_capable(pdev, PCI_D3cold))
>  			*flags |= PCI_HOST_PME_D3COLD_CAPABLE;
> +		else
> +			*flags &= ~PCI_HOST_D3COLD_ALLOWED;
>  	}
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905-shutdown-v5-0-f0c6bafa1d4f@oss.qualcomm.com?part=1

[PATCH v5 2/3] 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.

Add a "force_d3cold" flag to struct dw_pcie_rp that callers set
explicitly to force this behavior, rather than having
dw_pcie_suspend_noirq() infer the shutdown/reboot case itself from
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 force_d3cold is set and force L2 entry regardless.

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 5 +++--
 drivers/pci/controller/dwc/pcie-designware.h      | 1 +
 2 files changed, 4 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..a8a3861c2956 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1224,14 +1224,15 @@ 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 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))
+	d3cold = pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable);
+	if (!d3cold && !pci->pp.force_d3cold)
 		return 0;
 
 	if (pci->pp.ops->pme_turn_off) {
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index a53ac27cd244..8becf4e62703 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -471,6 +471,7 @@ struct dw_pcie_rp {
 	bool			native_ecam;
 	bool                    skip_l23_ready;
 	bool			skip_pwrctrl_off;
+	bool			force_d3cold;
 };
 
 struct dw_pcie_ep_ops {

-- 
2.34.1

Re: [PATCH v5 2/3] 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:

Pre-existing issues:
- [High] Functions in the dw_pcie_suspend_noirq() call path use sleeping functions like read_poll_timeout() and ioremap() in a noirq atomic context, leading to kernel panics.
--

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

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

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. This patch adds a
force_d3cold flag to force this behavior.

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index f5a38e6fd8d79..a8a3861c29560 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -1224,14 +1224,15 @@ 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 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))
> +	d3cold = pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable);
> +	if (!d3cold && !pci->pp.force_d3cold)
>  		return 0;
>  
>  	if (pci->pp.ops->pme_turn_off) {

[Severity: High]
This isn't a bug introduced by this patch, but does the path continuing past
this check lead to a kernel panic in an atomic noirq context?

If we do not return early here, dw_pcie_suspend_noirq() can call
dw_pcie_pme_turn_off(), which dynamically maps memory from a context where
interrupts are disabled:

drivers/pci/controller/dwc/pcie-designware-host.c:dw_pcie_pme_turn_off() {
    ...
    mem = ioremap(pci->pp.msg_res->start, pci->region_align);
    ...
}

Functions like ioremap() take locks and use GFP_KERNEL allocations which can
sleep.

Furthermore, dw_pcie_suspend_noirq() then calls read_poll_timeout():

drivers/pci/controller/dwc/pcie-designware-host.c:dw_pcie_suspend_noirq() {
    ...
    ret = read_poll_timeout(dw_pcie_get_ltssm, val,
                            val == DW_PCIE_LTSSM_L2_IDLE ||
                            val <= DW_PCIE_LTSSM_DETECT_WAIT,
                            PCIE_PME_TO_L2_TIMEOUT_US/10,
                            PCIE_PME_TO_L2_TIMEOUT_US, false, pci);
    ...
}

The read_poll_timeout() macro invokes usleep_range() when the sleep parameter
is non-zero, requiring timer interrupts to be enabled.

In an atomic noirq context, this causes a scheduling while atomic panic or
system deadlock because local interrupts are disabled and scheduling is
forbidden.

Should read_poll_timeout_atomic() be used here instead, and can the ioremap()
call be avoided during noirq suspend?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905-shutdown-v5-0-f0c6bafa1d4f@oss.qualcomm.com?part=2

[PATCH v5 3/3] 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.

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 | 38 ++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index b58a607b713f..38801830fb70 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,40 @@ static int qcom_pcie_probe(struct platform_device *pdev)
 	return ret;
 }
 
+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->pp.use_imsi_rx)
+			dw_pcie_free_msi(&pci->pp);
+		return;
+	}
+
+	pcie = platform_get_drvdata(pdev);
+	if (pcie) {
+		if (pcie->pci->pp.use_imsi_rx)
+			dw_pcie_free_msi(&pcie->pci->pp);
+
+		if (pcie->global_irq)
+			disable_irq(pcie->global_irq);
+
+		pcie->pci->pp.force_d3cold = true;
+		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 +2556,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 v5 3/3] PCI: qcom: Implement shutdown() callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Setting platform drvdata to `struct dw_pcie *` for `firmware_managed` devices introduces a fatal type confusion in system suspend/resume callbacks.
- [High] Calling dw_pcie_free_msi() during shutdown removes chained handlers without synchronization and destroys the non-empty MSI IRQ domain, causing WARN_ON and potential system crashes.
--

commit 8fb4302517aeceeb9b7232585ce3188b6458e92d
Author: Manivannan Sadhasivam <mani@kernel.org>

PCI: qcom: Implement shutdown() callback

PCIe host controllers should bring the link down cleanly before system
shutdown/reboot proceeds to remove power/clocks from the controller.
This commit reuses dw_pcie_suspend_noirq() in the shutdown path and
masks chained MSI and global IRQs before teardown.

> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index b58a607b713f5..38801830fb70f 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);
>  }

[Severity: Critical]
Does setting the platform device's drvdata to a struct dw_pcie pointer here
introduce a fatal type confusion in the system suspend/resume callbacks?

The PM suspend callback qcom_pcie_suspend_noirq() unconditionally retrieves
drvdata expecting a struct qcom_pcie pointer:

qcom_pcie_suspend_noirq() {
	pcie = dev_get_drvdata(dev);
	if (!pcie)
		return 0;

	ret = dw_pcie_suspend_noirq(pcie->pci);
	...
}

For firmware_managed devices, accessing pcie->pci will evaluate to the
first member of struct dw_pcie (which is struct device *dev), passing a
corrupted memory pointer to dw_pcie_suspend_noirq() and causing a guaranteed
kernel panic on sleep.

[ ... ]

> +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->pp.use_imsi_rx)
> +			dw_pcie_free_msi(&pci->pp);
> +		return;
> +	}
> +
> +	pcie = platform_get_drvdata(pdev);
> +	if (pcie) {
> +		if (pcie->pci->pp.use_imsi_rx)
> +			dw_pcie_free_msi(&pcie->pci->pp);

[Severity: High]
Is it safe to call dw_pcie_free_msi() during the shutdown path while child
endpoint mappings still exist and interrupts might be in flight?

In dw_pcie_free_msi(), the code unhooks the chained handler and destroys the
IRQ domain:

drivers/pci/controller/dwc/pcie-designware-host.c:dw_pcie_free_msi() {
	for (ctrl = 0; ctrl < MAX_MSI_CTRLS; ctrl++) {
		if (pp->msi_irq[ctrl] > 0)
			irq_set_chained_handler_and_data(pp->msi_irq[ctrl], NULL, NULL);
	}

	irq_domain_remove(pp->irq_domain);
}

Removing the active IRQ domain triggers a WARN_ON because the domain is
not empty, as child PCI devices haven't freed their IRQs during shutdown.

Furthermore, removing the chained handler does not synchronize against
in-flight IRQs. If an interrupt executes on another CPU while the
controller's clocks and PHY are powered off later in this shutdown
sequence, it risks an SError (synchronous external abort) crashing the
system.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905-shutdown-v5-0-f0c6bafa1d4f@oss.qualcomm.com?part=3