* [PATCH v7 1/3] PCI: host-common: Fix early bus-walk exit in d3cold_possible()
2026-10-29 4:19 [PATCH v7 0/3] PCI: qcom: Implement shutdown() to avoid SMMU/NoC errors on reboot Krishna Chaitanya Chundru
@ 2026-10-29 4:19 ` Krishna Chaitanya Chundru
2026-10-29 4:23 ` sashiko-bot
2026-10-29 4:19 ` [PATCH v7 2/3] PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check Krishna Chaitanya Chundru
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-10-29 4:19 UTC (permalink / raw)
To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
Cc: konrad.dybcio, linux-pci, linux-kernel, linux-arm-msm,
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 | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/drivers/pci/controller/pci-host-common.c b/drivers/pci/controller/pci-host-common.c
index a23907a875e5..c2c852c64aa8 100644
--- a/drivers/pci/controller/pci-host-common.c
+++ b/drivers/pci/controller/pci-host-common.c
@@ -275,21 +275,16 @@ static int __pci_host_common_d3cold_possible(struct pci_dev *pdev,
return 0;
if (pdev->current_state != PCI_D3hot)
- goto exit;
+ *flags &= ~PCI_HOST_D3COLD_ALLOWED;
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v7 1/3] PCI: host-common: Fix early bus-walk exit in d3cold_possible()
2026-10-29 4:19 ` [PATCH v7 1/3] PCI: host-common: Fix early bus-walk exit in d3cold_possible() Krishna Chaitanya Chundru
@ 2026-10-29 4:23 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-10-29 4:23 UTC (permalink / raw)
To: Krishna Chaitanya Chundru; +Cc: linux-pci
> __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>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-shutdown-v7-0-3fd7a76a6991@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v7 2/3] PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check
2026-10-29 4:19 [PATCH v7 0/3] PCI: qcom: Implement shutdown() to avoid SMMU/NoC errors on reboot Krishna Chaitanya Chundru
2026-10-29 4:19 ` [PATCH v7 1/3] PCI: host-common: Fix early bus-walk exit in d3cold_possible() Krishna Chaitanya Chundru
@ 2026-10-29 4:19 ` Krishna Chaitanya Chundru
2026-10-29 4:25 ` sashiko-bot
2026-10-29 4:19 ` [PATCH v7 3/3] PCI: qcom: Implement shutdown() callback Krishna Chaitanya Chundru
2026-11-05 14:34 ` [PATCH v7 0/3] PCI: qcom: Implement shutdown() to avoid SMMU/NoC errors on reboot Manivannan Sadhasivam
3 siblings, 1 reply; 9+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-10-29 4:19 UTC (permalink / raw)
To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
Cc: konrad.dybcio, linux-pci, linux-kernel, linux-arm-msm,
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 parameter to dw_pcie_suspend_noirq() that callers set
explicitly to force this behavior, still call
pci_host_common_d3cold_possible() unconditionally, since it's also how
pme_capable gets set, but ignore its return value and force L2 entry
regardless when force is set.
Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
drivers/pci/controller/dwc/pci-imx6.c | 2 +-
drivers/pci/controller/dwc/pci-layerscape.c | 2 +-
drivers/pci/controller/dwc/pcie-designware-host.c | 9 ++++-----
drivers/pci/controller/dwc/pcie-designware.h | 4 ++--
drivers/pci/controller/dwc/pcie-eswin.c | 2 +-
drivers/pci/controller/dwc/pcie-nxp-s32g.c | 2 +-
drivers/pci/controller/dwc/pcie-qcom.c | 2 +-
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 2 +-
drivers/pci/controller/dwc/pcie-stm32.c | 2 +-
drivers/pci/controller/dwc/pcie-ultrarisc.c | 2 +-
10 files changed, 14 insertions(+), 15 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 83fbd998551d..1f2b942e8423 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1788,7 +1788,7 @@ static int imx_pcie_suspend_noirq(struct device *dev)
imx_pcie_assert_perst(imx_pcie, true);
imx_pcie->drvdata->enable_ref_clk(imx_pcie, false);
} else {
- return dw_pcie_suspend_noirq(imx_pcie->pci);
+ return dw_pcie_suspend_noirq(imx_pcie->pci, false);
}
return 0;
diff --git a/drivers/pci/controller/dwc/pci-layerscape.c b/drivers/pci/controller/dwc/pci-layerscape.c
index 7f37a0537159..bfed194834d4 100644
--- a/drivers/pci/controller/dwc/pci-layerscape.c
+++ b/drivers/pci/controller/dwc/pci-layerscape.c
@@ -382,7 +382,7 @@ static int ls_pcie_suspend_noirq(struct device *dev)
if (!pcie->drvdata->pm_support)
return 0;
- return dw_pcie_suspend_noirq(pcie->pci);
+ return dw_pcie_suspend_noirq(pcie->pci, false);
}
static int ls_pcie_resume_noirq(struct device *dev)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 869a39107a81..e4d47999613c 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -1235,23 +1235,22 @@ static int dw_pcie_pme_turn_off(struct dw_pcie *pci)
return 0;
}
-int dw_pcie_suspend_noirq(struct dw_pcie *pci)
+int dw_pcie_suspend_noirq(struct dw_pcie *pci, bool force)
{
- bool d3cold, pme_capable = false;
+ bool d3cold_possible, pme_capable = false;
int ret = 0;
u32 val;
if (!dw_pcie_link_up(pci))
goto stop_link;
- d3cold = pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable);
-
/*
* Enter L2 if D3cold is possible, or if the platform loses RC power
* during suspend (in which case L2 entry is required to notify
* endpoints before power loss, regardless of D3cold support).
*/
- if (!d3cold && !pci->pp.suspend_poweroff)
+ d3cold_possible = pci_host_common_d3cold_possible(pci->pp.bridge, &pme_capable);
+ if (!d3cold_possible && !force && !pci->pp.suspend_poweroff)
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 6d788aa00011..4905932cfaf9 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -861,7 +861,7 @@ static inline enum dw_pcie_ltssm dw_pcie_get_ltssm(struct dw_pcie *pci)
const char *dw_pcie_ltssm_status_string(enum dw_pcie_ltssm ltssm);
#ifdef CONFIG_PCIE_DW_HOST
-int dw_pcie_suspend_noirq(struct dw_pcie *pci);
+int dw_pcie_suspend_noirq(struct dw_pcie *pci, bool force);
int dw_pcie_resume_noirq(struct dw_pcie *pci);
void dw_handle_msi_irq(struct dw_pcie_rp *pp);
void dw_pcie_msi_init(struct dw_pcie_rp *pp);
@@ -874,7 +874,7 @@ int dw_pcie_allocate_msi_domains(struct dw_pcie_rp *pp);
void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus, unsigned int devfn,
int where);
#else
-static inline int dw_pcie_suspend_noirq(struct dw_pcie *pci)
+static inline int dw_pcie_suspend_noirq(struct dw_pcie *pci, bool force)
{
return 0;
}
diff --git a/drivers/pci/controller/dwc/pcie-eswin.c b/drivers/pci/controller/dwc/pcie-eswin.c
index 1bb44696057e..21d077172724 100644
--- a/drivers/pci/controller/dwc/pcie-eswin.c
+++ b/drivers/pci/controller/dwc/pcie-eswin.c
@@ -367,7 +367,7 @@ static int eswin_pcie_suspend_noirq(struct device *dev)
{
struct eswin_pcie *pcie = dev_get_drvdata(dev);
- return dw_pcie_suspend_noirq(&pcie->pci);
+ return dw_pcie_suspend_noirq(&pcie->pci, false);
}
static int eswin_pcie_resume_noirq(struct device *dev)
diff --git a/drivers/pci/controller/dwc/pcie-nxp-s32g.c b/drivers/pci/controller/dwc/pcie-nxp-s32g.c
index 386e44dd0477..cb9cc9327d85 100644
--- a/drivers/pci/controller/dwc/pcie-nxp-s32g.c
+++ b/drivers/pci/controller/dwc/pcie-nxp-s32g.c
@@ -365,7 +365,7 @@ static int s32g_pcie_suspend_noirq(struct device *dev)
struct s32g_pcie *s32g_pp = dev_get_drvdata(dev);
struct dw_pcie *pci = &s32g_pp->pci;
- return dw_pcie_suspend_noirq(pci);
+ return dw_pcie_suspend_noirq(pci, false);
}
static int s32g_pcie_resume_noirq(struct device *dev)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index d903f10211c2..a7835e8a51e0 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -2464,7 +2464,7 @@ static int qcom_pcie_suspend_noirq(struct device *dev)
if (!pcie)
return 0;
- ret = dw_pcie_suspend_noirq(pcie->pci);
+ ret = dw_pcie_suspend_noirq(pcie->pci, false);
if (ret)
return ret;
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index fceecbd16355..045d33ac61ea 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -1071,7 +1071,7 @@ static int rcar_gen4_pcie_suspend_noirq(struct device *dev)
if (rcar->drvdata->mode != DW_PCIE_RC_TYPE)
return 0;
- return dw_pcie_suspend_noirq(&rcar->dw);
+ return dw_pcie_suspend_noirq(&rcar->dw, false);
}
static int rcar_gen4_pcie_resume_noirq(struct device *dev)
diff --git a/drivers/pci/controller/dwc/pcie-stm32.c b/drivers/pci/controller/dwc/pcie-stm32.c
index 349618ea5b9c..7b3a8c1b04a4 100644
--- a/drivers/pci/controller/dwc/pcie-stm32.c
+++ b/drivers/pci/controller/dwc/pcie-stm32.c
@@ -78,7 +78,7 @@ static int stm32_pcie_suspend_noirq(struct device *dev)
struct stm32_pcie *stm32_pcie = dev_get_drvdata(dev);
int ret;
- ret = dw_pcie_suspend_noirq(&stm32_pcie->pci);
+ ret = dw_pcie_suspend_noirq(&stm32_pcie->pci, false);
if (ret)
return ret;
diff --git a/drivers/pci/controller/dwc/pcie-ultrarisc.c b/drivers/pci/controller/dwc/pcie-ultrarisc.c
index 523f454c6c52..59341682213c 100644
--- a/drivers/pci/controller/dwc/pcie-ultrarisc.c
+++ b/drivers/pci/controller/dwc/pcie-ultrarisc.c
@@ -193,7 +193,7 @@ static int ultrarisc_pcie_suspend_noirq(struct device *dev)
if (pci->suspended)
return 0;
- ret = dw_pcie_suspend_noirq(pci);
+ ret = dw_pcie_suspend_noirq(pci, false);
if (ret)
return ret;
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v7 2/3] PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check
2026-10-29 4:19 ` [PATCH v7 2/3] PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check Krishna Chaitanya Chundru
@ 2026-10-29 4:25 ` sashiko-bot
0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-10-29 4:25 UTC (permalink / raw)
To: Krishna Chaitanya Chundru; +Cc: linux-pci
> 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 parameter to dw_pcie_suspend_noirq() that callers set
> explicitly to force this behavior, still call
> pci_host_common_d3cold_possible() unconditionally, since it's also how
> pme_capable gets set, but ignore its return value and force L2 entry
> regardless when force is set.
>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-shutdown-v7-0-3fd7a76a6991@oss.qualcomm.com?part=2
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v7 3/3] PCI: qcom: Implement shutdown() callback
2026-10-29 4:19 [PATCH v7 0/3] PCI: qcom: Implement shutdown() to avoid SMMU/NoC errors on reboot Krishna Chaitanya Chundru
2026-10-29 4:19 ` [PATCH v7 1/3] PCI: host-common: Fix early bus-walk exit in d3cold_possible() Krishna Chaitanya Chundru
2026-10-29 4:19 ` [PATCH v7 2/3] PCI: dwc: Force L2 link entry on shutdown/reboot without D3cold check Krishna Chaitanya Chundru
@ 2026-10-29 4:19 ` Krishna Chaitanya Chundru
2026-10-29 4:39 ` sashiko-bot
2026-10-30 11:26 ` kernel test robot
2026-11-05 14:34 ` [PATCH v7 0/3] PCI: qcom: Implement shutdown() to avoid SMMU/NoC errors on reboot Manivannan Sadhasivam
3 siblings, 2 replies; 9+ messages in thread
From: Krishna Chaitanya Chundru @ 2026-10-29 4:19 UTC (permalink / raw)
To: Jingoo Han, Manivannan Sadhasivam, Lorenzo Pieralisi,
Krzysztof Wilczyński, Rob Herring, Bjorn Helgaas
Cc: konrad.dybcio, linux-pci, linux-kernel, linux-arm-msm,
Krishna Chaitanya Chundru, Manivannan Sadhasivam
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 | 35 ++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
index a7835e8a51e0..423a90d6adfb 100644
--- a/drivers/pci/controller/dwc/pcie-qcom.c
+++ b/drivers/pci/controller/dwc/pcie-qcom.c
@@ -2031,6 +2031,9 @@ static int qcom_pcie_ecam_host_init(struct pci_config_window *cfg)
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);
}
@@ -2451,6 +2454,37 @@ 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;
+ struct dw_pcie *pci;
+
+ 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.
+ */
+ pci = platform_get_drvdata(pdev);
+ } else {
+ pcie = platform_get_drvdata(pdev);
+ pci = pcie->pci;
+
+ if (pcie->global_irq)
+ disable_irq(pcie->global_irq);
+ }
+
+ if (pci->pp.use_imsi_rx)
+ dw_pcie_free_msi(&pci->pp);
+
+ if (pcie)
+ dw_pcie_suspend_noirq(pcie->pci, true);
+
+ pm_runtime_put_sync(&pdev->dev);
+ pm_runtime_disable(&pdev->dev);
+}
+
static int qcom_pcie_suspend_noirq(struct device *dev)
{
struct qcom_pcie *pcie;
@@ -2641,5 +2675,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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v7 3/3] PCI: qcom: Implement shutdown() callback
2026-10-29 4:19 ` [PATCH v7 3/3] PCI: qcom: Implement shutdown() callback Krishna Chaitanya Chundru
@ 2026-10-29 4:39 ` sashiko-bot
2026-10-30 11:26 ` kernel test robot
1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-10-29 4:39 UTC (permalink / raw)
To: Krishna Chaitanya Chundru; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] PCI: qcom: uninitialized pcie pointer in qcom_pcie_shutdown()
- [High] PCI: qcom: premature MSI domain removal in qcom_pcie_shutdown()
- [Medium] PCI: qcom: unclocked DBI access in qcom_pcie_shutdown() if already suspended
--
commit 6dda2390c78b4d6a37d9cd056de73eff45df08e7
Author: Manivannan Sadhasivam <mani@kernel.org>
PCI: qcom: Implement shutdown() callback
This commit implements the platform driver shutdown callback for Qualcomm
PCIe host controllers to mask the Global and chained MSI interrupts, bring
the PCIe link down into L2 via dw_pcie_suspend_noirq(), and disable runtime
PM before system shutdown or reboot cuts clocks and power.
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index a7835e8a51e02..423a90d6adfb4 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
[ ... ]
> @@ -2451,6 +2454,37 @@ 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;
> + struct dw_pcie *pci;
> +
> + 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.
> + */
> + pci = platform_get_drvdata(pdev);
> + } else {
> + pcie = platform_get_drvdata(pdev);
> + pci = pcie->pci;
> +
> + if (pcie->global_irq)
> + disable_irq(pcie->global_irq);
> + }
[Severity: High]
Should pcie be initialized to NULL in qcom_pcie_shutdown() in
drivers/pci/controller/dwc/pcie-qcom.c?
When pcie_cfg && pcie_cfg->firmware_managed is true, the first branch only
assigns pci = platform_get_drvdata(pdev) and leaves pcie uninitialized on
the stack.
Later in qcom_pcie_shutdown():
if (pcie)
dw_pcie_suspend_noirq(pcie->pci, true);
evaluating if (pcie) reads the uninitialized stack variable and dereferences
pcie->pci on firmware-managed platforms.
> +
> + if (pci->pp.use_imsi_rx)
> + dw_pcie_free_msi(&pci->pp);
[Severity: High]
Is it safe for qcom_pcie_shutdown() in drivers/pci/controller/dwc/pcie-qcom.c
to call dw_pcie_free_msi() while downstream PCI devices and their MSI
mappings are still active?
In drivers/pci/controller/dwc/pcie-designware-host.c, dw_pcie_free_msi()
does not just detach the chained handler, it also destroys the MSI domain:
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->msi_irq_domain);
Unlike dw_pcie_host_deinit(), device_shutdown() does not remove PCI devices
or tear down per-device child MSI domains, and endpoint drivers that do not
implement a shutdown callback (or do not call pci_free_irq_vectors() in
shutdown) leave active MSI mappings and child domains referencing
pp->msi_irq_domain after irq_domain_remove() frees it.
During a kexec reboot, after device_shutdown() completes and
qcom_pcie_shutdown() has disabled controller clocks and runtime PM,
kernel_kexec() takes down non-boot CPUs via:
machine_shutdown()
smp_shutdown_nonboot_cpus()
irq_migrate_all_off_this_cpu()
migrate_one_irq()
In migrate_one_irq(), managed-affinity MSI interrupts call
irq_shutdown_and_deactivate() -> irq_domain_deactivate_irq() ->
__irq_domain_deactivate_irq() in kernel/irq/irqdomain.c, which walks
irq_data->parent_data and dereferences the freed pp->msi_irq_domain. For
other active MSI interrupts, migrate_one_irq() masks and unmasks the IRQ via
dw_pci_bottom_mask() and dw_pci_bottom_unmask(), which call
dw_pcie_writel_dbi() on DBI registers after the controller clocks and power
have been turned off.
Would detaching only the chained MSI handler (or disabling the parent MSI
IRQs) without removing pp->msi_irq_domain avoid both the use-after-free and
oraning active MSI descriptors?
> +
> + if (pcie)
> + dw_pcie_suspend_noirq(pcie->pci, true);
[Severity: Medium]
What happens here in qcom_pcie_shutdown() in
drivers/pci/controller/dwc/pcie-qcom.c if the controller is already
suspended when system shutdown or reboot runs?
If a previous system suspend succeeded via qcom_pcie_suspend_noirq() ->
dw_pcie_suspend_noirq(), and a subsequent qcom_pcie_resume_noirq() failed
(for example, if dw_pcie_resume_noirq() or icc_enable() returned an error),
pcie->pci->suspended remains true while the controller clocks, PHY,
regulators, and icc_cpu/icc_mem interconnects are disabled.
When qcom_pcie_shutdown() then calls dw_pcie_suspend_noirq(pcie->pci, true)
without checking pcie->pci->suspended, dw_pcie_suspend_noirq() in
drivers/pci/controller/dwc/pcie-designware-host.c immediately accesses DBI
registers via dw_pcie_link_up() -> qcom_pcie_link_up() while clocks and
interconnects are off, and then runs qcom_pcie_host_deinit() a second time.
Should this check if (!pcie->pci->suspended) before calling
dw_pcie_suspend_noirq()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261029-shutdown-v7-0-3fd7a76a6991@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v7 3/3] PCI: qcom: Implement shutdown() callback
2026-10-29 4:19 ` [PATCH v7 3/3] PCI: qcom: Implement shutdown() callback Krishna Chaitanya Chundru
2026-10-29 4:39 ` sashiko-bot
@ 2026-10-30 11:26 ` kernel test robot
1 sibling, 0 replies; 9+ messages in thread
From: kernel test robot @ 2026-10-30 11:26 UTC (permalink / raw)
To: Krishna Chaitanya Chundru, Jingoo Han, Manivannan Sadhasivam,
Lorenzo Pieralisi, Krzysztof Wilczyński, Rob Herring,
Bjorn Helgaas
Cc: llvm, oe-kbuild-all, konrad.dybcio, linux-pci, linux-kernel,
linux-arm-msm, Krishna Chaitanya Chundru
Hi Krishna,
kernel test robot noticed the following build warnings:
[auto build test WARNING on 17bb0e2f6c8043f70a693a5f3797471cbed354c8]
url: https://github.com/intel-lab-lkp/linux/commits/Krishna-Chaitanya-Chundru/PCI-host-common-Fix-early-bus-walk-exit-in-d3cold_possible/20261029-094927
base: 17bb0e2f6c8043f70a693a5f3797471cbed354c8
patch link: https://lore.kernel.org/r/20261029-shutdown-v7-3-3fd7a76a6991%40oss.qualcomm.com
patch subject: [PATCH v7 3/3] PCI: qcom: Implement shutdown() callback
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20261030/202610302144.Iwq8RoDq-lkp@intel.com/config)
compiler: clang version 24.0.0git (https://github.com/llvm/llvm-project c2ea4440d4b64c148de4fdfca1fc2c437a0701e4)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261030/202610302144.Iwq8RoDq-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610302144.Iwq8RoDq-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> drivers/pci/controller/dwc/pcie-qcom.c:2463:6: warning: variable 'pcie' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
2463 | if (pcie_cfg && pcie_cfg->firmware_managed) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/pci/controller/dwc/pcie-qcom.c:2481:6: note: uninitialized use occurs here
2481 | if (pcie)
| ^~~~
drivers/pci/controller/dwc/pcie-qcom.c:2463:2: note: remove the 'if' if its condition is always false
2463 | if (pcie_cfg && pcie_cfg->firmware_managed) {
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2464 | /*
| ~~
2465 | * Firmware owns the link teardown and clock/PHY shutdown in
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2466 | * this mode; Linux only owns the chained MSI IRQ(s), which
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2467 | * still need to be masked off before shutdown proceeds.
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2468 | */
| ~~
2469 | pci = platform_get_drvdata(pdev);
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2470 | } else {
| ~~~~~~
drivers/pci/controller/dwc/pcie-qcom.c:2460:24: note: initialize the variable 'pcie' to silence this warning
2460 | struct qcom_pcie *pcie;
| ^
| = NULL
1 warning generated.
vim +2463 drivers/pci/controller/dwc/pcie-qcom.c
2456
2457 static void qcom_pcie_shutdown(struct platform_device *pdev)
2458 {
2459 const struct qcom_pcie_cfg *pcie_cfg = of_device_get_match_data(&pdev->dev);
2460 struct qcom_pcie *pcie;
2461 struct dw_pcie *pci;
2462
> 2463 if (pcie_cfg && pcie_cfg->firmware_managed) {
2464 /*
2465 * Firmware owns the link teardown and clock/PHY shutdown in
2466 * this mode; Linux only owns the chained MSI IRQ(s), which
2467 * still need to be masked off before shutdown proceeds.
2468 */
2469 pci = platform_get_drvdata(pdev);
2470 } else {
2471 pcie = platform_get_drvdata(pdev);
2472 pci = pcie->pci;
2473
2474 if (pcie->global_irq)
2475 disable_irq(pcie->global_irq);
2476 }
2477
2478 if (pci->pp.use_imsi_rx)
2479 dw_pcie_free_msi(&pci->pp);
2480
2481 if (pcie)
2482 dw_pcie_suspend_noirq(pcie->pci, true);
2483
2484 pm_runtime_put_sync(&pdev->dev);
2485 pm_runtime_disable(&pdev->dev);
2486 }
2487
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v7 0/3] PCI: qcom: Implement shutdown() to avoid SMMU/NoC errors on reboot
2026-10-29 4:19 [PATCH v7 0/3] PCI: qcom: Implement shutdown() to avoid SMMU/NoC errors on reboot Krishna Chaitanya Chundru
` (2 preceding siblings ...)
2026-10-29 4:19 ` [PATCH v7 3/3] PCI: qcom: Implement shutdown() callback Krishna Chaitanya Chundru
@ 2026-11-05 14:34 ` Manivannan Sadhasivam
3 siblings, 0 replies; 9+ messages in thread
From: Manivannan Sadhasivam @ 2026-11-05 14:34 UTC (permalink / raw)
To: Krishna Chaitanya Chundru
Cc: Jingoo Han, Lorenzo Pieralisi, Krzysztof Wilczyński,
Rob Herring, Bjorn Helgaas, konrad.dybcio, linux-pci,
linux-kernel, linux-arm-msm
On Thu, Oct 29, 2026 at 09:49:26AM +0530, Krishna Chaitanya Chundru wrote:
> 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 parameter to dw_pcie_suspend_noirq() so 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>
Please fix the build issue reported by kbot.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 9+ messages in thread