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

[Shawn Lin]

在 2026/04/14 星期二 14:52, Yadu M G 写道:
> A lockdep warning is observed during boot on a Qcom firmware managed
> platform:
>
>    INFO: trying to register non-static key.
>    The code is fine but needs lockdep annotation, or maybe
>    you didn't initialize this object before use?
>    turning off the locking correctness validator.
>    ...
>    Call trace:
>     register_lock_class+0x128/0x4d8
>     __lock_acquire+0x110/0x1db0
>     lock_acquire+0x278/0x3d8
>     _raw_spin_lock_irq+0x6c/0xc0
>     dw_pcie_irq_domain_alloc+0x48/0x190
>     irq_domain_alloc_irqs_parent+0x2c/0x48
>     msi_domain_alloc+0x90/0x160
>    ...
>
> dw_pcie_irq_domain_alloc() takes pp->lock while allocating MSI
> interrupts. pp->lock was initialized in dw_pcie_host_init(). However,
> some DWC glue drivers like those on Qcom firmware managed platforms
> allocate MSI domains from their .msi_init callback by calling
> dw_pcie_allocate_domains() directly, bypassing dw_pcie_host_init(). This
> leaves pp->lock without a lockdep key and triggers the warning when MSI
> vectors are later allocated.
>
> Initialize pp->lock in dw_pcie_allocate_domains() immediately before
> registering the MSI IRQ domain so the lock is always initialized for
> every caller path.

If pci_msi_enabled()returns false in dw_pcie_host_init()(e.g., via the
pci=nomsi), the call to dw_pcie_allocate_domains() is skipped entirely,
leaving pp‑>lock uninitialized. Since the same lock is also used to
protect legacy INTx interrupts in some glue drivers (pcie‑sophgo.c or
pcie‑uniphier.c), could those drivers attempt to acquire an
uninitialized lock when unmasking or handling INTx interrupts? If so, we
would need to ensure the lock is initialized regardless of the MSI
enable state.

Moreover, sophgo_pcie_host_init() calls sophgo_pcie_msi_enable() which
need to acquires pp->lock with a uninitialized lock with your patch now.
That could possible break all other variant drivers which need this lock
in the .init() callback.

> Signed-off-by: Yadu M G <yadu.mg@xxxxxxxxxxxxxxxx>
> ---
>   drivers/pci/controller/dwc/pcie-designware-host.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
> index c9517a348836..8acd671bbc8c 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -215,6 +215,14 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
>   		.host_data	= pp,
>   	};
>   
> +	/*
> +	 * Initialize the lock here rather than in dw_pcie_host_init() so that
> +	 * drivers using a custom msi_init() callback that call
> +	 * dw_pcie_allocate_domains() directly also have the lock properly
> +	 * initialized before dw_pcie_irq_domain_alloc() can be invoked.
> +	 */
> +	raw_spin_lock_init(&pp->lock);
> +
>   	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");
> @@ -573,8 +581,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   	struct pci_host_bridge *bridge;
>   	int ret;
>   
> -	raw_spin_lock_init(&pp->lock);
> -
>   	bridge = devm_pci_alloc_host_bridge(dev, 0);
>   	if (!bridge)
>   		return -ENOMEM;