On Tue, 13 Dec 2016 16:36:11 +1100
Balbir Singh [off-list ref] wrote:

On Mon, 2016-12-12 at 23:31 +1000, Nicholas Piggin wrote:

quoted

On Mon, 12 Dec 2016 20:50:03 +1100
Balbir Singh [off-list ref] wrote:
  

quoted

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>

quoted

quoted

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
 

quoted

quoted

+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
 

quoted

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

Other thing is, I would do the masked external interrupt as its own
patch. NMI is basically independent as far as I can see.

Thanks,
Nick