Re: [PATCH 01/14] powerpc/64s: idle move soft interrupt mask logic into C code

[Nicholas Piggin <npiggin@gmail.com>]

Hi Gautham,

Thanks for the reviews.

On Mon, 12 Jun 2017 14:07:27 +0530
Gautham R Shenoy <ego@linux.vnet.ibm.com> wrote:

> Hi Nick,
> 
> (Added Paul Mackerass to the Cc)
> On Mon, Jun 12, 2017 at 09:58:22AM +1000, Nicholas Piggin wrote:
> > This simplifies the asm and fixes irq-off tracing over sleep
> > instructions.
> > 
> > Also move powersave_nap check for POWER8 into C code, and move
> > PSSCR register value calculation for POWER9 into C.
> >  
> 
> Thanks for doing this. Only one minor comment.
> 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> > index 98a6d07ecb5c..35cf5bb7daed 100644
> > --- a/arch/powerpc/kernel/idle_book3s.S
> > +++ b/arch/powerpc/kernel/idle_book3s.S  
> 
> [..snip..]
> 
> > @@ -109,13 +109,9 @@ core_idle_lock_held:
> >  /*
> >   * Pass requested state in r3:
> >   *	r3 - PNV_THREAD_NAP/SLEEP/WINKLE in POWER8
> > - *	   - Requested STOP state in POWER9
> > + *	   - Requested PSSCR value in POWER9
> >   *
> > - * To check IRQ_HAPPENED in r4
> > - * 	0 - don't check
> > - * 	1 - check
> > - *
> > - * Address to 'rfid' to in r5
> > + * Address of idle handler to 'rfid' to in r4
> >   */
> >  pnv_powersave_common:
> >  	/* Use r3 to pass state nap/sleep/winkle */
> > @@ -131,30 +127,7 @@ pnv_powersave_common:
> >  	std	r0,_LINK(r1)
> >  	std	r0,_NIP(r1)
> > 
> > -	/* Hard disable interrupts */
> > -	mfmsr	r9
> > -	rldicl	r9,r9,48,1
> > -	rotldi	r9,r9,16
> > -	mtmsrd	r9,1			/* hard-disable interrupts */
> > -
> > -	/* Check if something happened while soft-disabled */
> > -	lbz	r0,PACAIRQHAPPENED(r13)
> > -	andi.	r0,r0,~PACA_IRQ_HARD_DIS@l
> > -	beq	1f
> > -	cmpwi	cr0,r4,0  
> 
> There were callers to power7_nap() in the dynamic-core-split/unsplit
> which wanted nap to be forced irrespective of whether an irq was
> pending or not. With this new patch, there won't be a way to enforce
> that on POWER8.

Actually there are two APIs to sleep now, the low level one which
does not check lazy irq or nap disable is power7_idle_insn(). The
one which sets up the lazy irq state is power7_idle_type(). The
dynamic core split calls the former, so AFAIKS it should be equivalent.

Not the best names perhaps. Open to better suggestions.

Thanks,
Nick