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

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

[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 is initialized in dw_pcie_host_init(). However, on
Qcom firmware managed platforms the MSI domain is allocated from a
custom .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() before registering the
MSI IRQ domain so direct callers from custom .msi_init() paths always
have a properly initialized lock. Keep the existing initialization in
dw_pcie_host_init(), since pp->lock is also used by legacy INTx handling
and may be taken from glue .init() callbacks even when MSI is disabled.

Signed-off-by: Yadu M G <yadu.mg@oss.qualcomm.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index c9517a348836..de316f103c73 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);
+
 	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");
-- 
2.43.0

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

[sashiko-bot]

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