Re: [PATCH v2] PCI: dwc: Initialize pp->lock before MSI domain registration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional double initialization of `pp->lock` in `dw_pcie_allocate_domains()` destroys lock state and corrupts lockdep tracking for standard platforms.
--

commit 34c5622cf23f408a8b7191136ff7caf71c5a0456
Author: Yadu M G <yadu.mg@oss.qualcomm.com>

PCI: dwc: Initialize pp->lock before MSI domain registration

This commit adds an initialization of pp->lock in dw_pcie_allocate_domains()
to fix a lockdep warning on Qcom platforms. These platforms allocate the MSI
domain from a custom .msi_init callback, bypassing the standard
dw_pcie_host_init() flow.

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index c9517a3488368..de316f103c73b 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -215,6 +215,13 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
>  		.host_data	= pp,
>  	};
>  
> +	/*
> +	 * Initialize pp->lock here to cover direct callers from custom
> +	 * .msi_init() paths (e.g. firmware-managed platforms) that
> +	 * bypass dw_pcie_host_init().
> +	 */
> +	raw_spin_lock_init(&pp->lock);
> +

Does this unconditionally re-initialize the lock for standard platforms?

In the standard DesignWare host initialization flow, dw_pcie_host_init()
initializes pp->lock at the beginning of the function. It then calls the
platform-specific pp->ops->init() callback, which on some platforms like
Uniphier configures and enables INTx interrupts.

If an INTx interrupt fires asynchronously and its handler legitimately
acquires pp->lock, would this second raw_spin_lock_init() call in
dw_pcie_allocate_domains() destroy the lock state while it is in use?

Re-initializing the spinlock forcibly resets its internal state to
unlocked and assigns a new lockdep key, which could introduce a data
race or corrupt lock dependency tracking for platforms using the
standard initialization flow.

>  	pp->irq_domain = msi_create_parent_irq_domain(&info, &dw_pcie_msi_parent_ops);
>  	if (!pp->irq_domain) {
>  		dev_err(pci->dev, "Failed to create IRQ domain\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260514181901.315647-1-yadu.mg@oss.qualcomm.com?part=1