linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powernv: Load correct TOC pointer while waking up from winkle.
@ 2016-08-05 13:43 Mahesh J Salgaonkar
  2016-08-05 22:38 ` Benjamin Herrenschmidt
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Mahesh J Salgaonkar @ 2016-08-05 13:43 UTC (permalink / raw)
  To: linuxppc-dev, Michael Ellerman
  Cc: Benjamin Herrenschmidt, Vaidyanathan Srinivasan, Paul Mackerras,
	Gautham R. Shenoy

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(-)

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
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] powernv: Load correct TOC pointer while waking up from winkle.
  2016-08-05 13:43 [PATCH] powernv: Load correct TOC pointer while waking up from winkle Mahesh J Salgaonkar
@ 2016-08-05 22:38 ` Benjamin Herrenschmidt
  2016-08-06  3:00   ` Mahesh Jagannath Salgaonkar
  2016-08-08  7:00   ` Vaidyanathan Srinivasan
  2016-08-08  6:39 ` Vaidyanathan Srinivasan
  2016-08-09 11:26 ` Michael Ellerman
  2 siblings, 2 replies; 6+ messages in thread
From: Benjamin Herrenschmidt @ 2016-08-05 22:38 UTC (permalink / raw)
  To: Mahesh J Salgaonkar, linuxppc-dev, Michael Ellerman
  Cc: Vaidyanathan Srinivasan, Paul Mackerras, Gautham R. Shenoy

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
>  

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powernv: Load correct TOC pointer while waking up from winkle.
  2016-08-05 22:38 ` Benjamin Herrenschmidt
@ 2016-08-06  3:00   ` Mahesh Jagannath Salgaonkar
  2016-08-08  7:00   ` Vaidyanathan Srinivasan
  1 sibling, 0 replies; 6+ messages in thread
From: Mahesh Jagannath Salgaonkar @ 2016-08-06  3:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, linuxppc-dev, Michael Ellerman
  Cc: Gautham R. Shenoy, Paul Mackerras

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
>>  
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powernv: Load correct TOC pointer while waking up from winkle.
  2016-08-05 13:43 [PATCH] powernv: Load correct TOC pointer while waking up from winkle Mahesh J Salgaonkar
  2016-08-05 22:38 ` Benjamin Herrenschmidt
@ 2016-08-08  6:39 ` Vaidyanathan Srinivasan
  2016-08-09 11:26 ` Michael Ellerman
  2 siblings, 0 replies; 6+ messages in thread
From: Vaidyanathan Srinivasan @ 2016-08-08  6:39 UTC (permalink / raw)
  To: Mahesh J Salgaonkar
  Cc: linuxppc-dev, Michael Ellerman, Benjamin Herrenschmidt,
	Paul Mackerras, Gautham R. Shenoy

* 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

 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] powernv: Load correct TOC pointer while waking up from winkle.
  2016-08-05 22:38 ` Benjamin Herrenschmidt
  2016-08-06  3:00   ` Mahesh Jagannath Salgaonkar
@ 2016-08-08  7:00   ` Vaidyanathan Srinivasan
  1 sibling, 0 replies; 6+ messages in thread
From: Vaidyanathan Srinivasan @ 2016-08-08  7:00 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: Mahesh J Salgaonkar, linuxppc-dev, Michael Ellerman,
	Paul Mackerras, Gautham R. Shenoy

* 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: powernv: Load correct TOC pointer while waking up from winkle.
  2016-08-05 13:43 [PATCH] powernv: Load correct TOC pointer while waking up from winkle Mahesh J Salgaonkar
  2016-08-05 22:38 ` Benjamin Herrenschmidt
  2016-08-08  6:39 ` Vaidyanathan Srinivasan
@ 2016-08-09 11:26 ` Michael Ellerman
  2 siblings, 0 replies; 6+ messages in thread
From: Michael Ellerman @ 2016-08-09 11:26 UTC (permalink / raw)
  To: Mahesh Salgaonkar, linuxppc-dev; +Cc: Paul Mackerras, Gautham R. Shenoy

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-08-09 11:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-05 13:43 [PATCH] powernv: Load correct TOC pointer while waking up from winkle Mahesh J Salgaonkar
2016-08-05 22:38 ` Benjamin Herrenschmidt
2016-08-06  3:00   ` Mahesh Jagannath Salgaonkar
2016-08-08  7:00   ` Vaidyanathan Srinivasan
2016-08-08  6:39 ` Vaidyanathan Srinivasan
2016-08-09 11:26 ` Michael Ellerman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).