Re: [PATCH v3 1/3] PCI: dw-rockchip: Move the INTx irq setup to probe

[Niklas Cassel <cassel@kernel.org>]

On Tue, Sep 22, 2026 at 02:51:39AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] pcie: missing dw_pcie_host_deinit() on error paths causes UAF
> - [High] pcie: late INTx IRQ domain registration breaks downstream INTx interrupts
> --
> 
> commit af2f81da80dcb1ab2a4b7035c602bf3d2a40f1c9
> Author: Shawn Lin <shawn.lin@rock-chips.com>
> 
> PCI: dw-rockchip: Move the INTx irq setup to probe
> 
> The commit moves INTx IRQ domain setup out of the host ops .init() callback
> and into rockchip_pcie_configure_rc() to avoid leaking the domain upon
> controller reset. It performs the IRQ initialization immediately after
> dw_pcie_host_init() completes.
> 
> > diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > index af26a07c0c9e5..59d9a1b095df6 100644
> > --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> > +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> 
> [ ... ]
> 
> > @@ -739,6 +725,24 @@ static int rockchip_pcie_configure_rc(struct platform_device *pdev,
> >  		return ret;
> >  	}
> >  
> > +	/*
> > +	 * This is done here instead of in the host ops .init() callback,
> > +	 * which is also re-run by .reset_root_port(), so that the INTx irq
> > +	 * domain is only created once, at probe time.
> > +	 */
> > +	irq = of_irq_get_byname(dev->of_node, "legacy");
> 
> [Severity: High]
> Does moving the INTx IRQ domain creation here cause downstream devices to lose
> their INTx interrupts?
> 
> Since rockchip_pcie_configure_rc() calls dw_pcie_host_init() right before this,
> which internally invokes pci_host_probe() to enumerate the PCIe bus, downstream
> devices will be probed before this new code is reached.  When those devices
> attempt to map their legacy INTx interrupts via the device tree, they won't
> find the IRQ domain since it hasn't been created yet, which seems like it would
> cause the mapping to silently fail.
> 
> This architectural issue appears to remain unaddressed at the end of the patch
> series.

This appears like a legit concern.

I was the one who recommended that we call the of_irq_get_byname(), from
looking at e.g. pcie-qcom.c.

But I incorrectly said we should do it after dw_pcie_host_init().

Looking at pcie-qcom.c platform_get_irq_byname_optional() call was before
dw_pcie_host_init(). But if we look at e.g. pcie-tegra194.c, the
platform_get_irq_byname() calls are before the tegra_pcie_config_rp() call,
which calls tegra_pcie_init_controller(), which calls dw_pcie_host_init().

So I think Sashiko is right, of_irq_get_byname() (and the IRQ domain creation)
probably has to be done before calling dw_pcie_host_init().


If we do that, the other problem reported by Sashiko will probably also be
resolved, since you will return error before calling dw_pcie_host_init().


Kind regards,
Niklas