Re: [PATCH Dovetail 2/2] arm64: irq_pipeline: Fix the demotion checks for el0 and el1 IRQs

On Tue, 2026-02-17 at 11:11 +0100, Philippe Gerum wrote:
> Philippe Gerum <[email protected]> writes:
> 
> > Florian Bezdeka <[email protected]> writes:
> > 
> > > On Tue, 2026-02-17 at 09:41 +0100, Philippe Gerum wrote:
> > > > > Comparing with x86 again, I think that my proposal is "correct" in terms
> > > > > of identical to what x86 does. Do all architectures have a gap here?
> > > > 
> > > > x86 has a single implementation for both user and kernel preemption
> > > > paths, arm64 has two since the privilege level is explicitly stated by
> > > > the irq handler being called, but this still must translate identically
> > > > logically speaking. Your implementation is missing the kernel preemption
> > > > path after demotion.
> > > > 
> > > > i.e. when checking for running_oob() || irqs_disabled(), the cases
> > > > covered are:
> > > > 
> > > > (1) in-band user path on entry (implies !irqs_disabled())
> > > > (2) oob user path on entry (might be demoted)
> > > > (3) (virtually) stalled in-band kernel path on entry (implies no reschedule,
> > > >   filtered out by irqentry_exit())
> > > > (4) oob kernel path on entry (might be demoted)
> > > > 
> > > > Therefore, with your patch in, el1 is now missing (4).
> > > 
> > > Right, but I'm wondering if x86 ignores this case as well.
> > > 
> > > After demotion of a oob kernel path entry, user_mode() should still be
> > > false - bypassing the call to irqentry_exit_to_user_mode() - No?
> > 
> > Yes, x86 assumes that a kernel path demoted to in-band is going to cross
> > an IRQ synchronization point shortly after on return to the preempted
> > context. Now, with hindsight, the question is: are we 100% certain of
> > that? Any real (hw) IRQ over the in-band stage would trigger the
> > synchronization as expected, but a synthetic one posted from the oob
> > stage might linger if this assumption ends up being wrong. I need to
> > have a second look at this code.
> 
> Which means that your latest patch series is ok and complete. The change
> if any would most likely happen in the generic pipeline bits.

I think (2) is still wrong on arm64 now, so:

@@ -65,15 +65,17 @@ static noinstr void arm64_pipeline_el0_irq(struct
pt_regs *regs,
                                void (*handler)(struct pt_regs *))
 {
        struct irq_stage_data *prevd;
+       bool oob_on_enter;
 
        arm64_enter_from_user_mode(regs);
        instrumentation_begin();
        /* Prep for handling, switching oob if needed. */
+       oob_on_enter = running_oob();
        prevd = handle_irq_pipelined_prepare(regs);
        do_interrupt_handler(regs, handler);
        /* Done, unwind now. */
        handle_irq_pipelined_finish(prevd, regs);
-       if (running_inband()) {
+       if (oob_on_enter && running_inband()) {
                stall_inband_nocheck();
                irqentry_exit_to_user_mode(regs);
        }

Do you agree?

I will prepare a v2 after some more CI testing.

Florian