lkml.org 
[lkml]   [2021]   [Feb]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [RFC] IRQ handlers run with some high-priority interrupts(not NMI) enabled on some platform
Date


> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@kernel.org]
> Sent: Saturday, February 13, 2021 11:34 AM
> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>
> Cc: tglx@linutronix.de; gregkh@linuxfoundation.org; arnd@arndb.de;
> geert@linux-m68k.org; funaho@jurai.org; philb@gnu.org; corbet@lwn.net;
> mingo@redhat.com; linux-m68k@lists.linux-m68k.org;
> fthain@telegraphics.com.au; linux-kernel@vger.kernel.org
> 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@hisilicon.com> 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

\
 
 \ /
  Last update: 2021-02-13 00:01    [W:0.092 / U:1.196 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site