On Tue, 29 Aug 2017 10:20:48 +1000 Paul Mackerras <pau...@ozlabs.org> wrote:
> On Fri, Aug 25, 2017 at 02:30:36PM +1000, Nicholas Piggin wrote: > > When stop is executed with EC=ESL=0, it appears to execute like a > > normal instruction (resuming from NIP when woken by interrupt). > > So all the save/restore handling can be avoided completely. In > > particular NV GPRs do not have to be saved, and MSR does not have > > to be switched back to kernel MSR. > > > > So move the test for "lite" sleep states out to power9_idle_stop. > > > > Reviewed-by: Gautham R. Shenoy <e...@linux.vnet.ibm.com> > > Signed-off-by: Nicholas Piggin <npig...@gmail.com> > > --- > > arch/powerpc/kernel/idle_book3s.S | 35 ++++++++++++++++++++++++----------- > > 1 file changed, 24 insertions(+), 11 deletions(-) > > > > diff --git a/arch/powerpc/kernel/idle_book3s.S > > b/arch/powerpc/kernel/idle_book3s.S > > index 32d65ee323a0..fa56120bd0bc 100644 > > --- a/arch/powerpc/kernel/idle_book3s.S > > +++ b/arch/powerpc/kernel/idle_book3s.S > > @@ -315,9 +315,6 @@ enter_winkle: > > > > ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE) > > > > -/* > > - * r3 - PSSCR value corresponding to the requested stop state. > > - */ > > #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE > > power_enter_stop_kvm_rm: > > /* > > @@ -330,14 +327,11 @@ power_enter_stop_kvm_rm: > > li r4,KVM_HWTHREAD_IN_IDLE > > /* DO THIS IN REAL MODE! See comment above. */ > > stb r4,HSTATE_HWTHREAD_STATE(r13) > > -#endif > > -power_enter_stop: > > /* > > * Check if we are executing the lite variant with ESL=EC=0 > > */ > > - andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED > > - clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */ > > - bne .Lhandle_esl_ec_set > > + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED > > + bne power_enter_stop_esl > > PPC_STOP > > li r3,0 /* Since we didn't lose state, return 0 */ > > > > @@ -354,8 +348,13 @@ power_enter_stop: > > */ > > li r12, 0 > > b pnv_wakeup_noloss > > +#endif > > > > -.Lhandle_esl_ec_set: > > +/* > > + * r3 - PSSCR value corresponding to the requested stop state. > > + */ > > +power_enter_stop_esl: > > + clrldi r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */ > > /* > > * POWER9 DD2 can incorrectly set PMAO when waking up after a > > * state-loss idle. Saving and restoring MMCR0 over idle is a > > @@ -428,9 +427,23 @@ ALT_FTR_SECTION_END_NESTED_IFSET(CPU_FTR_ARCH_207S, > > 66); \ > > * r3 contains desired PSSCR register value. > > */ > > _GLOBAL(power9_idle_stop) > > - std r3, PACA_REQ_PSSCR(r13) > > mtspr SPRN_PSSCR,r3 > > - LOAD_REG_ADDR(r4,power_enter_stop) > > + > > + /* > > + * Check if we are executing the lite variant with ESL=EC=0 > > + * This case resumes execution after the stop instruction without > > + * losing any state, so nothing has to be saved. The following > > + * instructions up to the blr must be skipped if we want to > > + * use power_enter_stop_kvm_rm. > > + */ > > + andis. r4,r3,PSSCR_EC_ESL_MASK_SHIFTED > > I realize you're just moving existing code, but I think this would be > clearer (to me, anyway) as > > andis. r4, r3, (PSSCR_EC | PSSCR_ESL)@h Agreed, it's not a very helpful extra indirection. Perhaps we could do a cleanup patch to fix that (and your other points like the 206/207 confusion). Thanks for the reviews. > Apart from that very minor nit, > > Reviewed-by: Paul Mackerras <pau...@ozlabs.org>
