[PATCH 1/2] PCI: rcar-host: Drop PMSR spinlock

[PATCH 1/2] PCI: rcar-host: Drop PMSR spinlock

[Marek Vasut]

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>
---
=============================
[ BUG: Invalid wait context ]
6.17.0-rc4-next-20250905-00048-ga08e553145e7-dirty #1116 Not tainted
-----------------------------
swapper/0/1 is trying to lock:
ffffffd92cf69c30 (pmsr_lock){....}-{3:3}, at: rcar_pcie_config_access+0x48/0x260
other info that might help us debug this:
context-{5:5}
3 locks held by swapper/0/1:
 #0: ffffff84c0f890f8 (&dev->mutex){....}-{4:4}, at: device_lock+0x14/0x1c
 #1: ffffffd92cf675b0 (pci_rescan_remove_lock){+.+.}-{4:4}, at: pci_lock_rescan_remove+0x18/0x20
 #2: ffffffd92cf674a0 (pci_lock){....}-{2:2}, at: pci_bus_read_config_dword+0x54/0xd8
stack backtrace:
CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.17.0-rc4-next-20250905-00048-ga08e553145e7-dirty #1116 PREEMPT
Hardware name: Renesas Salvator-X 2nd version board based on r8a77951 (DT)
Call trace:
 dump_backtrace+0x6c/0x7c (C)
 show_stack+0x14/0x1c
 dump_stack_lvl+0x68/0x8c
 dump_stack+0x14/0x1c
 __lock_acquire+0x3e8/0x1064
 lock_acquire+0x17c/0x2ac
 _raw_spin_lock_irqsave+0x54/0x70
 rcar_pcie_config_access+0x48/0x260
 rcar_pcie_read_conf+0x44/0xd8
 pci_bus_read_config_dword+0x78/0xd8
 pci_bus_generic_read_dev_vendor_id+0x30/0x138
 pci_bus_read_dev_vendor_id+0x60/0x68
 pci_scan_single_device+0x11c/0x1ec
 pci_scan_slot+0x7c/0x170
 pci_scan_child_bus_extend+0x5c/0x29c
 pci_scan_child_bus+0x10/0x18
 pci_scan_root_bus_bridge+0x90/0xc8
 pci_host_probe+0x24/0xc4
 rcar_pcie_probe+0x5e8/0x650
 platform_probe+0x58/0x88
 really_probe+0x190/0x350
 __driver_probe_device+0x120/0x138
 driver_probe_device+0x38/0xec
 __driver_attach+0x158/0x168
 bus_for_each_dev+0x7c/0xd0
 driver_attach+0x20/0x28
 bus_add_driver+0xe0/0x1d8
 driver_register+0xac/0xe8
 __platform_driver_register+0x1c/0x24
 rcar_pcie_driver_init+0x18/0x20
 do_one_initcall+0xd4/0x220
 kernel_init_freeable+0x308/0x30c
 kernel_init+0x20/0x11c
 ret_from_fork+0x10/0x20
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/pci/controller/pcie-rcar-host.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
index 4780e0109e583..625a00f3b2230 100644
--- a/drivers/pci/controller/pcie-rcar-host.c
+++ b/drivers/pci/controller/pcie-rcar-host.c
@@ -52,20 +52,13 @@ struct rcar_pcie_host {
 	int			(*phy_init_fn)(struct rcar_pcie_host *host);
 };
 
-static DEFINE_SPINLOCK(pmsr_lock);
-
 static int rcar_pcie_wakeup(struct device *pcie_dev, void __iomem *pcie_base)
 {
-	unsigned long flags;
 	u32 pmsr, val;
 	int ret = 0;
 
-	spin_lock_irqsave(&pmsr_lock, flags);
-
-	if (!pcie_base || pm_runtime_suspended(pcie_dev)) {
-		ret = -EINVAL;
-		goto unlock_exit;
-	}
+	if (!pcie_base || pm_runtime_suspended(pcie_dev))
+		return -EINVAL;
 
 	pmsr = readl(pcie_base + PMSR);
 
@@ -87,8 +80,6 @@ static int rcar_pcie_wakeup(struct device *pcie_dev, void __iomem *pcie_base)
 		writel(L1FAEG | PMEL1RX, pcie_base + PMSR);
 	}
 
-unlock_exit:
-	spin_unlock_irqrestore(&pmsr_lock, flags);
 	return ret;
 }
 
-- 
2.51.0

[PATCH 2/2] PCI: rcar-host: Convert struct rcar_msi mask_lock into raw spinlock

[Marek Vasut]

The rcar_msi_irq_unmask() function may be called from a PCI driver
request_threaded_irq() function. This triggers kernel/irq/manage.c
__setup_irq() which locks raw spinlock &desc->lock descriptor lock
and with that descriptor lock held, calls rcar_msi_irq_unmask().

Since the &desc->lock descriptor lock is a raw spinlock , and the
rcar_msi .mask_lock is not a raw spinlock, this setup triggers
'BUG: Invalid wait context' with CONFIG_PROVE_RAW_LOCK_NESTING=y .

Use scoped_guard() to simplify the locking.

Fixes: 83ed8d4fa656 ("PCI: rcar: Convert to MSI domains")
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>
---
=============================
[ BUG: Invalid wait context ]
6.17.0-rc4-next-20250905-00049-g13b74d3367a3-dirty #1117 Not tainted
-----------------------------
swapper/0/1 is trying to lock:
ffffff84c1974e58 (&msi->mask_lock){....}-{3:3}, at: rcar_msi_irq_unmask+0x28/0x70
other info that might help us debug this:
context-{5:5}
6 locks held by swapper/0/1:
 #0: ffffff84c0e0d0f8 (&dev->mutex){....}-{4:4}, at: device_lock+0x14/0x1c
 #1: ffffffc0817675b0 (pci_rescan_remove_lock){+.+.}-{4:4}, at: pci_lock_rescan_remove+0x18/0x20
 #2: ffffff84c2a691b0 (&dev->mutex){....}-{4:4}, at: device_lock+0x14/0x1c
 #3: ffffff84c1976108 (&dev->mutex){....}-{4:4}, at: device_lock+0x14/0x1c
 #4: ffffff84c2a71640 (&desc->request_mutex){+.+.}-{4:4}, at: __setup_irq+0xa4/0x5bc
 #5: ffffff84c2a714c8 (&irq_desc_lock_class){....}-{2:2}, at: __setup_irq+0x230/0x5bc
stack backtrace:
CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.17.0-rc4-next-20250905-00049-g13b74d3367a3-dirty #1117 PREEMPT
Hardware name: Renesas Salvator-X 2nd version board based on r8a77951 (DT)
Call trace:
 dump_backtrace+0x6c/0x7c (C)
 show_stack+0x14/0x1c
 dump_stack_lvl+0x68/0x8c
 dump_stack+0x14/0x1c
 __lock_acquire+0x3e8/0x1064
 lock_acquire+0x17c/0x2ac
 _raw_spin_lock_irqsave+0x54/0x70
 rcar_msi_irq_unmask+0x28/0x70
 irq_chip_unmask_parent+0x18/0x20
 cond_startup_parent+0x40/0x44
 pci_irq_startup_msi+0x20/0x58
 __irq_startup+0x34/0x84
 irq_startup+0x64/0x114
 __setup_irq+0x3f8/0x5bc
 request_threaded_irq+0x11c/0x148
 pcie_pme_probe+0xec/0x190
 pcie_port_probe_service+0x34/0x5c
 really_probe+0x190/0x350
 __driver_probe_device+0x120/0x138
 driver_probe_device+0x38/0xec
 __device_attach_driver+0x100/0x114
 bus_for_each_drv+0xa8/0xd0
 __device_attach+0xdc/0x15c
 device_initial_probe+0x10/0x18
 bus_probe_device+0x38/0xa0
 device_add+0x554/0x68c
 device_register+0x1c/0x28
 pcie_portdrv_probe+0x480/0x518
 pci_device_probe+0xcc/0x13c
 really_probe+0x190/0x350
 __driver_probe_device+0x120/0x138
 driver_probe_device+0x38/0xec
 __device_attach_driver+0x100/0x114
 bus_for_each_drv+0xa8/0xd0
 __device_attach+0xdc/0x15c
 device_initial_probe+0x10/0x18
 pci_bus_add_device+0xb8/0x130
 pci_bus_add_devices+0x50/0x74
 pci_host_probe+0x90/0xc4
 rcar_pcie_probe+0x5e8/0x650
 platform_probe+0x58/0x88
 really_probe+0x190/0x350
 __driver_probe_device+0x120/0x138
 driver_probe_device+0x38/0xec
 __driver_attach+0x158/0x168
 bus_for_each_dev+0x7c/0xd0
 driver_attach+0x20/0x28
 bus_add_driver+0xe0/0x1d8
 driver_register+0xac/0xe8
 __platform_driver_register+0x1c/0x24
 rcar_pcie_driver_init+0x18/0x20
 do_one_initcall+0xd4/0x220
 kernel_init_freeable+0x308/0x30c
 kernel_init+0x20/0x11c
 ret_from_fork+0x10/0x20
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Magnus Damm <magnus.damm@gmail.com>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
 drivers/pci/controller/pcie-rcar-host.c | 27 ++++++++++++-------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/controller/pcie-rcar-host.c b/drivers/pci/controller/pcie-rcar-host.c
index 625a00f3b2230..213028052aa58 100644
--- a/drivers/pci/controller/pcie-rcar-host.c
+++ b/drivers/pci/controller/pcie-rcar-host.c
@@ -12,6 +12,7 @@
  */
 
 #include <linux/bitops.h>
+#include <linux/cleanup.h>
 #include <linux/clk.h>
 #include <linux/clk-provider.h>
 #include <linux/delay.h>
@@ -38,7 +39,7 @@ struct rcar_msi {
 	DECLARE_BITMAP(used, INT_PCI_MSI_NR);
 	struct irq_domain *domain;
 	struct mutex map_lock;
-	spinlock_t mask_lock;
+	raw_spinlock_t mask_lock;
 	int irq1;
 	int irq2;
 };
@@ -602,28 +603,26 @@ static void rcar_msi_irq_mask(struct irq_data *d)
 {
 	struct rcar_msi *msi = irq_data_get_irq_chip_data(d);
 	struct rcar_pcie *pcie = &msi_to_host(msi)->pcie;
-	unsigned long flags;
 	u32 value;
 
-	spin_lock_irqsave(&msi->mask_lock, flags);
-	value = rcar_pci_read_reg(pcie, PCIEMSIIER);
-	value &= ~BIT(d->hwirq);
-	rcar_pci_write_reg(pcie, value, PCIEMSIIER);
-	spin_unlock_irqrestore(&msi->mask_lock, flags);
+	scoped_guard(raw_spinlock_irqsave, &msi->mask_lock) {
+		value = rcar_pci_read_reg(pcie, PCIEMSIIER);
+		value &= ~BIT(d->hwirq);
+		rcar_pci_write_reg(pcie, value, PCIEMSIIER);
+	}
 }
 
 static void rcar_msi_irq_unmask(struct irq_data *d)
 {
 	struct rcar_msi *msi = irq_data_get_irq_chip_data(d);
 	struct rcar_pcie *pcie = &msi_to_host(msi)->pcie;
-	unsigned long flags;
 	u32 value;
 
-	spin_lock_irqsave(&msi->mask_lock, flags);
-	value = rcar_pci_read_reg(pcie, PCIEMSIIER);
-	value |= BIT(d->hwirq);
-	rcar_pci_write_reg(pcie, value, PCIEMSIIER);
-	spin_unlock_irqrestore(&msi->mask_lock, flags);
+	scoped_guard(raw_spinlock_irqsave, &msi->mask_lock) {
+		value = rcar_pci_read_reg(pcie, PCIEMSIIER);
+		value |= BIT(d->hwirq);
+		rcar_pci_write_reg(pcie, value, PCIEMSIIER);
+	}
 }
 
 static void rcar_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
@@ -736,7 +735,7 @@ static int rcar_pcie_enable_msi(struct rcar_pcie_host *host)
 	int err;
 
 	mutex_init(&msi->map_lock);
-	spin_lock_init(&msi->mask_lock);
+	raw_spin_lock_init(&msi->mask_lock);
 
 	err = of_address_to_resource(dev->of_node, 0, &res);
 	if (err)
-- 
2.51.0

Re: [PATCH 1/2] PCI: rcar-host: Drop PMSR spinlock

[Geert Uytterhoeven]

Hi Marek,

CC tglx

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...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] PCI: rcar-host: Convert struct rcar_msi mask_lock into raw spinlock

[Geert Uytterhoeven]

Hi Marek,

CC tegra

On Tue, 9 Sept 2025 at 18:27, Marek Vasut
<marek.vasut+renesas@mailbox.org> wrote:
> The rcar_msi_irq_unmask() function may be called from a PCI driver
> request_threaded_irq() function. This triggers kernel/irq/manage.c
> __setup_irq() which locks raw spinlock &desc->lock descriptor lock
> and with that descriptor lock held, calls rcar_msi_irq_unmask().
>
> Since the &desc->lock descriptor lock is a raw spinlock , and the
> rcar_msi .mask_lock is not a raw spinlock, this setup triggers
> 'BUG: Invalid wait context' with CONFIG_PROVE_RAW_LOCK_NESTING=y .
>
> Use scoped_guard() to simplify the locking.
>
> Fixes: 83ed8d4fa656 ("PCI: rcar: Convert to MSI domains")
> 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>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

drivers/pci/controller/pci-tegra.c seems to have the same issue.

> ---
> =============================
> [ BUG: Invalid wait context ]
> 6.17.0-rc4-next-20250905-00049-g13b74d3367a3-dirty #1117 Not tainted
> -----------------------------
> swapper/0/1 is trying to lock:
> ffffff84c1974e58 (&msi->mask_lock){....}-{3:3}, at: rcar_msi_irq_unmask+0x28/0x70
> other info that might help us debug this:
> context-{5:5}
> 6 locks held by swapper/0/1:
>  #0: ffffff84c0e0d0f8 (&dev->mutex){....}-{4:4}, at: device_lock+0x14/0x1c
>  #1: ffffffc0817675b0 (pci_rescan_remove_lock){+.+.}-{4:4}, at: pci_lock_rescan_remove+0x18/0x20
>  #2: ffffff84c2a691b0 (&dev->mutex){....}-{4:4}, at: device_lock+0x14/0x1c
>  #3: ffffff84c1976108 (&dev->mutex){....}-{4:4}, at: device_lock+0x14/0x1c
>  #4: ffffff84c2a71640 (&desc->request_mutex){+.+.}-{4:4}, at: __setup_irq+0xa4/0x5bc
>  #5: ffffff84c2a714c8 (&irq_desc_lock_class){....}-{2:2}, at: __setup_irq+0x230/0x5bc
> stack backtrace:
> CPU: 1 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.17.0-rc4-next-20250905-00049-g13b74d3367a3-dirty #1117 PREEMPT
> Hardware name: Renesas Salvator-X 2nd version board based on r8a77951 (DT)
> Call trace:
>  dump_backtrace+0x6c/0x7c (C)
>  show_stack+0x14/0x1c
>  dump_stack_lvl+0x68/0x8c
>  dump_stack+0x14/0x1c
>  __lock_acquire+0x3e8/0x1064
>  lock_acquire+0x17c/0x2ac
>  _raw_spin_lock_irqsave+0x54/0x70
>  rcar_msi_irq_unmask+0x28/0x70
>  irq_chip_unmask_parent+0x18/0x20
>  cond_startup_parent+0x40/0x44
>  pci_irq_startup_msi+0x20/0x58
>  __irq_startup+0x34/0x84
>  irq_startup+0x64/0x114
>  __setup_irq+0x3f8/0x5bc
>  request_threaded_irq+0x11c/0x148
>  pcie_pme_probe+0xec/0x190
>  pcie_port_probe_service+0x34/0x5c
>  really_probe+0x190/0x350
>  __driver_probe_device+0x120/0x138
>  driver_probe_device+0x38/0xec
>  __device_attach_driver+0x100/0x114
>  bus_for_each_drv+0xa8/0xd0
>  __device_attach+0xdc/0x15c
>  device_initial_probe+0x10/0x18
>  bus_probe_device+0x38/0xa0
>  device_add+0x554/0x68c
>  device_register+0x1c/0x28
>  pcie_portdrv_probe+0x480/0x518
>  pci_device_probe+0xcc/0x13c
>  really_probe+0x190/0x350
>  __driver_probe_device+0x120/0x138
>  driver_probe_device+0x38/0xec
>  __device_attach_driver+0x100/0x114
>  bus_for_each_drv+0xa8/0xd0
>  __device_attach+0xdc/0x15c
>  device_initial_probe+0x10/0x18
>  pci_bus_add_device+0xb8/0x130
>  pci_bus_add_devices+0x50/0x74
>  pci_host_probe+0x90/0xc4
>  rcar_pcie_probe+0x5e8/0x650

> --- a/drivers/pci/controller/pcie-rcar-host.c
> +++ b/drivers/pci/controller/pcie-rcar-host.c
> @@ -12,6 +12,7 @@
>   */
>
>  #include <linux/bitops.h>
> +#include <linux/cleanup.h>
>  #include <linux/clk.h>
>  #include <linux/clk-provider.h>
>  #include <linux/delay.h>
> @@ -38,7 +39,7 @@ struct rcar_msi {
>         DECLARE_BITMAP(used, INT_PCI_MSI_NR);
>         struct irq_domain *domain;
>         struct mutex map_lock;
> -       spinlock_t mask_lock;
> +       raw_spinlock_t mask_lock;
>         int irq1;
>         int irq2;
>  };
> @@ -602,28 +603,26 @@ static void rcar_msi_irq_mask(struct irq_data *d)
>  {
>         struct rcar_msi *msi = irq_data_get_irq_chip_data(d);
>         struct rcar_pcie *pcie = &msi_to_host(msi)->pcie;
> -       unsigned long flags;
>         u32 value;
>
> -       spin_lock_irqsave(&msi->mask_lock, flags);
> -       value = rcar_pci_read_reg(pcie, PCIEMSIIER);
> -       value &= ~BIT(d->hwirq);
> -       rcar_pci_write_reg(pcie, value, PCIEMSIIER);
> -       spin_unlock_irqrestore(&msi->mask_lock, flags);
> +       scoped_guard(raw_spinlock_irqsave, &msi->mask_lock) {
> +               value = rcar_pci_read_reg(pcie, PCIEMSIIER);
> +               value &= ~BIT(d->hwirq);
> +               rcar_pci_write_reg(pcie, value, PCIEMSIIER);
> +       }
>  }
>
>  static void rcar_msi_irq_unmask(struct irq_data *d)
>  {
>         struct rcar_msi *msi = irq_data_get_irq_chip_data(d);
>         struct rcar_pcie *pcie = &msi_to_host(msi)->pcie;
> -       unsigned long flags;
>         u32 value;
>
> -       spin_lock_irqsave(&msi->mask_lock, flags);
> -       value = rcar_pci_read_reg(pcie, PCIEMSIIER);
> -       value |= BIT(d->hwirq);
> -       rcar_pci_write_reg(pcie, value, PCIEMSIIER);
> -       spin_unlock_irqrestore(&msi->mask_lock, flags);
> +       scoped_guard(raw_spinlock_irqsave, &msi->mask_lock) {
> +               value = rcar_pci_read_reg(pcie, PCIEMSIIER);
> +               value |= BIT(d->hwirq);
> +               rcar_pci_write_reg(pcie, value, PCIEMSIIER);
> +       }
>  }
>
>  static void rcar_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> @@ -736,7 +735,7 @@ static int rcar_pcie_enable_msi(struct rcar_pcie_host *host)
>         int err;
>
>         mutex_init(&msi->map_lock);
> -       spin_lock_init(&msi->mask_lock);
> +       raw_spin_lock_init(&msi->mask_lock);
>
>         err = of_address_to_resource(dev->of_node, 0, &res);
>         if (err)

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/2] PCI: rcar-host: Convert struct rcar_msi mask_lock into raw spinlock

[Marc Zyngier]

On Tue, 09 Sep 2025 17:26:25 +0100,
Marek Vasut <marek.vasut+renesas@mailbox.org> wrote:
> 
> The rcar_msi_irq_unmask() function may be called from a PCI driver
> request_threaded_irq() function. This triggers kernel/irq/manage.c
> __setup_irq() which locks raw spinlock &desc->lock descriptor lock
> and with that descriptor lock held, calls rcar_msi_irq_unmask().
> 
> Since the &desc->lock descriptor lock is a raw spinlock , and the
> rcar_msi .mask_lock is not a raw spinlock, this setup triggers
> 'BUG: Invalid wait context' with CONFIG_PROVE_RAW_LOCK_NESTING=y .
> 
> Use scoped_guard() to simplify the locking.
> 
> Fixes: 83ed8d4fa656 ("PCI: rcar: Convert to MSI domains")
> 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>

Acked-by: Marc Zyngier <maz@kernel.org>

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 1/2] PCI: rcar-host: Drop PMSR spinlock

[Marek Vasut]

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 ?

Re: [PATCH 1/2] PCI: rcar-host: Drop PMSR spinlock

[Geert Uytterhoeven]

Hi Marek,

On Mon, 22 Sept 2025 at 12:44, Marek Vasut <marek.vasut@mailbox.org> wrote:
> On 9/22/25 11:14 AM, Geert Uytterhoeven wrote:
> > 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 ?

Or simply just "depends on !CONFIG_PCI_LOCKLESS_CONFIG"?
What do the PCIe maintainers think?

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 1/2] PCI: rcar-host: Drop PMSR spinlock

[Marek Vasut]

On 9/22/25 12:53 PM, Geert Uytterhoeven wrote:

Hello Geert,

>>> 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 ?
> 
> Or simply just "depends on !CONFIG_PCI_LOCKLESS_CONFIG"?
> What do the PCIe maintainers think?
I send a patch in the meantime:

[PATCH] PCI: rcar-host: Add static assertion to check !PCI_LOCKLESS_CONFIG

Re: [PATCH 1/2] PCI: rcar-host: Drop PMSR spinlock

[Manivannan Sadhasivam]

On Tue, 09 Sep 2025 18:26:24 +0200, Marek Vasut 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.
> 
> [...]

Applied, thanks!

[1/2] PCI: rcar-host: Drop PMSR spinlock
      commit: 0a8f173d9dad13930d5888505dc4c4fd6a1d4262
[2/2] PCI: rcar-host: Convert struct rcar_msi mask_lock into raw spinlock
      commit: 945878aa8b574f66ead4ab1844185376c0d0add4

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>