Re: [PATCH v5] PCI: rcar-gen4: Add missing PM ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pcie: deadlocks due to ioremap and request_firmware in NOIRQ PM phase
--

commit e6c055f1eedf6bd5c13e231da24f0c6a76a24ba2
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>

PCI: rcar-gen4: Add missing PM ops

The R-Car Gen4 PCIe controller needs to gracefully suspend and resume its
link across system suspend/resume cycles to support power domains shutting
down. This patch adds suspend_noirq and resume_noirq PM operations to tear
down the link on suspend and restart it on resume.

> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index 2e7a8d85c0060..fceecbd16355c 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c

[ ... ]

> @@ -1062,6 +1064,26 @@ static int rcar_gen4_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable
>  	return 0;
>  }
>  
> +static int rcar_gen4_pcie_suspend_noirq(struct device *dev)
> +{
> +	struct rcar_gen4_pcie *rcar = dev_get_drvdata(dev);
> +
> +	if (rcar->drvdata->mode != DW_PCIE_RC_TYPE)
> +		return 0;
> +
> +	return dw_pcie_suspend_noirq(&rcar->dw);
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does this code trigger a deadlock or kernel panic during the NOIRQ PM phase?

Setting pp->use_atu_msg to true causes dw_pcie_suspend_noirq() to eventually
call ioremap():

dw_pcie_suspend_noirq()
  dw_pcie_pme_turn_off()
    ioremap()

Since this runs during the suspend_noirq phase with local interrupts disabled
and the system frozen, ioremap() allocates memory with GFP_KERNEL and may
sleep, which is strictly prohibited in NOIRQ context.

> +}
> +
> +static int rcar_gen4_pcie_resume_noirq(struct device *dev)
> +{
> +	struct rcar_gen4_pcie *rcar = dev_get_drvdata(dev);
> +
> +	if (rcar->drvdata->mode != DW_PCIE_RC_TYPE)
> +		return 0;
> +
> +	return dw_pcie_resume_noirq(&rcar->dw);
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Will this result in a sleeping function called from invalid context warning?

During the resume_noirq phase, this calls into the firmware request API:

dw_pcie_resume_noirq()
  dw_pcie_start_link()
    rcar_gen4_pcie_ltssm_control()
      rcar_gen4_pcie_download_phy_firmware()
        request_firmware()

Similar to the suspend path, request_firmware() allocates memory using
GFP_KERNEL and may block. Executing this with interrupts disabled in the
NOIRQ phase is unsafe and can lead to a system panic.

> +}
> +
>  static struct rcar_gen4_pcie_drvdata drvdata_r8a779f0_pcie = {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004011851.866833-1-marek.vasut+renesas@mailbox.org?part=1