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. Not the best names perhaps. Open to better suggestions. Thanks, Nick