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

[sashiko-bot@kernel.org]

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