Re: [PATCH v5 2/4] PCI: rzg3s-host: Use shared reset controls for power domain resets

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The PCIe reference clock (`port->refclk`) is permanently leaked (enable count imbalance) if `pci_host_probe()` fails during driver probe.
- [High] Hardware access violation in `rzg3s_pcie_suspend_noirq()` causes system crash by writing to unclocked AXI registers.
- [High] Missing chained IRQ handler cleanup on probe error paths leads to Use-After-Free.
--

commit f0476ca1c132af88924c762e84517c32e45ebd21
Author: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

PCI: rzg3s-host: Use shared reset controls for power domain resets

This commit switches the PCIe power resets to use shared reset controls
to prepare for RZ/V2H(P) support. On this platform, multiple PCIe
controllers share the same reset line.

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 66f687304c1cb..cca2617f29157 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1277,9 +1277,9 @@ static int rzg3s_pcie_resets_prepare_and_get(struct rzg3s_pcie_host *host)
>  	for (i = 0; i < data->num_cfg_resets; i++)
>  		host->cfg_resets[i].id = data->cfg_resets[i];
>  
> -	ret = devm_reset_control_bulk_get_exclusive(host->dev,
> -						    data->num_power_resets,
> -						    host->power_resets);
> +	ret = devm_reset_control_bulk_get_shared(host->dev,
> +						 data->num_power_resets,
> +						 host->power_resets);

[Severity: High]
This isn't a bug introduced by this patch, but looking at rzg3s_pcie_probe(),
is it possible we are leaking the reference clock enable count on probe failure?

In rzg3s_pcie_host_init_port(), the clock is prepared and enabled:

	ret = clk_prepare_enable(port->refclk);
	if (ret)
		return dev_err_probe(dev, ret, "Failed to enable refclk!\n");

If pci_host_probe() fails later in the probe sequence, the cleanup path
drops the reference without disabling it:

port_refclk_put:
	clk_put(host->port.refclk);

Could this keep the reference clock and its parent PLLs permanently enabled,
preventing the SoC from reaching deep sleep states?


[Severity: High]
This is a pre-existing issue, but does rzg3s_pcie_suspend_noirq() have a
hardware access violation?

The runtime PM domain and clocks are gated by pm_runtime_put_sync() before
invoking the de-initialization callback:

	ret = pm_runtime_put_sync(dev);
	if (ret)
		return ret;

	clk_disable_unprepare(port->refclk);

	/* SoC-specific de-initialization */
	ret = data->config_deinit(host);

When rzg3e_pcie_config_deinit() is called, it accesses the controller's
AXI register space:

static int rzg3e_pcie_config_deinit(struct rzg3s_pcie_host *host)
{
	writel_relaxed(0, host->axi + RZG3S_PCI_RESET);
	return 0;
}

Can writing to a memory-mapped peripheral register while its bus clock is
gated cause an immediate bus fault or system crash during suspend?


[Severity: High]
This isn't a bug introduced by this patch, but is there a use-after-free
risk in rzg3s_pcie_teardown_irqdomain() if a probe failure occurs?

During rzg3s_pcie_init_irqdomain(), chained handlers are registered and a
pointer to the host context is installed into the underlying interrupt:

		irq_set_chained_handler_and_data(irq,
						 rzg3s_pcie_intx_irq_handler,
						 host);

If the probe fails downstream, rzg3s_pcie_teardown_irqdomain() removes the
domain but doesn't clear the chained handlers:

static void rzg3s_pcie_teardown_irqdomain(struct rzg3s_pcie_host *host)
{
	if (IS_ENABLED(CONFIG_PCI_MSI))
		rzg3s_pcie_teardown_msi(host);

	irq_domain_remove(host->intx_domain);
}

Since the host structure is allocated with devm_pci_alloc_host_bridge(),
it gets freed when probe returns an error. Would an interrupt firing after
this point lead to a kernel panic by dereferencing the freed host pointer?

>  	if (ret)
>  		return ret;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629220932.861445-1-prabhakar.mahadev-lad.rj@bp.renesas.com?part=2