RE: [RFC] IRQ handlers run with some high-priority interrupts(not NMI) enabled on some platform

[Song Bao Hua (Barry Song)]

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@xxxxxxxxxx]
> Sent: Saturday, February 13, 2021 11:34 AM
> To: Song Bao Hua (Barry Song) <song.bao.hua@xxxxxxxxxxxxx>
> Cc: tglx@xxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; arnd@xxxxxxxx;
> geert@xxxxxxxxxxxxxx; funaho@xxxxxxxxx; philb@xxxxxxx; corbet@xxxxxxx;
> mingo@xxxxxxxxxx; linux-m68k@xxxxxxxxxxxxxxxxxxxx;
> fthain@xxxxxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [RFC] IRQ handlers run with some high-priority interrupts(not NMI)
> enabled on some platform
> 
> On Fri, Feb 12, 2021 at 2:18 AM Song Bao Hua (Barry Song)
> <song.bao.hua@xxxxxxxxxxxxx> wrote:
> 
> > So I am requesting comments on:
> > 1. are we expecting all interrupts except NMI to be disabled in irq handler,
> > or do we actually allow some high-priority interrupts between low and NMI
> to
> > come in some platforms?
> 
> I tried to come to an answer but this does not seem particularly well-defined.
> There are a few things I noticed:
> 
> - going through the local_irq_save()/restore() implementations on all
>   architectures, I did not find any other ones besides m68k that leave
>   high-priority interrupts enabled. I did see that at least alpha and openrisc
>   are designed to support that in hardware, but the code just leaves the
>   interrupts disabled.

The case is a little different. Explicit local_irq_save() does disable all
high priority interrupts on m68k. The only difference is arch_irqs_disabled()
of m68k will return true while low-priority interrupts are masked and high
-priority are still open. M68k's hardIRQ also runs in this context with high
priority interrupts enabled.

> 
> - The generic code is clearly prepared to handle nested hardirqs, and
>    the irq_enter()/irq_exit() functions have a counter in preempt_count
>    for the nesting level, using a 4-bit number for hardirq, plus another
>    4-bit number for NMI.

Yes, I understand nested interrupts are supported by an explicit 
local_irq_enable_in_hardirq(). Mk68k's case is different, nested
interrupts can come with arch_irqs_disabled() is true and while
nobody has called local_irq_enable_in_hardirq() in the previous
hardIRQ because hardIRQ keeps high-priority interrupts open.

> 
> - There are a couple of (ancient) drivers that enable interrupts in their
>    interrupt handlers, see the four callers of local_irq_enable_in_hardirq()
>    (all in the old drivers/ide stack) and arch/ia64/kernel/time.c, which
>    enables interupts in its timer function (I recently tried removing this
>    and my patch broke ia64 timers, but I'm not sure if the cause was
>    the local_irq_enable() or something else).
> 
> - The local_irq_enable_in_hardirq() function itself turns into a nop
>   when lockdep is enabled, since d7e9629de051 ("[PATCH] lockdep:
>   add local_irq_enable_in_hardirq() API"). According to the comment
>   in there, lockdep already enforces the behavior you suggest. Note that
>   lockdep support is missing on m68k (and also alpha, h8300, ia64, nios2,
>   and parisc).
> 
> > 2. If either side is true, I think we need to document it somewhere as there
> > is always confusion about this.
> >
> > Personally, I would expect all interrupts to be disabled and I like the way
> > of ARM64 to only use high-priority interrupt as pseudo NMI:
> > https://lwn.net/Articles/755906/
> > Though Finn argued that this will contribute to lose hardware feature of m68k.
> 
> Regardless of what is documented, I would argue that any platform
> that relies on this is at the minimum doing something risky because at
> the minimum this runs into hard to debug code paths that are not
> exercised on any of the common architectures.
> 
>         Arnd


Thanks
Barry