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

On Tue, 2026-02-17 at 14:31 +0100, Philippe Gerum wrote:
> Florian Bezdeka <[email protected]> writes:
> 
> > 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?
> > 
> 
> On a second look, _el0 needs no particular handling of the demotion
> case, because arm64_exit_to_user_mode() already does what is required.
> 

Right. All this funny findings along the way:
/* Temporary workaround to keep ARM64 alive */

Preparing v2 now.