Subject: Re: [powerpc/nmi: RFC 2/2] Keep interrupts enabled even on soft disable

From: Balbir Singh <[email-masked]>
Date: Tue, 13 Dec 2016 16:36:11 +1100

On Mon, 2016-12-12 at 23:31 +1000, Nicholas Piggin wrote:
> On Mon, 12 Dec 2016 20:50:03 +1100
> Balbir Singh <[email-masked]> wrote:
> 
> > This patch removes the disabling of interrupts
> > in soft-disable mode, when interrupts are received
> > (in lazy mode). The new scheme keeps the interrupts
> > enabled when we receive an interrupt and does the
> > following
> > 
> > a. On decrementer interrupt, instead of setting
> > dec to maximum and returning, we do the following
> >   i. Call a function handle_nmi_dec, which in
> >      turn calls handle_soft_nmi
> >   ii. handle_soft_nmi sets the decrementer value
> >       to 1 second and checks if more than 30
> >       seconds have passed since starting it. If
> >       so it calls BUG_ON(1), we can do an NMI
> >       panic as well.
> > b. When an external interrupt is received, we
> >    store the interrupt in local_paca via
> >    ppc_md.get_irq(). Later when interrupts are
> >    enabled and replayed, we reuse the stored
> >    interrupt and process it via generic_handle_irq
> 
> This seems pretty good. My NMI handler should plug in just
> the same to the masked decrementer, so that wouldn't be a
> problem.

Thats good to know, I believe so as well.

<snip>

> > while soft-disable */
> > +	u32 irq;			/* IRQ pending */
> >  	u8 nap_state_lost;		/* NV GPR values lost in
> > power7_idle */
> >  	u64 sprg_vdso;			/* Saved user-
> > visible sprg */
> 
> Can you avoid some padding if you move it to below irq_happened?
>

Will do
 
> > +EXC_COMMON(handle_nmi_dec, 0x900, handle_soft_nmi)
> > +EXC_COMMON(elevate_save_irq, 0x500, handle_elevated_irq)
> 
> I wonder if the name should match the type of interrupt rather than
> implementation detail (elevated?), and match the existing handlers
> e.g, hardware_interrupt_masked common handler could call
> do_IRQ_masked.
>

Sure, will rename them
 
> As for the NMI, I would prefer just to keep it out of the timer path
> completely and schedule a Linux timer for it as I had.
> 
> Otherwise, this looks nice if it does the right thing with the
> interrupt
> controller. It hasn't taken a lot of lines to implement which is very
> cool.
> 

Yep, although the code works for PPC_XICS only which is good for now.
When we do XIVE, we can add more bits

Balbir