On Fri, Mar 20 2026 at 15:37, Mark Rutland wrote:

On Fri, Mar 20, 2026 at 03:59:40PM +0100, Thomas Gleixner wrote:

quoted

Why are you routing NMI like exceptions through irqentry_enter() and
irqentry_exit() in the first place? That's just wrong.

Sorry, the above was not clear, and some of this logic is gunk that has
been carried over unnnecessarily from our old exception handling flow.

The issue with pseudo-NMI is that it uses the same exception as regular
interrupts, but we don't know whether we have a pseudo-NMI until we
acknowledge the event at the irqchip level. When a pseudo-NMI is taken,
there are two possibilities:

(1) The pseudo-NMI is taken from a context where interrupts were
    *disabled*. The entry code immediately knows it must be a
    pseudo-NMI, and we call irqentry_nmi_{enter,exit}(), NOT
    irqentry_{enter,exit}(), treating it as an NMI.


(2) The pseudo-NMI was taken from a context where interrupts were
    *enabled*. The entry code doesn't know whether it's a pseudo-NMI or
    a regular interrupt, so it calls irqentry_{enter,exit}(), and then
    within that we'll call nmi_{enter,exit}() to transiently enter NMI
    context.

I realise this is crazy. I would love to delete pseudo-NMI.
Unfortunately people are using it.

What is it used for?

Putting aside the nesting here, I think it's fine to preempt upon return
from case (2), and we can delete the logic to avoid preempting.

Correct.

quoted

That means at the point where irqentry_entry() is invoked, the
architecture side should have made sure that everything is set up for
the kernel to operate until irqentry_exit() returns.

Ok. I think you're saying I should try:

* At entry, *before* irqentry_enter():
  - unmask everything EXCEPT regular interrupts.
  - fix up all the necessary state.

* At exception exit, *after* irqentry_exit():
  - mask everything.
  - fix up all the necessary state.

... right?

Yes.

quoted

Looking at your example:

quoted

| static void noinstr el1_abort(struct pt_regs *regs, unsigned long esr)
| {
|         unsigned long far = read_sysreg(far_el1);
|         irqentry_state_t state;
|
|         state = enter_from_kernel_mode(regs);
|         local_daif_inherit(regs);
|         do_mem_abort(far, esr, regs);
|         local_daif_mask();
|         exit_to_kernel_mode(regs, state);

and the paragraph right below that:

quoted

Currently, the generic irq entry code will attempt to preempt from any
exception under irqentry_exit() where interrupts were unmasked in the
original context. As arm64's entry code will have already masked
exceptions via DAIF, this results in the problems described above.

To me this looks like your ordering is wrong. Why are you doing the DAIF
inherit _after_ irqentry_enter() and the mask _before_ irqentry_exit()?

As above, I can go look at reworking this. 

For context, we do it this way today for several reasons, including:

(1) Because some of the arch-specific bits (such as checking the TFSR
    for MTE) in enter_from_kernel_mode() and exit_to_kernel_mode() need
    to be done while RCU is watching, etc, but needs other exceptions
    masked. I can look at reworking that.

Something like the below should do that for you. If you need more than
regs, then you can either stick something on your stack frame or we go
and extend irqentry_enter()/exit() with an additional argument which
allows you to hand some exception/interrupt specific cookie in. The core
code would just hand it through to arch_irqenter_enter/exit_rcu() along
with @regs. That cookie might be data or even a function pointer. The
core does not have to know about it.

(2) To minimize the number of times we have to write to things like
    DAIF, as that can be expensive.

(3) To simplify the management of things like DAIF, so that we don't
    have several points in time at which we need to inherit different
    pieces of state.

The above should cover your #2/3 too, no?

(4) Historical, as that's the flow we had in assembly, and prior to the
    move to generic irq entry.

No comment :)

quoted

I might be missing something, but this smells more than fishy.

As no other architecture has that problem I'm pretty sure that the
problem is not in the way how the generic code was designed. Why?

Hey, I'm not saying the generic entry code is wrong, just that there's a
mismatch between it and what would be optimal for arm64.

quoted

Because your architecture is _not_ sooo special! :)

I think it's pretty special, but not necessarily in the same sense. ;)

Hehehe.

Thanks,

        tglx
---        

--- a/kernel/entry/common.c
+++ b/kernel/entry/common.c

@@ -149,6 +149,7 @@ noinstr irqentry_state_t irqentry_enter(
 		instrumentation_begin();
 		kmsan_unpoison_entry_regs(regs);
 		trace_hardirqs_off_finish();
+		arch_irqentry_enter_rcu(regs);
 		instrumentation_end();
 
 		ret.exit_rcu = true;

@@ -166,6 +167,7 @@ noinstr irqentry_state_t irqentry_enter(
 	kmsan_unpoison_entry_regs(regs);
 	rcu_irq_enter_check_tick();
 	trace_hardirqs_off_finish();
+	arch_irqentry_enter_rcu(regs);
 	instrumentation_end();
 
 	return ret;

@@ -225,6 +227,7 @@ noinstr void irqentry_exit(struct pt_reg
 		 */
 		if (state.exit_rcu) {
 			instrumentation_begin();
+			arch_irqentry_exit_rcu(regs);
 			hrtimer_rearm_deferred();
 			/* Tell the tracer that IRET will enable interrupts */
 			trace_hardirqs_on_prepare();

@@ -239,11 +242,13 @@ noinstr void irqentry_exit(struct pt_reg
 		if (IS_ENABLED(CONFIG_PREEMPTION))
 			irqentry_exit_cond_resched();
 
+		arch_irqentry_exit_rcu(regs);
 		hrtimer_rearm_deferred();
 		/* Covers both tracing and lockdep */
 		trace_hardirqs_on();
 		instrumentation_end();
 	} else {
+		arch_irqentry_exit_rcu(regs);
 		/*
 		 * IRQ flags state is correct already. Just tell RCU if it
 		 * was not watching on entry.