* [PATCH v2] arm64/irqflags: __always_inline the arch_local_irq_*() helpers
@ 2026-04-21 15:58 Breno Leitao
2026-04-21 16:07 ` Leonardo Bras
2026-04-23 16:45 ` Breno Leitao
0 siblings, 2 replies; 3+ messages in thread
From: Breno Leitao @ 2026-04-21 15:58 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, mark.rutland
Cc: leo.bras, leo.yan, linux-arm-kernel, linux-kernel, palmer,
paulmck, puranjay, usama.arif, rmikey, kernel-team, Breno Leitao
The arch_local_irq_*() wrappers in <asm/irqflags.h> dispatch between two
underlying primitives: the __daif_* path on most systems, and the
__pmr_* path on builds that use GIC PMR-based masking (Pseudo-NMI). The
leaf primitives are already __always_inline, but the wrappers themselves
are plain "static inline".
That is unsafe for noinstr callers: nothing prevents the compiler from
emitting an out-of-line copy of e.g. arch_local_irq_disable(), and an
out-of-line copy can be instrumented (ftrace, kcov, sanitizers), which
breaks the noinstr contract on the entry/idle paths that rely on these
helpers.
x86 hit and fixed exactly this class of bug in commit 7a745be1cc90
("x86/entry: __always_inline irqflags for noinstr").
Force-inline all of the arch_local_irq_*() wrappers so they cannot be
emitted out-of-line:
- arch_local_irq_enable()
- arch_local_irq_disable()
- arch_local_save_flags()
- arch_irqs_disabled_flags()
- arch_irqs_disabled()
- arch_local_irq_save()
- arch_local_irq_restore()
The primary motivation is noinstr safety. There is a useful side effect
for fleet-wide profiling: when the wrapper is emitted out-of-line,
samples taken inside it during the post-WFI IRQ unmask in
default_idle_call() are attributed to arch_local_irq_enable rather than
default_idle_call(), and the FP-unwinder loses default_idle_call() from
the chain.
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v2:
- Expand the functions that uses always_inline in arm64
- Link to v1: https://patch.msgid.link/20260420-arm64_always_inline-v1-1-dba919cf46bc@debian.org
---
arch/arm64/include/asm/irqflags.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
index d4d7451c2c129..a8cb5a5c93b78 100644
--- a/arch/arm64/include/asm/irqflags.h
+++ b/arch/arm64/include/asm/irqflags.h
@@ -40,7 +40,7 @@ static __always_inline void __pmr_local_irq_enable(void)
barrier();
}
-static inline void arch_local_irq_enable(void)
+static __always_inline void arch_local_irq_enable(void)
{
if (system_uses_irq_prio_masking()) {
__pmr_local_irq_enable();
@@ -68,7 +68,7 @@ static __always_inline void __pmr_local_irq_disable(void)
barrier();
}
-static inline void arch_local_irq_disable(void)
+static __always_inline void arch_local_irq_disable(void)
{
if (system_uses_irq_prio_masking()) {
__pmr_local_irq_disable();
@@ -90,7 +90,7 @@ static __always_inline unsigned long __pmr_local_save_flags(void)
/*
* Save the current interrupt enable state.
*/
-static inline unsigned long arch_local_save_flags(void)
+static __always_inline unsigned long arch_local_save_flags(void)
{
if (system_uses_irq_prio_masking()) {
return __pmr_local_save_flags();
@@ -109,7 +109,7 @@ static __always_inline bool __pmr_irqs_disabled_flags(unsigned long flags)
return flags != GIC_PRIO_IRQON;
}
-static inline bool arch_irqs_disabled_flags(unsigned long flags)
+static __always_inline bool arch_irqs_disabled_flags(unsigned long flags)
{
if (system_uses_irq_prio_masking()) {
return __pmr_irqs_disabled_flags(flags);
@@ -128,7 +128,7 @@ static __always_inline bool __pmr_irqs_disabled(void)
return __pmr_irqs_disabled_flags(__pmr_local_save_flags());
}
-static inline bool arch_irqs_disabled(void)
+static __always_inline bool arch_irqs_disabled(void)
{
if (system_uses_irq_prio_masking()) {
return __pmr_irqs_disabled();
@@ -160,7 +160,7 @@ static __always_inline unsigned long __pmr_local_irq_save(void)
return flags;
}
-static inline unsigned long arch_local_irq_save(void)
+static __always_inline unsigned long arch_local_irq_save(void)
{
if (system_uses_irq_prio_masking()) {
return __pmr_local_irq_save();
@@ -187,7 +187,7 @@ static __always_inline void __pmr_local_irq_restore(unsigned long flags)
/*
* restore saved IRQ state
*/
-static inline void arch_local_irq_restore(unsigned long flags)
+static __always_inline void arch_local_irq_restore(unsigned long flags)
{
if (system_uses_irq_prio_masking()) {
__pmr_local_irq_restore(flags);
---
base-commit: bee6ea30c48788e18348309f891ed8afbf7702ac
change-id: 20260420-arm64_always_inline-6bc9dd3c17e6
Best regards,
--
Breno Leitao <leitao@debian.org>
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v2] arm64/irqflags: __always_inline the arch_local_irq_*() helpers
2026-04-21 15:58 [PATCH v2] arm64/irqflags: __always_inline the arch_local_irq_*() helpers Breno Leitao
@ 2026-04-21 16:07 ` Leonardo Bras
2026-04-23 16:45 ` Breno Leitao
1 sibling, 0 replies; 3+ messages in thread
From: Leonardo Bras @ 2026-04-21 16:07 UTC (permalink / raw)
To: Breno Leitao
Cc: Leonardo Bras, Catalin Marinas, Will Deacon, mark.rutland,
leo.yan, linux-arm-kernel, linux-kernel, palmer, paulmck,
puranjay, usama.arif, rmikey, kernel-team
On Tue, Apr 21, 2026 at 08:58:57AM -0700, Breno Leitao wrote:
> The arch_local_irq_*() wrappers in <asm/irqflags.h> dispatch between two
> underlying primitives: the __daif_* path on most systems, and the
> __pmr_* path on builds that use GIC PMR-based masking (Pseudo-NMI). The
> leaf primitives are already __always_inline, but the wrappers themselves
> are plain "static inline".
>
> That is unsafe for noinstr callers: nothing prevents the compiler from
> emitting an out-of-line copy of e.g. arch_local_irq_disable(), and an
> out-of-line copy can be instrumented (ftrace, kcov, sanitizers), which
> breaks the noinstr contract on the entry/idle paths that rely on these
> helpers.
>
> x86 hit and fixed exactly this class of bug in commit 7a745be1cc90
> ("x86/entry: __always_inline irqflags for noinstr").
>
> Force-inline all of the arch_local_irq_*() wrappers so they cannot be
> emitted out-of-line:
>
> - arch_local_irq_enable()
> - arch_local_irq_disable()
> - arch_local_save_flags()
> - arch_irqs_disabled_flags()
> - arch_irqs_disabled()
> - arch_local_irq_save()
> - arch_local_irq_restore()
>
> The primary motivation is noinstr safety. There is a useful side effect
> for fleet-wide profiling: when the wrapper is emitted out-of-line,
> samples taken inside it during the post-WFI IRQ unmask in
> default_idle_call() are attributed to arch_local_irq_enable rather than
> default_idle_call(), and the FP-unwinder loses default_idle_call() from
> the chain.
>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> Changes in v2:
> - Expand the functions that uses always_inline in arm64
> - Link to v1: https://patch.msgid.link/20260420-arm64_always_inline-v1-1-dba919cf46bc@debian.org
> ---
> arch/arm64/include/asm/irqflags.h | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/include/asm/irqflags.h b/arch/arm64/include/asm/irqflags.h
> index d4d7451c2c129..a8cb5a5c93b78 100644
> --- a/arch/arm64/include/asm/irqflags.h
> +++ b/arch/arm64/include/asm/irqflags.h
> @@ -40,7 +40,7 @@ static __always_inline void __pmr_local_irq_enable(void)
> barrier();
> }
>
> -static inline void arch_local_irq_enable(void)
> +static __always_inline void arch_local_irq_enable(void)
> {
> if (system_uses_irq_prio_masking()) {
> __pmr_local_irq_enable();
> @@ -68,7 +68,7 @@ static __always_inline void __pmr_local_irq_disable(void)
> barrier();
> }
>
> -static inline void arch_local_irq_disable(void)
> +static __always_inline void arch_local_irq_disable(void)
> {
> if (system_uses_irq_prio_masking()) {
> __pmr_local_irq_disable();
> @@ -90,7 +90,7 @@ static __always_inline unsigned long __pmr_local_save_flags(void)
> /*
> * Save the current interrupt enable state.
> */
> -static inline unsigned long arch_local_save_flags(void)
> +static __always_inline unsigned long arch_local_save_flags(void)
> {
> if (system_uses_irq_prio_masking()) {
> return __pmr_local_save_flags();
> @@ -109,7 +109,7 @@ static __always_inline bool __pmr_irqs_disabled_flags(unsigned long flags)
> return flags != GIC_PRIO_IRQON;
> }
>
> -static inline bool arch_irqs_disabled_flags(unsigned long flags)
> +static __always_inline bool arch_irqs_disabled_flags(unsigned long flags)
> {
> if (system_uses_irq_prio_masking()) {
> return __pmr_irqs_disabled_flags(flags);
> @@ -128,7 +128,7 @@ static __always_inline bool __pmr_irqs_disabled(void)
> return __pmr_irqs_disabled_flags(__pmr_local_save_flags());
> }
>
> -static inline bool arch_irqs_disabled(void)
> +static __always_inline bool arch_irqs_disabled(void)
> {
> if (system_uses_irq_prio_masking()) {
> return __pmr_irqs_disabled();
> @@ -160,7 +160,7 @@ static __always_inline unsigned long __pmr_local_irq_save(void)
> return flags;
> }
>
> -static inline unsigned long arch_local_irq_save(void)
> +static __always_inline unsigned long arch_local_irq_save(void)
> {
> if (system_uses_irq_prio_masking()) {
> return __pmr_local_irq_save();
> @@ -187,7 +187,7 @@ static __always_inline void __pmr_local_irq_restore(unsigned long flags)
> /*
> * restore saved IRQ state
> */
> -static inline void arch_local_irq_restore(unsigned long flags)
> +static __always_inline void arch_local_irq_restore(unsigned long flags)
> {
> if (system_uses_irq_prio_masking()) {
> __pmr_local_irq_restore(flags);
>
> ---
> base-commit: bee6ea30c48788e18348309f891ed8afbf7702ac
> change-id: 20260420-arm64_always_inline-6bc9dd3c17e6
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>
Looks correct to what was discussed in V1. FWIW:
Reviewed-by: Leonardo Bras <leo.bras@arm.com>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v2] arm64/irqflags: __always_inline the arch_local_irq_*() helpers
2026-04-21 15:58 [PATCH v2] arm64/irqflags: __always_inline the arch_local_irq_*() helpers Breno Leitao
2026-04-21 16:07 ` Leonardo Bras
@ 2026-04-23 16:45 ` Breno Leitao
1 sibling, 0 replies; 3+ messages in thread
From: Breno Leitao @ 2026-04-23 16:45 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, mark.rutland
Cc: leo.bras, leo.yan, linux-arm-kernel, linux-kernel, palmer,
paulmck, puranjay, usama.arif, rmikey, kernel-team
On Tue, Apr 21, 2026 at 08:58:57AM -0700, Breno Leitao wrote:
> The arch_local_irq_*() wrappers in <asm/irqflags.h> dispatch between two
> underlying primitives: the __daif_* path on most systems, and the
> __pmr_* path on builds that use GIC PMR-based masking (Pseudo-NMI). The
> leaf primitives are already __always_inline, but the wrappers themselves
> are plain "static inline".
>
> That is unsafe for noinstr callers: nothing prevents the compiler from
> emitting an out-of-line copy of e.g. arch_local_irq_disable(), and an
> out-of-line copy can be instrumented (ftrace, kcov, sanitizers), which
> breaks the noinstr contract on the entry/idle paths that rely on these
> helpers.
>
> x86 hit and fixed exactly this class of bug in commit 7a745be1cc90
> ("x86/entry: __always_inline irqflags for noinstr").
>
> Force-inline all of the arch_local_irq_*() wrappers so they cannot be
> emitted out-of-line:
>
> - arch_local_irq_enable()
> - arch_local_irq_disable()
> - arch_local_save_flags()
> - arch_irqs_disabled_flags()
> - arch_irqs_disabled()
> - arch_local_irq_save()
> - arch_local_irq_restore()
>
> The primary motivation is noinstr safety. There is a useful side effect
> for fleet-wide profiling: when the wrapper is emitted out-of-line,
> samples taken inside it during the post-WFI IRQ unmask in
> default_idle_call() are attributed to arch_local_irq_enable rather than
> default_idle_call(), and the FP-unwinder loses default_idle_call() from
> the chain.
FWIW I run scripts/bloat-o-meter on the kernel with and without the
patch, and the the code size is mostly the same. here is the result:
add/remove: 4/12 grow/shrink: 40/0 up/down: 1684/-652 (1032)
Function old new delta
__schedule 8892 9024 +132
irqentry_exit 816 892 +76
lockdep_hardirqs_off 396 452 +56
lock_is_held_type 412 468 +56
ct_idle_exit 76 132 +56
cpu_idle_poll 304 360 +56
arch_stack_walk_reliable 1152 1196 +44
arch_stack_walk 1184 1228 +44
arch_bpf_stack_walk 996 1040 +44
lockdep_hardirqs_on 464 504 +40
el0_watchpt 576 616 +40
el0_undef 560 600 +40
el0_sys 560 600 +40
el0_sve_acc 560 600 +40
el0_svc 600 640 +40
el0_sp 564 604 +40
el0_softstp 728 768 +40
el0_sme_acc 560 600 +40
el0_pc 740 780 +40
el0_mops 560 600 +40
el0_inv 564 604 +40
el0_interrupt 656 696 +40
el0_ia 716 756 +40
el0_gcs 560 600 +40
el0_fpsimd_exc 560 600 +40
el0_fpsimd_acc 560 600 +40
el0_fpac 560 600 +40
el0_da 568 608 +40
el0_bti 552 592 +40
el0_brk64 560 600 +40
el0_breakpt 720 760 +40
asm_exit_to_user_mode 416 456 +40
__el0_error_handler_common 592 632 +40
cpuidle_enter_state 1220 1248 +28
check_preemption_disabled 228 252 +24
default_idle_call 252 272 +20
ct_kernel_enter 388 404 +16
ct_idle_enter 52 68 +16
look_up_lock_class 364 376 +12
check_flags 492 504 +12
__CortexA53843419_FFFF800081146000 - 8 +8
__CortexA53843419_FFFF8000809C3004 - 8 +8
__CortexA53843419_FFFF8000809AE000 - 8 +8
__CortexA53843419_FFFF800080248004 - 8 +8
__CortexA53843419_FFFF80008100C000 8 - -8
__CortexA53843419_FFFF8000809A9000 8 - -8
__CortexA53843419_FFFF8000809A8004 8 - -8
__CortexA53843419_FFFF800080448008 8 - -8
__CortexA53843419_FFFF8000801EE000 8 - -8
arch_local_irq_restore 48 - -48
arch_local_save_flags 80 - -80
arch_local_irq_save 80 - -80
arch_local_irq_enable 84 - -84
arch_local_irq_disable 96 - -96
arch_irqs_disabled_flags 96 - -96
arch_irqs_disabled 128 - -128
Total: Before=163062863, After=163063895, chg +0.00%
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-23 16:45 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-21 15:58 [PATCH v2] arm64/irqflags: __always_inline the arch_local_irq_*() helpers Breno Leitao
2026-04-21 16:07 ` Leonardo Bras
2026-04-23 16:45 ` Breno Leitao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox