[3/6] powernv:idle: Restore LPCR on wakeup from deep-stop

Submitted by Gautham R. Shenoy on May 16, 2017, 8:49 a.m.

Details

Message ID 147a42823bae78b98b76b91f736c620b886213ae.1494585671.git.ego@linux.vnet.ibm.com
State Accepted
Commit cb0be7ec03077a31712183bfbe7801061e2966b8
Headers show

Commit Message

Gautham R. Shenoy May 16, 2017, 8:49 a.m.
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>

On wakeup from a deep stop state which is supposed to lose the
hypervisor state, we don't restore the LPCR to the old value but set
it to a "sane" value via cur_cpu_spec->cpu_restore().

The problem is that the "sane" value doesn't include UPRT and the HR
bits which are required to run correctly in Radix mode.

Fix this on POWER9 onwards by restoring the LPCR value whatever it was
before executing the stop instruction.

Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/idle_book3s.S | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Nicholas Piggin May 30, 2017, 6:17 a.m.
On Tue, 16 May 2017 14:19:45 +0530
"Gautham R. Shenoy" <ego@linux.vnet.ibm.com> wrote:

> From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> 
> On wakeup from a deep stop state which is supposed to lose the
> hypervisor state, we don't restore the LPCR to the old value but set
> it to a "sane" value via cur_cpu_spec->cpu_restore().
> 
> The problem is that the "sane" value doesn't include UPRT and the HR
> bits which are required to run correctly in Radix mode.
> 
> Fix this on POWER9 onwards by restoring the LPCR value whatever it was
> before executing the stop instruction.
> 
> Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>

Yes I think we need this. I have a plan to rework some of
that cpu_restore and early CPU init stuff, but for now we
need this.

Does the OCC restore LPCR properly then we just trash it
with ->cpu_restore(), or is it always junk?

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>


> ---
>  arch/powerpc/kernel/idle_book3s.S | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index afd029f..6c9920d 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -31,6 +31,7 @@
>   * registers for winkle support.
>   */
>  #define _SDR1	GPR3
> +#define _PTCR	GPR3
>  #define _RPR	GPR4
>  #define _SPURR	GPR5
>  #define _PURR	GPR6
> @@ -39,7 +40,7 @@
>  #define _AMOR	GPR9
>  #define _WORT	GPR10
>  #define _WORC	GPR11
> -#define _PTCR	GPR12
> +#define _LPCR	GPR12
>  
>  #define PSSCR_EC_ESL_MASK_SHIFTED          (PSSCR_EC | PSSCR_ESL) >> 16
>  
> @@ -55,12 +56,14 @@ save_sprs_to_stack:
>  	 * here since any thread in the core might wake up first
>  	 */
>  BEGIN_FTR_SECTION
> -	mfspr	r3,SPRN_PTCR
> -	std	r3,_PTCR(r1)
>  	/*
>  	 * Note - SDR1 is dropped in Power ISA v3. Hence not restoring
>  	 * SDR1 here
>  	 */
> +	mfspr	r3,SPRN_PTCR
> +	std	r3,_PTCR(r1)
> +	mfspr	r3,SPRN_LPCR
> +	std	r3,_LPCR(r1)
>  FTR_SECTION_ELSE
>  	mfspr	r3,SPRN_SDR1
>  	std	r3,_SDR1(r1)
> @@ -813,6 +816,10 @@ no_segments:
>  	mtctr	r12
>  	bctrl
>  
> +BEGIN_FTR_SECTION
> +	ld	r4,_LPCR(r1)
> +	mtspr	SPRN_LPCR,r4
> +END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
>  hypervisor_state_restored:
>  
>  	mtspr	SPRN_SRR1,r16
Gautham R. Shenoy May 30, 2017, 10:35 a.m.
On Tue, May 30, 2017 at 04:17:31PM +1000, Nicholas Piggin wrote:
> On Tue, 16 May 2017 14:19:45 +0530
> "Gautham R. Shenoy" <ego@linux.vnet.ibm.com> wrote:
> 
> > From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
> > 
> > On wakeup from a deep stop state which is supposed to lose the
> > hypervisor state, we don't restore the LPCR to the old value but set
> > it to a "sane" value via cur_cpu_spec->cpu_restore().
> > 
> > The problem is that the "sane" value doesn't include UPRT and the HR
> > bits which are required to run correctly in Radix mode.
> > 
> > Fix this on POWER9 onwards by restoring the LPCR value whatever it was
> > before executing the stop instruction.
> > 
> > Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
> 
> Yes I think we need this. I have a plan to rework some of
> that cpu_restore and early CPU init stuff, but for now we
> need this.
> 
> Does the OCC restore LPCR properly then we just trash it
> with ->cpu_restore(), or is it always junk?

The microcode restores LPCR to the value that the kernel asks it to
set to in pnv_save_sprs_for_deep_states() via the opal_slw_set_reg. So
it would be some sane state.

However when we return from stop to either the cpuidle or cpuhotplug,
we want LPCR to be restored to the value it hand when it entered stop.


> 
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> 
>

Thanks for the review.

--
Thanks and Regards
gautham.

Patch hide | download patch | download mbox

diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index afd029f..6c9920d 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -31,6 +31,7 @@ 
  * registers for winkle support.
  */
 #define _SDR1	GPR3
+#define _PTCR	GPR3
 #define _RPR	GPR4
 #define _SPURR	GPR5
 #define _PURR	GPR6
@@ -39,7 +40,7 @@ 
 #define _AMOR	GPR9
 #define _WORT	GPR10
 #define _WORC	GPR11
-#define _PTCR	GPR12
+#define _LPCR	GPR12
 
 #define PSSCR_EC_ESL_MASK_SHIFTED          (PSSCR_EC | PSSCR_ESL) >> 16
 
@@ -55,12 +56,14 @@  save_sprs_to_stack:
 	 * here since any thread in the core might wake up first
 	 */
 BEGIN_FTR_SECTION
-	mfspr	r3,SPRN_PTCR
-	std	r3,_PTCR(r1)
 	/*
 	 * Note - SDR1 is dropped in Power ISA v3. Hence not restoring
 	 * SDR1 here
 	 */
+	mfspr	r3,SPRN_PTCR
+	std	r3,_PTCR(r1)
+	mfspr	r3,SPRN_LPCR
+	std	r3,_LPCR(r1)
 FTR_SECTION_ELSE
 	mfspr	r3,SPRN_SDR1
 	std	r3,_SDR1(r1)
@@ -813,6 +816,10 @@  no_segments:
 	mtctr	r12
 	bctrl
 
+BEGIN_FTR_SECTION
+	ld	r4,_LPCR(r1)
+	mtspr	SPRN_LPCR,r4
+END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
 hypervisor_state_restored:
 
 	mtspr	SPRN_SRR1,r16