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

Apart from that very minor nit,

Reviewed-by: Paul Mackerras <pau...@ozlabs.org>

Reply via email to