On Tue, Jun 13, 2017 at 12:46:02AM +1000, Nicholas Piggin wrote: > Hi Gautham, > > Thanks for the reviews. > > On Mon, 12 Jun 2017 14:07:27 +0530 > Gautham R Shenoy <e...@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 <npig...@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.
Ah, I see. Yes, my bad. I didn't check which one we are calling in the dynamic split case. Reviewed-by: Gautham R. Shenoy <e...@linux.vnet.ibm.com> > > Not the best names perhaps. Open to better suggestions. > > Thanks, > Nick > -- Thanks and Regards gautham.
