On 9/22/25 11:14 AM, Geert Uytterhoeven wrote:
Hello Geert,
On Tue, 9 Sept 2025 at 18:27, Marek Vasut marek.vasut+renesas@mailbox.org wrote:
The pmsr_lock spinlock used to be necessary to synchronize access to the PMSR register, because that access could have been triggered from either config space access in rcar_pcie_config_access() or an exception handler rcar_pcie_aarch32_abort_handler().
The rcar_pcie_aarch32_abort_handler() case is no longer applicable since commit 6e36203bc14c ("PCI: rcar: Use PCI_SET_ERROR_RESPONSE after read which triggered an exception"), which performs more accurate, controlled invocation of the exception, and a fixup.
This leaves rcar_pcie_config_access() as the only call site from which rcar_pcie_wakeup() is called. The rcar_pcie_config_access() can only be called from the controller struct pci_ops .read and .write callbacks, and those are serialized in drivers/pci/access.c using raw spinlock 'pci_lock' . CONFIG_PCI_LOCKLESS_CONFIG is never set on this platform.
Since the 'pci_lock' is a raw spinlock , and the 'pmsr_lock' is not a raw spinlock, this constellation triggers 'BUG: Invalid wait context' with CONFIG_PROVE_RAW_LOCK_NESTING=y .
Remove the pmsr_lock to fix the locking.
Fixes: a115b1bd3af0 ("PCI: rcar: Add L1 link state fix into data abort hook") Reported-by: Duy Nguyen duy.nguyen.rh@renesas.com Reported-by: Thuan Nguyen thuan.nguyen-hong@banvien.com.vn Cc: stable@vger.kernel.org Signed-off-by: Marek Vasut marek.vasut+renesas@mailbox.org
Thanks for your patch!
Your reasoning above LGTM, so Reviewed-by: Geert Uytterhoeven geert+renesas@glider.be
My only worry is that PCI_LOCKLESS_CONFIG may be selected on non-x86 one day, breaking your assumptions. IMHO, the mechanism behind this config option, introduced in commit 714fe383d6c9bd95 ("PCI: Provide Kconfig option for lockless config space accessors") looks very fragile to me: it is intended to be selected by an architecture, if "all" low level PCI configuration space accessors use their own serialization or can operate completely lockless. Usually we use the safer, inverted approach (PCI_NOLOCKLESS_CONFIG), to be selected by all drivers that do not adhere to the assumption. But perhaps I am missing something, and this does not depend on individual PCIe host drivers?
Regardless, improving that is clearly out-of-scope for this patch...
I could send a follow up patch which would add build-time assertion that PCI_LOCKLESS_CONFIG must not be selected for this driver to work. Would that be an option ?