diff mbox

powernv: Load correct TOC pointer while waking up from winkle.

Message ID 147040459202.19770.16560483806363144997.stgit@jupiter.in.ibm.com (mailing list archive)
State Accepted
Headers show

Commit Message

Mahesh J Salgaonkar Aug. 5, 2016, 1:43 p.m. UTC 
From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

The function pnv_restore_hyp_resource() loads the TOC into r2 from
the invalid PACA pointer before fixing r13 value. This do not affect
POWER ISA 3.0 but it does have an impact on POWER ISA 2.07 or less
leading CPU to get stuck forever.

	login: [  471.830433] Processor 120 is stuck.


This can be easily reproducible using following steps:
- Turn off SMT
	$ ppc64_cpu --smt=off
- offline/online any online cpu (Thread 0 of any core which is online)
	$ echo 0 > /sys/devices/system/cpu/cpu<num>/online
	$ echo 1 > /sys/devices/system/cpu/cpu<num>/online

For POWER ISA 2.07 or less, the last bit of HSPRG0 is set indicating
that thread is waking up from winkle. Hence, the last bit of HSPRG0(r13)
needs to be clear before accessing it as PACA to avoid loading invalid
values from invalid PACA pointer.

Fix this by loading TOC after r13 register is corrected.

Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/idle_book3s.S |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Benjamin Herrenschmidt Aug. 5, 2016, 10:38 p.m. UTC | #1 
On Fri, 2016-08-05 at 19:13 +0530, Mahesh J Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> 
> The function pnv_restore_hyp_resource() loads the TOC into r2 from
> the invalid PACA pointer before fixing r13 value. This do not affect
> POWER ISA 3.0 but it does have an impact on POWER ISA 2.07 or less
> leading CPU to get stuck forever.

When was this broken ? Should this get backported to stable ?

> 	login: [  471.830433] Processor 120 is stuck.
> 
> 
> This can be easily reproducible using following steps:
> - Turn off SMT
> 	$ ppc64_cpu --smt=off
> - offline/online any online cpu (Thread 0 of any core which is
> online)
> 	$ echo 0 > /sys/devices/system/cpu/cpu<num>/online
> 	$ echo 1 > /sys/devices/system/cpu/cpu<num>/online
> 
> For POWER ISA 2.07 or less, the last bit of HSPRG0 is set indicating
> that thread is waking up from winkle. Hence, the last bit of
> HSPRG0(r13)
> needs to be clear before accessing it as PACA to avoid loading
> invalid
> values from invalid PACA pointer.
> 
> Fix this by loading TOC after r13 register is corrected.
> 
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/idle_book3s.S |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/idle_book3s.S
> b/arch/powerpc/kernel/idle_book3s.S
> index 8a56a51..45784ec 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -363,8 +363,8 @@ _GLOBAL(power9_idle_stop)
>   * cr3 - set to gt if waking up with partial/complete hypervisor
> state loss
>   */
>  _GLOBAL(pnv_restore_hyp_resource)
> -	ld	r2,PACATOC(r13);
>  BEGIN_FTR_SECTION
> +	ld	r2,PACATOC(r13);
>  	/*
>  	 * POWER ISA 3. Use PSSCR to determine if we
>  	 * are waking up from deep idle state
> @@ -395,6 +395,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
>  	 */
>  	clrldi	r5,r13,63
>  	clrrdi	r13,r13,1
> +
> +	/* Now that we are sure r13 is corrected, load TOC */
> +	ld	r2,PACATOC(r13);
>  	cmpwi	cr4,r5,1
>  	mtspr	SPRN_HSPRG0,r13
>
Mahesh J Salgaonkar Aug. 6, 2016, 3 a.m. UTC | #2 
On 08/06/2016 04:08 AM, Benjamin Herrenschmidt wrote:
> On Fri, 2016-08-05 at 19:13 +0530, Mahesh J Salgaonkar wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> The function pnv_restore_hyp_resource() loads the TOC into r2 from
>> the invalid PACA pointer before fixing r13 value. This do not affect
>> POWER ISA 3.0 but it does have an impact on POWER ISA 2.07 or less
>> leading CPU to get stuck forever.
> 
> When was this broken ? Should this get backported to stable ?

This is broken with recent Power9 cpu idle changes (commit bcef83a00)
that gone in Linus' master after V4.7. We are fine with v4.7

-Mahesh.

> 
>> 	login: [  471.830433] Processor 120 is stuck.
>>
>>
>> This can be easily reproducible using following steps:
>> - Turn off SMT
>> 	$ ppc64_cpu --smt=off
>> - offline/online any online cpu (Thread 0 of any core which is
>> online)
>> 	$ echo 0 > /sys/devices/system/cpu/cpu<num>/online
>> 	$ echo 1 > /sys/devices/system/cpu/cpu<num>/online
>>
>> For POWER ISA 2.07 or less, the last bit of HSPRG0 is set indicating
>> that thread is waking up from winkle. Hence, the last bit of
>> HSPRG0(r13)
>> needs to be clear before accessing it as PACA to avoid loading
>> invalid
>> values from invalid PACA pointer.
>>
>> Fix this by loading TOC after r13 register is corrected.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/kernel/idle_book3s.S |    5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kernel/idle_book3s.S
>> b/arch/powerpc/kernel/idle_book3s.S
>> index 8a56a51..45784ec 100644
>> --- a/arch/powerpc/kernel/idle_book3s.S
>> +++ b/arch/powerpc/kernel/idle_book3s.S
>> @@ -363,8 +363,8 @@ _GLOBAL(power9_idle_stop)
>>   * cr3 - set to gt if waking up with partial/complete hypervisor
>> state loss
>>   */
>>  _GLOBAL(pnv_restore_hyp_resource)
>> -	ld	r2,PACATOC(r13);
>>  BEGIN_FTR_SECTION
>> +	ld	r2,PACATOC(r13);
>>  	/*
>>  	 * POWER ISA 3. Use PSSCR to determine if we
>>  	 * are waking up from deep idle state
>> @@ -395,6 +395,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
>>  	 */
>>  	clrldi	r5,r13,63
>>  	clrrdi	r13,r13,1
>> +
>> +	/* Now that we are sure r13 is corrected, load TOC */
>> +	ld	r2,PACATOC(r13);
>>  	cmpwi	cr4,r5,1
>>  	mtspr	SPRN_HSPRG0,r13
>>  
>
Vaidyanathan Srinivasan Aug. 8, 2016, 6:39 a.m. UTC | #3 
* Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> [2016-08-05 19:13:12]:

> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> 
> The function pnv_restore_hyp_resource() loads the TOC into r2 from
> the invalid PACA pointer before fixing r13 value. This do not affect
> POWER ISA 3.0 but it does have an impact on POWER ISA 2.07 or less
> leading CPU to get stuck forever.
> 
> 	login: [  471.830433] Processor 120 is stuck.
> 
> 
> This can be easily reproducible using following steps:
> - Turn off SMT
> 	$ ppc64_cpu --smt=off
> - offline/online any online cpu (Thread 0 of any core which is online)
> 	$ echo 0 > /sys/devices/system/cpu/cpu<num>/online
> 	$ echo 1 > /sys/devices/system/cpu/cpu<num>/online
> 
> For POWER ISA 2.07 or less, the last bit of HSPRG0 is set indicating
> that thread is waking up from winkle. Hence, the last bit of HSPRG0(r13)
> needs to be clear before accessing it as PACA to avoid loading invalid
> values from invalid PACA pointer.
> 
> Fix this by loading TOC after r13 register is corrected.
> 
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

Acked-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>


> ---
>  arch/powerpc/kernel/idle_book3s.S |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 8a56a51..45784ec 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -363,8 +363,8 @@ _GLOBAL(power9_idle_stop)
>   * cr3 - set to gt if waking up with partial/complete hypervisor state loss
>   */
>  _GLOBAL(pnv_restore_hyp_resource)
> -	ld	r2,PACATOC(r13);
>  BEGIN_FTR_SECTION
> +	ld	r2,PACATOC(r13);
>  	/*
>  	 * POWER ISA 3. Use PSSCR to determine if we
>  	 * are waking up from deep idle state
> @@ -395,6 +395,9 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
>  	 */
>  	clrldi	r5,r13,63
>  	clrrdi	r13,r13,1
> +
> +	/* Now that we are sure r13 is corrected, load TOC */
> +	ld	r2,PACATOC(r13);
>  	cmpwi	cr4,r5,1
>  	mtspr	SPRN_HSPRG0,r13
> 

Thanks Mahesh for this fix.

--Vaidy
Vaidyanathan Srinivasan Aug. 8, 2016, 7 a.m. UTC | #4 
* Benjamin Herrenschmidt <benh@kernel.crashing.org> [2016-08-06 08:38:53]:

> On Fri, 2016-08-05 at 19:13 +0530, Mahesh J Salgaonkar wrote:
> > From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> > 
> > The function pnv_restore_hyp_resource() loads the TOC into r2 from
> > the invalid PACA pointer before fixing r13 value. This do not affect
> > POWER ISA 3.0 but it does have an impact on POWER ISA 2.07 or less
> > leading CPU to get stuck forever.
> 
> When was this broken ? Should this get backported to stable ?

Broken for 4.8-rc1 only after moving the SPRN_HSPRG0 checking to
idle_book3s.S. Hence no back ports needed.

--Vaidy
Michael Ellerman Aug. 9, 2016, 11:26 a.m. UTC | #5 
On Fri, 2016-05-08 at 13:43:12 UTC, Mahesh Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> 
> The function pnv_restore_hyp_resource() loads the TOC into r2 from
> the invalid PACA pointer before fixing r13 value. This do not affect
> POWER ISA 3.0 but it does have an impact on POWER ISA 2.07 or less
> leading CPU to get stuck forever.
> 
> 	login: [  471.830433] Processor 120 is stuck.
> 
> 
> This can be easily reproducible using following steps:
> - Turn off SMT
> 	$ ppc64_cpu --smt=off
> - offline/online any online cpu (Thread 0 of any core which is online)
> 	$ echo 0 > /sys/devices/system/cpu/cpu<num>/online
> 	$ echo 1 > /sys/devices/system/cpu/cpu<num>/online
> 
> For POWER ISA 2.07 or less, the last bit of HSPRG0 is set indicating
> that thread is waking up from winkle. Hence, the last bit of HSPRG0(r13)
> needs to be clear before accessing it as PACA to avoid loading invalid
> values from invalid PACA pointer.
> 
> Fix this by loading TOC after r13 register is corrected.
> 
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

Applied to powerpc fixes, thanks.

https://git.kernel.org/powerpc/c/e325d76f8bd2d222a1f577aba0

cheers
diff mbox

Patch

diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
index 8a56a51..45784ec 100644
--- a/arch/powerpc/kernel/idle_book3s.S
+++ b/arch/powerpc/kernel/idle_book3s.S
@@ -363,8 +363,8 @@  _GLOBAL(power9_idle_stop)
  * cr3 - set to gt if waking up with partial/complete hypervisor state loss
  */
 _GLOBAL(pnv_restore_hyp_resource)
-	ld	r2,PACATOC(r13);
 BEGIN_FTR_SECTION
+	ld	r2,PACATOC(r13);
 	/*
 	 * POWER ISA 3. Use PSSCR to determine if we
 	 * are waking up from deep idle state
@@ -395,6 +395,9 @@  END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
 	 */
 	clrldi	r5,r13,63
 	clrrdi	r13,r13,1
+
+	/* Now that we are sure r13 is corrected, load TOC */
+	ld	r2,PACATOC(r13);
 	cmpwi	cr4,r5,1
 	mtspr	SPRN_HSPRG0,r13