* [ISSUE + PATCH] Interrupts were enabled early by spinlock guard
@ 2025-08-14 12:59 Edgar Bonet
2025-08-14 14:39 ` Geert Uytterhoeven
2025-08-25 12:33 ` Nicolas Ferre
0 siblings, 2 replies; 9+ messages in thread
From: Edgar Bonet @ 2025-08-14 12:59 UTC (permalink / raw)
To: Thomas Gleixner, Nicolas Ferre, Alexandre Belloni, Claudiu Beznea
Cc: linux-kernel, linux-arm-kernel, Geert Uytterhoeven
Hello everybody!
I am facing an "Interrupts were enabled early" kernel warning which, as
far as I can tell, is caused by a spinlock guard in an ARM/Microchip
IRQCHIP driver. I think I solved the issue, and I am proposing a patch
below, after the scissor. But I must first disclose that:
* I am completely new to Linux internals and its development process.
This is why I chose to err on the side of providing too much
information on my issue. It is not unlikely that I am doing
something very wrong.
* I am not subscribed to either of the linux-{,arm-}kernel mailing
lists
* I will be far from the Internet in the few days to come. I should be
connected an responsive starting from 2025-08-24.
## Context
I am playing with an Acmesystems Acqua[1] system on module, which is
based on a SAMA5D31 SoC (single core Cortex-A5). I maintain the
Buildroot defconfig for this board,[2] which is currently based on a
vanilla Linux kernel v6.12.41.
As I wanted to check that the board runs fine on newer kernels, I built
a v6.16 for it using gcc 14, binutils 2.43, the in-tree sama5_defconfig
merged with this fragment:
# CONFIG_BRIDGE is not set
# CONFIG_MODULES is not set
# CONFIG_NET_DSA is not set
# CONFIG_WIRELESS is not set
# CONFIG_USB_NET_DRIVERS is not set
# CONFIG_WLAN is not set
# CONFIG_MEDIA_SUPPORT is not set
# CONFIG_SOUND is not set
# CONFIG_AUTOFS_FS is not set
and the Buildroot-provided device tree, patched for compatibility with
Linux commit 510a6190cf5e ("ARM: dts: microchip: fix faulty ohci/ehci
node names").
The defconfig fragment above is meant to remove module support (with
some unused drivers along the way), which makes testing easier for me.
## Issue
While booting, Linux v6.16 printed this message on the serial console:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at init/main.c:1024 start_kernel+0x4d0/0x5dc
Interrupts were enabled early
CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.16.0 #1 NONE
Hardware name: Atmel SAMA5
Call trace:
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x38/0x48
dump_stack_lvl from __warn+0x78/0xd4
__warn from warn_slowpath_fmt+0x98/0xc8
warn_slowpath_fmt from start_kernel+0x4d0/0x5dc
start_kernel from 0x0
---[ end trace 0000000000000000 ]---
The board seemed to work fine, so maybe this warning is completely
harmless. It looked, however, scary enough to deserve some
investigation.
## Bug hunting
I could not reproduce the issue on a qemu virtual machine. All tests
were then done on the real hardware.
I looked for the Linux commit that introduced the issue. ‘git bisect’
told me this was 195298c3b116 ("genirq/generic-chip: Convert core code
to lock guards"). I then checked out a recent mainline kernel, namely
v6.17-rc1, and tried to revert that commit. For this, I first had to
revert two follow-up commits, namely 7ae844a6650c ("genirq/generic-chip:
Remove unused lock wrappers") and 771487050f83 ("genirq/generic-chip:
Fix incorrect lock guard conversions"). This was unsuccessful: I still
had the warning.
I went back to a clean v6.17-rc1 and tried to find the thing that was
enabling interrupts early. After lots of printk() debugging, I
discovered it was a guard(raw_spinlock_irq) destructor. The call chain
is this (line numbers are for v6.17-rc1):
start_kernel (init/main.c:1011)
-> time_init (arch/arm/kernel/time.c:96)
-> timer_probe (drivers/clocksource/timer-probe.c:30)
=> tcb_clksrc_init (drivers/clocksource/timer-atmel-tcb.c:413)
-> of_irq_get (drivers/of/irq.c:474)
-> irq_create_of_mapping (kernel/irq/irqdomain.c:980)
-> irq_create_fwspec_mapping (kernel/irq/irqdomain.c:848)
=> aic5_irq_domain_xlate (drivers/irqchip/irq-atmel-aic5.c:287)
-> guard destructor (raw_spin_unlock_irq?)
where (=>) is an indirect call through a function pointer.
## Tentative fix
Commit 771487050f83 gave me the inspiration. The guard in question was
introduced by b00bee8afaca ("irqchip: Convert generic irqchip locking to
guards"), which replaced calls to irq_gc_lock_irq{save,restore}() by
guard(raw_spinlock_irq) (with no “save” in the name). The commit log
states that this is “intended and correct”, but I could not make sense
of the explanation. My (possibly faulty) understanding is that the guard
constructor disables interrupts, and the destructor either
unconditionally enables them (raw_spinlock_irq), or restores the
previous interrupt state (raw_spinlock_irqsave).
I then replaced guard(raw_spinlock_irq) with guard(raw_spinlock_irqsave)
and that seemed to do the job: the warning is gone. See the patch below
the scissors.
Best regards, and thank-you for reading so far.
Edgar Bonet.
[1] https://www.acmesystems.it/acqua
[2] https://gitlab.com/buildroot.org/buildroot/-/blob/2025.08-rc1/configs/acmesystems_acqua_a5_512mb_defconfig
------------------------------------------------------------------ >8 --
Subject: [PATCH] irqchip/atmel-aic5: Fix incorrect lock guard conversion
Commit b00bee8afaca ("irqchip: Convert generic irqchip locking to guards")
replaced calls to irq_gc_lock_irq{save,restore}() with
guard(raw_spinlock_irq). However, in irq-atmel-aic5.c, one such guard is
created early in the boot process, before interrupts are initially enabled.
As its destructor enables interrupts, this results in the following warning
on a SAMA5D31-based system:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 0 at init/main.c:1024 start_kernel+0x4d0/0x5dc
Interrupts were enabled early
CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.16.0 #1 NONE
Hardware name: Atmel SAMA5
Call trace:
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x38/0x48
dump_stack_lvl from __warn+0x78/0xd4
__warn from warn_slowpath_fmt+0x98/0xc8
warn_slowpath_fmt from start_kernel+0x4d0/0x5dc
start_kernel from 0x0
---[ end trace 0000000000000000 ]---
Fix this by using guard(raw_spinlock_irqsave) instead.
Fixes: b00bee8afaca ("irqchip: Convert generic irqchip locking to guards")
Signed-off-by: Edgar Bonet <bonet@grenoble.cnrs.fr>
---
drivers/irqchip/irq-atmel-aic5.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
index 60b00d2c3d7a..1f14b401f71d 100644
--- a/drivers/irqchip/irq-atmel-aic5.c
+++ b/drivers/irqchip/irq-atmel-aic5.c
@@ -279,7 +279,7 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
if (ret)
return ret;
- guard(raw_spinlock_irq)(&bgc->lock);
+ guard(raw_spinlock_irqsave)(&bgc->lock);
irq_reg_writel(bgc, *out_hwirq, AT91_AIC5_SSR);
smr = irq_reg_readl(bgc, AT91_AIC5_SMR);
aic_common_set_priority(intspec[2], &smr);
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [ISSUE + PATCH] Interrupts were enabled early by spinlock guard
2025-08-14 12:59 [ISSUE + PATCH] Interrupts were enabled early by spinlock guard Edgar Bonet
@ 2025-08-14 14:39 ` Geert Uytterhoeven
2025-08-14 15:28 ` Edgar Bonet
2025-08-25 12:33 ` Nicolas Ferre
1 sibling, 1 reply; 9+ messages in thread
From: Geert Uytterhoeven @ 2025-08-14 14:39 UTC (permalink / raw)
To: Edgar Bonet
Cc: Thomas Gleixner, Nicolas Ferre, Alexandre Belloni, Claudiu Beznea,
linux-kernel, linux-arm-kernel, Huacai Chen, Jiaxun Yang
Hi Edgar,
CC loongson
On Thu, 14 Aug 2025 at 15:00, Edgar Bonet <bonet@grenoble.cnrs.fr> wrote:
> Subject: [PATCH] irqchip/atmel-aic5: Fix incorrect lock guard conversion
>
> Commit b00bee8afaca ("irqchip: Convert generic irqchip locking to guards")
> replaced calls to irq_gc_lock_irq{save,restore}() with
> guard(raw_spinlock_irq). However, in irq-atmel-aic5.c, one such guard is
> created early in the boot process, before interrupts are initially enabled.
> As its destructor enables interrupts, this results in the following warning
> on a SAMA5D31-based system:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at init/main.c:1024 start_kernel+0x4d0/0x5dc
> Interrupts were enabled early
> CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.16.0 #1 NONE
> Hardware name: Atmel SAMA5
> Call trace:
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x38/0x48
> dump_stack_lvl from __warn+0x78/0xd4
> __warn from warn_slowpath_fmt+0x98/0xc8
> warn_slowpath_fmt from start_kernel+0x4d0/0x5dc
> start_kernel from 0x0
> ---[ end trace 0000000000000000 ]---
>
> Fix this by using guard(raw_spinlock_irqsave) instead.
>
> Fixes: b00bee8afaca ("irqchip: Convert generic irqchip locking to guards")
> Signed-off-by: Edgar Bonet <bonet@grenoble.cnrs.fr>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> --- a/drivers/irqchip/irq-atmel-aic5.c
> +++ b/drivers/irqchip/irq-atmel-aic5.c
> @@ -279,7 +279,7 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
> if (ret)
> return ret;
>
> - guard(raw_spinlock_irq)(&bgc->lock);
> + guard(raw_spinlock_irqsave)(&bgc->lock);
> irq_reg_writel(bgc, *out_hwirq, AT91_AIC5_SSR);
> smr = irq_reg_readl(bgc, AT91_AIC5_SMR);
> aic_common_set_priority(intspec[2], &smr);
I think the conversions in
drivers/irqchip/irq-atmel-aic.c:aic_irq_domain_xlate() and
drivers/irqchip/irq-loongson-liointc.c:liointc_set_type()
are also wrong, and need a similar change.
Unfortunately I have no hardware to verify.
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [ISSUE + PATCH] Interrupts were enabled early by spinlock guard
2025-08-14 14:39 ` Geert Uytterhoeven
@ 2025-08-14 15:28 ` Edgar Bonet
2025-08-23 19:33 ` Thomas Gleixner
2025-08-25 12:35 ` Nicolas Ferre
0 siblings, 2 replies; 9+ messages in thread
From: Edgar Bonet @ 2025-08-14 15:28 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Thomas Gleixner, Nicolas Ferre, Alexandre Belloni, Claudiu Beznea,
linux-kernel, linux-arm-kernel, Huacai Chen, Jiaxun Yang
Hello Geert, and thanks for you prompt review!
> I think the conversions in
> drivers/irqchip/irq-atmel-aic.c:aic_irq_domain_xlate() and
> drivers/irqchip/irq-loongson-liointc.c:liointc_set_type()
> are also wrong, and need a similar change.
The one in irq-atmel-aic.c looks indeed strikingly similar. The one in
irq-loongson-liointc.c is slightly different though. Instead of:
irq_gc_lock_irqsave() -> guard(raw_spinlock_irq)
it does:
irq_gc_lock_irqsave() -> guard(raw_spinlock)
I don't know what the implications are though.
> Unfortunately I have no hardware to verify.
Neither do I.
Regards,
Edgar.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [ISSUE + PATCH] Interrupts were enabled early by spinlock guard
2025-08-14 15:28 ` Edgar Bonet
@ 2025-08-23 19:33 ` Thomas Gleixner
2025-08-23 19:35 ` Thomas Gleixner
2025-08-25 12:35 ` Nicolas Ferre
1 sibling, 1 reply; 9+ messages in thread
From: Thomas Gleixner @ 2025-08-23 19:33 UTC (permalink / raw)
To: Edgar Bonet, Geert Uytterhoeven
Cc: Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, linux-kernel,
linux-arm-kernel, Huacai Chen, Jiaxun Yang
On Thu, Aug 14 2025 at 17:28, Edgar Bonet wrote:
>> I think the conversions in
>> drivers/irqchip/irq-atmel-aic.c:aic_irq_domain_xlate() and
>> drivers/irqchip/irq-loongson-liointc.c:liointc_set_type()
>> are also wrong, and need a similar change.
> The one in irq-atmel-aic.c looks indeed strikingly similar.
Yes. My bad.
I missed the fact, that this can be invoked during early boot when
interrupts are still disabled. After early boot they are always enabled
when xlate() is invoked.
> The one in irq-loongson-liointc.c is slightly different
> though. Instead of:
>
> irq_gc_lock_irqsave() -> guard(raw_spinlock_irq)
>
> it does:
>
> irq_gc_lock_irqsave() -> guard(raw_spinlock)
>
> I don't know what the implications are though.
That's in the set_type() callback which is always invoked with the
interrupt decriptor lock held and interrupts disabled, so doing the
'save/restore' dance there is pointless.
Can you send a patch for that atmel-aic thing too please?
Thanks,
tglx
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [ISSUE + PATCH] Interrupts were enabled early by spinlock guard
2025-08-23 19:33 ` Thomas Gleixner
@ 2025-08-23 19:35 ` Thomas Gleixner
0 siblings, 0 replies; 9+ messages in thread
From: Thomas Gleixner @ 2025-08-23 19:35 UTC (permalink / raw)
To: Edgar Bonet, Geert Uytterhoeven
Cc: Nicolas Ferre, Alexandre Belloni, Claudiu Beznea, linux-kernel,
linux-arm-kernel, Huacai Chen, Jiaxun Yang
On Sat, Aug 23 2025 at 21:33, Thomas Gleixner wrote:
> On Thu, Aug 14 2025 at 17:28, Edgar Bonet wrote:
>>> I think the conversions in
>>> drivers/irqchip/irq-atmel-aic.c:aic_irq_domain_xlate() and
>>> drivers/irqchip/irq-loongson-liointc.c:liointc_set_type()
>>> are also wrong, and need a similar change.
>
>> The one in irq-atmel-aic.c looks indeed strikingly similar.
>
> Yes. My bad.
>
> I missed the fact, that this can be invoked during early boot when
> interrupts are still disabled. After early boot they are always enabled
> when xlate() is invoked.
>
>> The one in irq-loongson-liointc.c is slightly different
>> though. Instead of:
>>
>> irq_gc_lock_irqsave() -> guard(raw_spinlock_irq)
>>
>> it does:
>>
>> irq_gc_lock_irqsave() -> guard(raw_spinlock)
>>
>> I don't know what the implications are though.
>
> That's in the set_type() callback which is always invoked with the
> interrupt decriptor lock held and interrupts disabled, so doing the
> 'save/restore' dance there is pointless.
>
> Can you send a patch for that atmel-aic thing too please?
Nah. Let me just fold it into your patch and be done with it.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [ISSUE + PATCH] Interrupts were enabled early by spinlock guard
2025-08-14 12:59 [ISSUE + PATCH] Interrupts were enabled early by spinlock guard Edgar Bonet
2025-08-14 14:39 ` Geert Uytterhoeven
@ 2025-08-25 12:33 ` Nicolas Ferre
2025-08-25 14:12 ` Nicolas Ferre
1 sibling, 1 reply; 9+ messages in thread
From: Nicolas Ferre @ 2025-08-25 12:33 UTC (permalink / raw)
To: Edgar Bonet, Thomas Gleixner, Alexandre Belloni, Claudiu Beznea
Cc: linux-kernel, linux-arm-kernel, Geert Uytterhoeven
On 14/08/2025 at 14:59, Edgar Bonet wrote:
> Hello everybody!
>
> I am facing an "Interrupts were enabled early" kernel warning which, as
> far as I can tell, is caused by a spinlock guard in an ARM/Microchip
> IRQCHIP driver. I think I solved the issue, and I am proposing a patch
> below, after the scissor. But I must first disclose that:
>
> * I am completely new to Linux internals and its development process.
> This is why I chose to err on the side of providing too much
> information on my issue. It is not unlikely that I am doing
> something very wrong.
>
> * I am not subscribed to either of the linux-{,arm-}kernel mailing
> lists
>
> * I will be far from the Internet in the few days to come. I should be
> connected an responsive starting from 2025-08-24.
Edgar,
Thanks a lot for having investigated this.
> ## Context
>
> I am playing with an Acmesystems Acqua[1] system on module, which is
> based on a SAMA5D31 SoC (single core Cortex-A5). I maintain the
> Buildroot defconfig for this board,[2] which is currently based on a
> vanilla Linux kernel v6.12.41.
>
> As I wanted to check that the board runs fine on newer kernels, I built
> a v6.16 for it using gcc 14, binutils 2.43, the in-tree sama5_defconfig
> merged with this fragment:
>
> # CONFIG_BRIDGE is not set
> # CONFIG_MODULES is not set
> # CONFIG_NET_DSA is not set
> # CONFIG_WIRELESS is not set
> # CONFIG_USB_NET_DRIVERS is not set
> # CONFIG_WLAN is not set
> # CONFIG_MEDIA_SUPPORT is not set
> # CONFIG_SOUND is not set
> # CONFIG_AUTOFS_FS is not set
>
> and the Buildroot-provided device tree, patched for compatibility with
> Linux commit 510a6190cf5e ("ARM: dts: microchip: fix faulty ohci/ehci
> node names").
>
> The defconfig fragment above is meant to remove module support (with
> some unused drivers along the way), which makes testing easier for me.
>
>
> ## Issue
>
> While booting, Linux v6.16 printed this message on the serial console:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at init/main.c:1024 start_kernel+0x4d0/0x5dc
> Interrupts were enabled early
> CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.16.0 #1 NONE
> Hardware name: Atmel SAMA5
> Call trace:
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x38/0x48
> dump_stack_lvl from __warn+0x78/0xd4
> __warn from warn_slowpath_fmt+0x98/0xc8
> warn_slowpath_fmt from start_kernel+0x4d0/0x5dc
> start_kernel from 0x0
> ---[ end trace 0000000000000000 ]---
Indeed, reproduced on sam9x75 (which has AIC5), on 6.17.0-rc3.
> The board seemed to work fine, so maybe this warning is completely
> harmless. It looked, however, scary enough to deserve some
> investigation.
>
>
> ## Bug hunting
>
> I could not reproduce the issue on a qemu virtual machine. All tests
> were then done on the real hardware.
>
> I looked for the Linux commit that introduced the issue. ‘git bisect’
> told me this was 195298c3b116 ("genirq/generic-chip: Convert core code
> to lock guards"). I then checked out a recent mainline kernel, namely
> v6.17-rc1, and tried to revert that commit. For this, I first had to
> revert two follow-up commits, namely 7ae844a6650c ("genirq/generic-chip:
> Remove unused lock wrappers") and 771487050f83 ("genirq/generic-chip:
> Fix incorrect lock guard conversions"). This was unsuccessful: I still
> had the warning.
>
> I went back to a clean v6.17-rc1 and tried to find the thing that was
> enabling interrupts early. After lots of printk() debugging, I
> discovered it was a guard(raw_spinlock_irq) destructor. The call chain
> is this (line numbers are for v6.17-rc1):
>
> start_kernel (init/main.c:1011)
> -> time_init (arch/arm/kernel/time.c:96)
> -> timer_probe (drivers/clocksource/timer-probe.c:30)
> => tcb_clksrc_init (drivers/clocksource/timer-atmel-tcb.c:413)
> -> of_irq_get (drivers/of/irq.c:474)
> -> irq_create_of_mapping (kernel/irq/irqdomain.c:980)
> -> irq_create_fwspec_mapping (kernel/irq/irqdomain.c:848)
> => aic5_irq_domain_xlate (drivers/irqchip/irq-atmel-aic5.c:287)
> -> guard destructor (raw_spin_unlock_irq?)
>
> where (=>) is an indirect call through a function pointer.
>
>
> ## Tentative fix
>
> Commit 771487050f83 gave me the inspiration. The guard in question was
> introduced by b00bee8afaca ("irqchip: Convert generic irqchip locking to
> guards"), which replaced calls to irq_gc_lock_irq{save,restore}() by
> guard(raw_spinlock_irq) (with no “save” in the name). The commit log
> states that this is “intended and correct”, but I could not make sense
> of the explanation. My (possibly faulty) understanding is that the guard
> constructor disables interrupts, and the destructor either
> unconditionally enables them (raw_spinlock_irq), or restores the
> previous interrupt state (raw_spinlock_irqsave).
>
> I then replaced guard(raw_spinlock_irq) with guard(raw_spinlock_irqsave)
> and that seemed to do the job: the warning is gone. See the patch below
> the scissors.
>
> Best regards, and thank-you for reading so far.
>
> Edgar Bonet.
>
> [1] https://www.acmesystems.it/acqua
> [2] https://gitlab.com/buildroot.org/buildroot/-/blob/2025.08-rc1/configs/acmesystems_acqua_a5_512mb_defconfig
>
> ------------------------------------------------------------------ >8 --
> Subject: [PATCH] irqchip/atmel-aic5: Fix incorrect lock guard conversion
>
> Commit b00bee8afaca ("irqchip: Convert generic irqchip locking to guards")
> replaced calls to irq_gc_lock_irq{save,restore}() with
> guard(raw_spinlock_irq). However, in irq-atmel-aic5.c, one such guard is
> created early in the boot process, before interrupts are initially enabled.
> As its destructor enables interrupts, this results in the following warning
> on a SAMA5D31-based system:
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at init/main.c:1024 start_kernel+0x4d0/0x5dc
> Interrupts were enabled early
> CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.16.0 #1 NONE
> Hardware name: Atmel SAMA5
> Call trace:
> unwind_backtrace from show_stack+0x10/0x14
> show_stack from dump_stack_lvl+0x38/0x48
> dump_stack_lvl from __warn+0x78/0xd4
> __warn from warn_slowpath_fmt+0x98/0xc8
> warn_slowpath_fmt from start_kernel+0x4d0/0x5dc
> start_kernel from 0x0
> ---[ end trace 0000000000000000 ]---
>
> Fix this by using guard(raw_spinlock_irqsave) instead.
>
> Fixes: b00bee8afaca ("irqchip: Convert generic irqchip locking to guards")
> Signed-off-by: Edgar Bonet <bonet@grenoble.cnrs.fr>
Indeed. Thanks for the detailed explanation and the patch:
Tested-by: Nicolas Ferre <nicolas.ferre@microchip.com> # on sam9x75
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> (if needed)
Best regards,
Nicolas
> ---
> drivers/irqchip/irq-atmel-aic5.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
> index 60b00d2c3d7a..1f14b401f71d 100644
> --- a/drivers/irqchip/irq-atmel-aic5.c
> +++ b/drivers/irqchip/irq-atmel-aic5.c
> @@ -279,7 +279,7 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
> if (ret)
> return ret;
>
> - guard(raw_spinlock_irq)(&bgc->lock);
> + guard(raw_spinlock_irqsave)(&bgc->lock);
> irq_reg_writel(bgc, *out_hwirq, AT91_AIC5_SSR);
> smr = irq_reg_readl(bgc, AT91_AIC5_SMR);
> aic_common_set_priority(intspec[2], &smr);
> --
> 2.43.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [ISSUE + PATCH] Interrupts were enabled early by spinlock guard
2025-08-14 15:28 ` Edgar Bonet
2025-08-23 19:33 ` Thomas Gleixner
@ 2025-08-25 12:35 ` Nicolas Ferre
2025-08-25 14:10 ` Nicolas Ferre
1 sibling, 1 reply; 9+ messages in thread
From: Nicolas Ferre @ 2025-08-25 12:35 UTC (permalink / raw)
To: Edgar Bonet, Geert Uytterhoeven, Thomas Gleixner
Cc: Alexandre Belloni, Claudiu Beznea, linux-kernel, linux-arm-kernel,
Huacai Chen, Jiaxun Yang
On 14/08/2025 at 17:28, Edgar Bonet wrote:
> Hello Geert, and thanks for you prompt review!
Yep, Geert, many thanks!
>> I think the conversions in
>> drivers/irqchip/irq-atmel-aic.c:aic_irq_domain_xlate() and
>> drivers/irqchip/irq-loongson-liointc.c:liointc_set_type()
>> are also wrong, and need a similar change.
>
> The one in irq-atmel-aic.c looks indeed strikingly similar. The one in
> irq-loongson-liointc.c is slightly different though. Instead of:
>
> irq_gc_lock_irqsave() -> guard(raw_spinlock_irq)
>
> it does:
>
> irq_gc_lock_irqsave() -> guard(raw_spinlock)
>
> I don't know what the implications are though.
>
>> Unfortunately I have no hardware to verify.
>
> Neither do I.
I'm on it. Expect feedback later today...
Thanks so much for the heads-up.
Best regards,
Nicolas
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [ISSUE + PATCH] Interrupts were enabled early by spinlock guard
2025-08-25 12:35 ` Nicolas Ferre
@ 2025-08-25 14:10 ` Nicolas Ferre
0 siblings, 0 replies; 9+ messages in thread
From: Nicolas Ferre @ 2025-08-25 14:10 UTC (permalink / raw)
To: Edgar Bonet, Geert Uytterhoeven, Thomas Gleixner
Cc: Alexandre Belloni, Claudiu Beznea, linux-kernel, linux-arm-kernel,
Huacai Chen, Jiaxun Yang
On 25/08/2025 at 14:35, Nicolas Ferre wrote:
> On 14/08/2025 at 17:28, Edgar Bonet wrote:
>> Hello Geert, and thanks for you prompt review!
>
> Yep, Geert, many thanks!
>
>>> I think the conversions in
>>> drivers/irqchip/irq-atmel-aic.c:aic_irq_domain_xlate() and
>>> drivers/irqchip/irq-loongson-liointc.c:liointc_set_type()
>>> are also wrong, and need a similar change.
>>
>> The one in irq-atmel-aic.c looks indeed strikingly similar. The one in
>> irq-loongson-liointc.c is slightly different though. Instead of:
>>
>> irq_gc_lock_irqsave() -> guard(raw_spinlock_irq)
>>
>> it does:
>>
>> irq_gc_lock_irqsave() -> guard(raw_spinlock)
>>
>> I don't know what the implications are though.
>>
>>> Unfortunately I have no hardware to verify.
>>
>> Neither do I.
>
> I'm on it. Expect feedback later today...
Answering to myself: tested working okay on at91sam9x35ek board (with
IRQ controller matching compatible string "atmel,at91rm9200-aic"
(handled by file irq-atmel-aic.c) and your modification:
--- a/drivers/irqchip/irq-atmel-aic.c
+++ b/drivers/irqchip/irq-atmel-aic.c
@@ -188,7 +188,7 @@ static int aic_irq_domain_xlate(struct irq_domain *d,
gc = dgc->gc[idx];
- guard(raw_spinlock_irq)(&gc->lock);
+ guard(raw_spinlock_irqsave)(&gc->lock);
smr = irq_reg_readl(gc, AT91_AIC_SMR(*out_hwirq));
aic_common_set_priority(intspec[2], &smr);
irq_reg_writel(gc, smr, AT91_AIC_SMR(*out_hwirq));
Thanks, best regards,
Nicolas
> Thanks so much for the heads-up.
>
> Best regards,
> Nicolas
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [ISSUE + PATCH] Interrupts were enabled early by spinlock guard
2025-08-25 12:33 ` Nicolas Ferre
@ 2025-08-25 14:12 ` Nicolas Ferre
0 siblings, 0 replies; 9+ messages in thread
From: Nicolas Ferre @ 2025-08-25 14:12 UTC (permalink / raw)
To: Edgar Bonet, Thomas Gleixner, Alexandre Belloni, Claudiu Beznea
Cc: linux-kernel, linux-arm-kernel, Geert Uytterhoeven
On 25/08/2025 at 14:33, Nicolas Ferre wrote:
> On 14/08/2025 at 14:59, Edgar Bonet wrote:
>> Hello everybody!
>>
>> I am facing an "Interrupts were enabled early" kernel warning which, as
>> far as I can tell, is caused by a spinlock guard in an ARM/Microchip
>> IRQCHIP driver. I think I solved the issue, and I am proposing a patch
>> below, after the scissor. But I must first disclose that:
>>
>> * I am completely new to Linux internals and its development process.
>> This is why I chose to err on the side of providing too much
>> information on my issue. It is not unlikely that I am doing
>> something very wrong.
>>
>> * I am not subscribed to either of the linux-{,arm-}kernel mailing
>> lists
>>
>> * I will be far from the Internet in the few days to come. I should be
>> connected an responsive starting from 2025-08-24.
>
> Edgar,
>
> Thanks a lot for having investigated this.
>
>
>> ## Context
>>
>> I am playing with an Acmesystems Acqua[1] system on module, which is
>> based on a SAMA5D31 SoC (single core Cortex-A5). I maintain the
>> Buildroot defconfig for this board,[2] which is currently based on a
>> vanilla Linux kernel v6.12.41.
>>
>> As I wanted to check that the board runs fine on newer kernels, I built
>> a v6.16 for it using gcc 14, binutils 2.43, the in-tree sama5_defconfig
>> merged with this fragment:
>>
>> # CONFIG_BRIDGE is not set
>> # CONFIG_MODULES is not set
>> # CONFIG_NET_DSA is not set
>> # CONFIG_WIRELESS is not set
>> # CONFIG_USB_NET_DRIVERS is not set
>> # CONFIG_WLAN is not set
>> # CONFIG_MEDIA_SUPPORT is not set
>> # CONFIG_SOUND is not set
>> # CONFIG_AUTOFS_FS is not set
>>
>> and the Buildroot-provided device tree, patched for compatibility with
>> Linux commit 510a6190cf5e ("ARM: dts: microchip: fix faulty ohci/ehci
>> node names").
>>
>> The defconfig fragment above is meant to remove module support (with
>> some unused drivers along the way), which makes testing easier for me.
>>
>>
>> ## Issue
>>
>> While booting, Linux v6.16 printed this message on the serial console:
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 0 at init/main.c:1024 start_kernel+0x4d0/0x5dc
>> Interrupts were enabled early
>> CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.16.0 #1 NONE
>> Hardware name: Atmel SAMA5
>> Call trace:
>> unwind_backtrace from show_stack+0x10/0x14
>> show_stack from dump_stack_lvl+0x38/0x48
>> dump_stack_lvl from __warn+0x78/0xd4
>> __warn from warn_slowpath_fmt+0x98/0xc8
>> warn_slowpath_fmt from start_kernel+0x4d0/0x5dc
>> start_kernel from 0x0
>> ---[ end trace 0000000000000000 ]---
>
> Indeed, reproduced on sam9x75 (which has AIC5), on 6.17.0-rc3.
>
>> The board seemed to work fine, so maybe this warning is completely
>> harmless. It looked, however, scary enough to deserve some
>> investigation.
>>
>>
>> ## Bug hunting
>>
>> I could not reproduce the issue on a qemu virtual machine. All tests
>> were then done on the real hardware.
>>
>> I looked for the Linux commit that introduced the issue. ‘git bisect’
>> told me this was 195298c3b116 ("genirq/generic-chip: Convert core code
>> to lock guards"). I then checked out a recent mainline kernel, namely
>> v6.17-rc1, and tried to revert that commit. For this, I first had to
>> revert two follow-up commits, namely 7ae844a6650c ("genirq/generic-chip:
>> Remove unused lock wrappers") and 771487050f83 ("genirq/generic-chip:
>> Fix incorrect lock guard conversions"). This was unsuccessful: I still
>> had the warning.
>>
>> I went back to a clean v6.17-rc1 and tried to find the thing that was
>> enabling interrupts early. After lots of printk() debugging, I
>> discovered it was a guard(raw_spinlock_irq) destructor. The call chain
>> is this (line numbers are for v6.17-rc1):
>>
>> start_kernel (init/main.c:1011)
>> -> time_init (arch/arm/kernel/time.c:96)
>> -> timer_probe (drivers/clocksource/timer-probe.c:30)
>> => tcb_clksrc_init (drivers/clocksource/timer-atmel-tcb.c:413)
>> -> of_irq_get (drivers/of/irq.c:474)
>> -> irq_create_of_mapping (kernel/irq/irqdomain.c:980)
>> -> irq_create_fwspec_mapping (kernel/irq/irqdomain.c:848)
>> => aic5_irq_domain_xlate (drivers/irqchip/irq-atmel-aic5.c:287)
>> -> guard destructor (raw_spin_unlock_irq?)
>>
>> where (=>) is an indirect call through a function pointer.
>>
>>
>> ## Tentative fix
>>
>> Commit 771487050f83 gave me the inspiration. The guard in question was
>> introduced by b00bee8afaca ("irqchip: Convert generic irqchip locking to
>> guards"), which replaced calls to irq_gc_lock_irq{save,restore}() by
>> guard(raw_spinlock_irq) (with no “save” in the name). The commit log
>> states that this is “intended and correct”, but I could not make sense
>> of the explanation. My (possibly faulty) understanding is that the guard
>> constructor disables interrupts, and the destructor either
>> unconditionally enables them (raw_spinlock_irq), or restores the
>> previous interrupt state (raw_spinlock_irqsave).
>>
>> I then replaced guard(raw_spinlock_irq) with guard(raw_spinlock_irqsave)
>> and that seemed to do the job: the warning is gone. See the patch below
>> the scissors.
>>
>> Best regards, and thank-you for reading so far.
>>
>> Edgar Bonet.
>>
>> [1] https://www.acmesystems.it/acqua
>> [2] https://gitlab.com/buildroot.org/buildroot/-/blob/2025.08-rc1/configs/acmesystems_acqua_a5_512mb_defconfig
>>
>> ------------------------------------------------------------------ >8 --
>> Subject: [PATCH] irqchip/atmel-aic5: Fix incorrect lock guard conversion
>>
>> Commit b00bee8afaca ("irqchip: Convert generic irqchip locking to guards")
>> replaced calls to irq_gc_lock_irq{save,restore}() with
>> guard(raw_spinlock_irq). However, in irq-atmel-aic5.c, one such guard is
>> created early in the boot process, before interrupts are initially enabled.
>> As its destructor enables interrupts, this results in the following warning
>> on a SAMA5D31-based system:
>>
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 0 at init/main.c:1024 start_kernel+0x4d0/0x5dc
>> Interrupts were enabled early
>> CPU: 0 UID: 0 PID: 0 Comm: swapper Not tainted 6.16.0 #1 NONE
>> Hardware name: Atmel SAMA5
>> Call trace:
>> unwind_backtrace from show_stack+0x10/0x14
>> show_stack from dump_stack_lvl+0x38/0x48
>> dump_stack_lvl from __warn+0x78/0xd4
>> __warn from warn_slowpath_fmt+0x98/0xc8
>> warn_slowpath_fmt from start_kernel+0x4d0/0x5dc
>> start_kernel from 0x0
>> ---[ end trace 0000000000000000 ]---
>>
>> Fix this by using guard(raw_spinlock_irqsave) instead.
>>
>> Fixes: b00bee8afaca ("irqchip: Convert generic irqchip locking to guards")
>> Signed-off-by: Edgar Bonet <bonet@grenoble.cnrs.fr>
>
> Indeed. Thanks for the detailed explanation and the patch:
>
> Tested-by: Nicolas Ferre <nicolas.ferre@microchip.com> # on sam9x75
For you Thomas, also this test:
Tested-by: Nicolas Ferre <nicolas.ferre@microchip.com> # on sam9x35 (for
similar changes to irq-atmel-aic.c)
Thanks, best regards,
Nicolas
> Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com> (if needed)
>
> Best regards,
> Nicolas
>
>> ---
>> drivers/irqchip/irq-atmel-aic5.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/irqchip/irq-atmel-aic5.c b/drivers/irqchip/irq-atmel-aic5.c
>> index 60b00d2c3d7a..1f14b401f71d 100644
>> --- a/drivers/irqchip/irq-atmel-aic5.c
>> +++ b/drivers/irqchip/irq-atmel-aic5.c
>> @@ -279,7 +279,7 @@ static int aic5_irq_domain_xlate(struct irq_domain *d,
>> if (ret)
>> return ret;
>>
>> - guard(raw_spinlock_irq)(&bgc->lock);
>> + guard(raw_spinlock_irqsave)(&bgc->lock);
>> irq_reg_writel(bgc, *out_hwirq, AT91_AIC5_SSR);
>> smr = irq_reg_readl(bgc, AT91_AIC5_SMR);
>> aic_common_set_priority(intspec[2], &smr);
>> --
>> 2.43.0
>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-08-25 15:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-14 12:59 [ISSUE + PATCH] Interrupts were enabled early by spinlock guard Edgar Bonet
2025-08-14 14:39 ` Geert Uytterhoeven
2025-08-14 15:28 ` Edgar Bonet
2025-08-23 19:33 ` Thomas Gleixner
2025-08-23 19:35 ` Thomas Gleixner
2025-08-25 12:35 ` Nicolas Ferre
2025-08-25 14:10 ` Nicolas Ferre
2025-08-25 12:33 ` Nicolas Ferre
2025-08-25 14:12 ` Nicolas Ferre
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).