[v3,2/4] powerpc/64s: idle POWER9 can execute stop without a sync sequence

Message ID 20170825043036.18236-3-npiggin@gmail.com
State Accepted
Headers show

Commit Message

Nicholas Piggin Aug. 25, 2017, 4:30 a.m.
Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/cpuidle.h | 16 ----------------
 arch/powerpc/kernel/idle_book3s.S  | 26 ++++++++++++++++++++------
 2 files changed, 20 insertions(+), 22 deletions(-)

Comments

Paul Mackerras Aug. 29, 2017, 12:11 a.m. | #1
On Fri, Aug 25, 2017 at 02:30:34PM +1000, Nicholas Piggin wrote:
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/include/asm/cpuidle.h | 16 ----------------
>  arch/powerpc/kernel/idle_book3s.S  | 26 ++++++++++++++++++++------
>  2 files changed, 20 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
> index 8a174cba5567..eb43b5c3a7b5 100644
> --- a/arch/powerpc/include/asm/cpuidle.h
> +++ b/arch/powerpc/include/asm/cpuidle.h
> @@ -101,20 +101,4 @@ static inline void report_invalid_psscr_val(u64 psscr_val, int err)
>  
>  #endif
>  
> -/* Idle state entry routines */
> -#ifdef	CONFIG_PPC_P7_NAP
> -#define IDLE_STATE_ENTER_SEQ(IDLE_INST)                         \
> -	/* Magic NAP/SLEEP/WINKLE mode enter sequence */	\
> -	std	r0,0(r1);					\
> -	ptesync;						\
> -	ld	r0,0(r1);					\
> -236:	cmpd	cr0,r0,r0;					\
> -	bne	236b;						\
> -	IDLE_INST;						\
> -
> -#define	IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST)			\
> -	IDLE_STATE_ENTER_SEQ(IDLE_INST)                         \
> -	b	.
> -#endif /* CONFIG_PPC_P7_NAP */
> -
>  #endif
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 4924647d964d..14e97f442167 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -205,6 +205,19 @@ pnv_powersave_common:
>  	mtmsrd	r7,0
>  	bctr
>  
> +/*
> + * This is the sequence required to execute idle instructions, as
> + * specified in ISA v2.07. MSR[IR] and MSR[DR] must be 0.
> + */
> +#define ARCH207_IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST)		\

We had to do this sequence on POWER7 also, which is architecture
v2.06.  Thus the comments and the naming (ARCH207_*) are a bit
misleading here.  The actual code change looks OK.

Paul.
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Ellerman Aug. 29, 2017, 10:30 a.m. | #2
Paul Mackerras <paulus@ozlabs.org> writes:
> On Fri, Aug 25, 2017 at 02:30:34PM +1000, Nicholas Piggin wrote:
>> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
>> index 4924647d964d..14e97f442167 100644
>> --- a/arch/powerpc/kernel/idle_book3s.S
>> +++ b/arch/powerpc/kernel/idle_book3s.S
>> @@ -205,6 +205,19 @@ pnv_powersave_common:
>>  	mtmsrd	r7,0
>>  	bctr
>>  
>> +/*
>> + * This is the sequence required to execute idle instructions, as
>> + * specified in ISA v2.07. MSR[IR] and MSR[DR] must be 0.
>> + */
>> +#define ARCH207_IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST)		\
>
> We had to do this sequence on POWER7 also, which is architecture
> v2.06.  Thus the comments and the naming (ARCH207_*) are a bit
> misleading here.  The actual code change looks OK.

I'll just drop the name change, I don't think it's crucial. That makes
P9 the special case.

We can come up with a better name or something in future.

Unless Nick objects?

cheers
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Ellerman Aug. 31, 2017, 11:36 a.m. | #3
On Fri, 2017-08-25 at 04:30:34 UTC, Nicholas Piggin wrote:
> Reviewed-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/56ee52408ed0bd4af400c04ad60f98

cheers
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/powerpc/include/asm/cpuidle.h b/arch/powerpc/include/asm/cpuidle.h
index 8a174cba5567..eb43b5c3a7b5 100644
--- a/arch/powerpc/include/asm/cpuidle.h
+++ b/arch/powerpc/include/asm/cpuidle.h
@@ -101,20 +101,4 @@  static inline void report_invalid_psscr_val(u64 psscr_val, int err)
 
 #endif
 
-/* Idle state entry routines */
-#ifdef	CONFIG_PPC_P7_NAP
-#define IDLE_STATE_ENTER_SEQ(IDLE_INST)                         \
-	/* Magic NAP/SLEEP/WINKLE mode enter sequence */	\
-	std	r0,0(r1);					\
-	ptesync;						\
-	ld	r0,0(r1);					\
-236:	cmpd	cr0,r0,r0;					\
-	bne	236b;						\
-	IDLE_INST;						\
-
-#define	IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST)			\
-	IDLE_STATE_ENTER_SEQ(IDLE_INST)                         \
-	b	.
-#endif /* CONFIG_PPC_P7_NAP */
-
 #endif
diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 4924647d964d..14e97f442167 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -205,6 +205,19 @@  pnv_powersave_common:
 	mtmsrd	r7,0
 	bctr
 
+/*
+ * This is the sequence required to execute idle instructions, as
+ * specified in ISA v2.07. MSR[IR] and MSR[DR] must be 0.
+ */
+#define ARCH207_IDLE_STATE_ENTER_SEQ_NORET(IDLE_INST)		\
+	/* Magic NAP/SLEEP/WINKLE mode enter sequence */	\
+	std	r0,0(r1);					\
+	ptesync;						\
+	ld	r0,0(r1);					\
+236:	cmpd	cr0,r0,r0;					\
+	bne	236b;						\
+	IDLE_INST;
+
 	.globl pnv_enter_arch207_idle_mode
 pnv_enter_arch207_idle_mode:
 #ifdef CONFIG_KVM_BOOK3S_HV_POSSIBLE
@@ -230,7 +243,7 @@  pnv_enter_arch207_idle_mode:
 	stb	r3,PACA_THREAD_IDLE_STATE(r13)
 	cmpwi	cr3,r3,PNV_THREAD_SLEEP
 	bge	cr3,2f
-	IDLE_STATE_ENTER_SEQ_NORET(PPC_NAP)
+	ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_NAP)
 	/* No return */
 2:
 	/* Sleep or winkle */
@@ -269,7 +282,7 @@  pnv_fastsleep_workaround_at_entry:
 
 common_enter: /* common code for all the threads entering sleep or winkle */
 	bgt	cr3,enter_winkle
-	IDLE_STATE_ENTER_SEQ_NORET(PPC_SLEEP)
+	ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_SLEEP)
 
 fastsleep_workaround_at_entry:
 	oris	r15,r15,PNV_CORE_IDLE_LOCK_BIT@h
@@ -291,7 +304,7 @@  fastsleep_workaround_at_entry:
 enter_winkle:
 	bl	save_sprs_to_stack
 
-	IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
+	ARCH207_IDLE_STATE_ENTER_SEQ_NORET(PPC_WINKLE)
 
 /*
  * r3 - PSSCR value corresponding to the requested stop state.
@@ -316,7 +329,7 @@  power_enter_stop:
 	andis.   r4,r3,PSSCR_EC_ESL_MASK_SHIFTED
 	clrldi   r3,r3,60 /* r3 = Bits[60:63] = Requested Level (RL) */
 	bne	 .Lhandle_esl_ec_set
-	IDLE_STATE_ENTER_SEQ(PPC_STOP)
+	PPC_STOP
 	li	r3,0  /* Since we didn't lose state, return 0 */
 
 	/*
@@ -349,7 +362,8 @@  power_enter_stop:
 	ld	r4,ADDROFF(pnv_first_deep_stop_state)(r5)
 	cmpd	r3,r4
 	bge	.Lhandle_deep_stop
-	IDLE_STATE_ENTER_SEQ_NORET(PPC_STOP)
+	PPC_STOP	/* Does not return (system reset interrupt) */
+
 .Lhandle_deep_stop:
 /*
  * Entering deep idle state.
@@ -371,7 +385,7 @@  lwarx_loop_stop:
 
 	bl	save_sprs_to_stack
 
-	IDLE_STATE_ENTER_SEQ_NORET(PPC_STOP)
+	PPC_STOP	/* Does not return (system reset interrupt) */
 
 /*
  * Entered with MSR[EE]=0 and no soft-masked interrupts pending.