[RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.

[RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.

[Mahesh J Salgaonkar]

From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>

With the powerpc next commit e7e81847478 (powerpc/mce: Fix SLB rebolting
during MCE recovery path.), the SLB error recovery is broken. The new
change now does not add index value to RB[52-63] that selects the SLB
entry while rebolting, instead it assumes that the shadow save area
already have index embeded correctly in esid field. While all valid bolted
save areas do contain index value set correctly, there is a case where
3rd (KSTACK_INDEX) entry for kernel stack does not embed index for NULL
esid entry. This patch fixes that.

Without this patch the SLB rebolt code overwrites the 1st entry of kernel
linear mapping and causes SLB recovery to fail.

Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/mm/slb.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
index 0b095fa54049..9f574e59d178 100644
--- a/arch/powerpc/mm/slb.c
+++ b/arch/powerpc/mm/slb.c
@@ -70,7 +70,7 @@ static inline void slb_shadow_update(unsigned long ea, int ssize,
 
 static inline void slb_shadow_clear(enum slb_index index)
 {
-	WRITE_ONCE(get_slb_shadow()->save_area[index].esid, 0);
+	WRITE_ONCE(get_slb_shadow()->save_area[index].esid, cpu_to_be64(index));
 }
 
 static inline void create_shadowed_slbe(unsigned long ea, int ssize,

Re: [RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.

[Michael Ellerman]

Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> writes:

> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>
> With the powerpc next commit e7e81847478 (powerpc/mce: Fix SLB rebolting
> during MCE recovery path.),

That commit description is wrong, I'll fix it up.

cheers

> the SLB error recovery is broken. The new
> change now does not add index value to RB[52-63] that selects the SLB
> entry while rebolting, instead it assumes that the shadow save area
> already have index embeded correctly in esid field. While all valid bolted
> save areas do contain index value set correctly, there is a case where
> 3rd (KSTACK_INDEX) entry for kernel stack does not embed index for NULL
> esid entry. This patch fixes that.
>
> Without this patch the SLB rebolt code overwrites the 1st entry of kernel
> linear mapping and causes SLB recovery to fail.
>
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  arch/powerpc/mm/slb.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> index 0b095fa54049..9f574e59d178 100644
> --- a/arch/powerpc/mm/slb.c
> +++ b/arch/powerpc/mm/slb.c
> @@ -70,7 +70,7 @@ static inline void slb_shadow_update(unsigned long ea, int ssize,
>  
>  static inline void slb_shadow_clear(enum slb_index index)
>  {
> -	WRITE_ONCE(get_slb_shadow()->save_area[index].esid, 0);
> +	WRITE_ONCE(get_slb_shadow()->save_area[index].esid, cpu_to_be64(index));
>  }
>  
>  static inline void create_shadowed_slbe(unsigned long ea, int ssize,

Re: [RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.

[Mahesh Jagannath Salgaonkar]

On 08/23/2018 12:14 PM, Michael Ellerman wrote:
> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
> 
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> With the powerpc next commit e7e81847478 (powerpc/mce: Fix SLB rebolting
>> during MCE recovery path.),
> 
> That commit description is wrong, I'll fix it up.

Ouch.. My bad.. :-(

> 
> cheers
> 
>> the SLB error recovery is broken. The new
>> change now does not add index value to RB[52-63] that selects the SLB
>> entry while rebolting, instead it assumes that the shadow save area
>> already have index embeded correctly in esid field. While all valid bolted
>> save areas do contain index value set correctly, there is a case where
>> 3rd (KSTACK_INDEX) entry for kernel stack does not embed index for NULL
>> esid entry. This patch fixes that.
>>
>> Without this patch the SLB rebolt code overwrites the 1st entry of kernel
>> linear mapping and causes SLB recovery to fail.
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  arch/powerpc/mm/slb.c |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
>> index 0b095fa54049..9f574e59d178 100644
>> --- a/arch/powerpc/mm/slb.c
>> +++ b/arch/powerpc/mm/slb.c
>> @@ -70,7 +70,7 @@ static inline void slb_shadow_update(unsigned long ea, int ssize,
>>  
>>  static inline void slb_shadow_clear(enum slb_index index)
>>  {
>> -	WRITE_ONCE(get_slb_shadow()->save_area[index].esid, 0);
>> +	WRITE_ONCE(get_slb_shadow()->save_area[index].esid, cpu_to_be64(index));
>>  }
>>  
>>  static inline void create_shadowed_slbe(unsigned long ea, int ssize,
> 

Re: [RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.

[Michael Ellerman]

Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> writes:

> On 08/23/2018 12:14 PM, Michael Ellerman wrote:
>> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
>> 
>>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>>
>>> With the powerpc next commit e7e81847478 (powerpc/mce: Fix SLB rebolting
>>> during MCE recovery path.),
>> 
>> That commit description is wrong, I'll fix it up.
>
> Ouch.. My bad.. :-(

To make it easier to get right, if you don't already, add these to your
~/.gitconfig:

[pretty]
	fixes = Fixes: %h (\"%s\")
	quote = %h (\"%s\")


And then you can do:

$ git log -1 --pretty=quote e7e81847478 
e7e81847478b ("powerpc/64s: move machine check SLB flushing to mm/slb.c")

$ git log -1 --pretty=fixes e7e81847478 
Fixes: e7e81847478b ("powerpc/64s: move machine check SLB flushing to mm/slb.c")


I then have a shell alias that pipes that into xclip, so I can just
paste it directly into the change log.

cheers

Re: [RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.

[Mahesh Jagannath Salgaonkar]

On 08/23/2018 05:35 PM, Michael Ellerman wrote:
> Mahesh Jagannath Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
> 
>> On 08/23/2018 12:14 PM, Michael Ellerman wrote:
>>> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> writes:
>>>
>>>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>>>
>>>> With the powerpc next commit e7e81847478 (powerpc/mce: Fix SLB rebolting
>>>> during MCE recovery path.),
>>>
>>> That commit description is wrong, I'll fix it up.
>>
>> Ouch.. My bad.. :-(
> 
> To make it easier to get right, if you don't already, add these to your
> ~/.gitconfig:
> 
> [pretty]
> 	fixes = Fixes: %h (\"%s\")
> 	quote = %h (\"%s\")
> 
> 
> And then you can do:
> 
> $ git log -1 --pretty=quote e7e81847478 
> e7e81847478b ("powerpc/64s: move machine check SLB flushing to mm/slb.c")
> 
> $ git log -1 --pretty=fixes e7e81847478 
> Fixes: e7e81847478b ("powerpc/64s: move machine check SLB flushing to mm/slb.c")

Thank you very much :-) This is going to be very handy...

-Mahesh.

Re: [RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.

[Nicholas Piggin]

On Thu, 23 Aug 2018 12:06:53 +0530
Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:

> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> 
> With the powerpc next commit e7e81847478 (powerpc/mce: Fix SLB rebolting
> during MCE recovery path.), the SLB error recovery is broken. The new
> change now does not add index value to RB[52-63] that selects the SLB
> entry while rebolting, instead it assumes that the shadow save area
> already have index embeded correctly in esid field. While all valid bolted
> save areas do contain index value set correctly, there is a case where
> 3rd (KSTACK_INDEX) entry for kernel stack does not embed index for NULL
> esid entry. This patch fixes that.
> 
> Without this patch the SLB rebolt code overwrites the 1st entry of kernel
> linear mapping and causes SLB recovery to fail.
> 
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Changelog just needs a little more work, maybe this?


The commit e7e81847478 ("powerpc/64s: move machine check SLB flushing
to mm/slb.c") introduced a bug in reloading bolted SLB entries. Unused
bolted entries are stored with .esid=0 in the slb_shadow area, and
that value is now used directly as the RB input to slbmte, which means
the RB[52:63] index field is set to 0, which causes SLB entry 0 to be
cleared.

Fix this by storing the index bits in the unused bolted entries, which
directs the slbmte to the right place.

The SLB shadow area is also used by the hypervisor, but PAPR is okay
with that, from LoPAPR v1.1, 14.11.1.3 SLB Shadow Buffer:

  Note: SLB is filled sequentially starting at index 0
  from the shadow buffer ignoring the contents of
  RB field bits 52-63

Fixes: e7e81847478 ("powerpc/64s: move machine check SLB flushing to
mm/slb.c")

Thanks,
Nick

> ---
>  arch/powerpc/mm/slb.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/slb.c b/arch/powerpc/mm/slb.c
> index 0b095fa54049..9f574e59d178 100644
> --- a/arch/powerpc/mm/slb.c
> +++ b/arch/powerpc/mm/slb.c
> @@ -70,7 +70,7 @@ static inline void slb_shadow_update(unsigned long ea, int ssize,
>  
>  static inline void slb_shadow_clear(enum slb_index index)
>  {
> -	WRITE_ONCE(get_slb_shadow()->save_area[index].esid, 0);
> +	WRITE_ONCE(get_slb_shadow()->save_area[index].esid, cpu_to_be64(index));
>  }
>  
>  static inline void create_shadowed_slbe(unsigned long ea, int ssize,
> 

Re: [RESEND PATCH v2] powerpc/mce: Fix SLB rebolting during MCE recovery path.

[Michael Ellerman]

Nicholas Piggin <npiggin@gmail.com> writes:
> On Thu, 23 Aug 2018 12:06:53 +0530
> Mahesh J Salgaonkar <mahesh@linux.vnet.ibm.com> wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> 
>> With the powerpc next commit e7e81847478 (powerpc/mce: Fix SLB rebolting
>> during MCE recovery path.), the SLB error recovery is broken. The new
>> change now does not add index value to RB[52-63] that selects the SLB
>> entry while rebolting, instead it assumes that the shadow save area
>> already have index embeded correctly in esid field. While all valid bolted
>> save areas do contain index value set correctly, there is a case where
>> 3rd (KSTACK_INDEX) entry for kernel stack does not embed index for NULL
>> esid entry. This patch fixes that.
>> 
>> Without this patch the SLB rebolt code overwrites the 1st entry of kernel
>> linear mapping and causes SLB recovery to fail.
>> 
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Changelog just needs a little more work, maybe this?
>
>
> The commit e7e81847478 ("powerpc/64s: move machine check SLB flushing
> to mm/slb.c") introduced a bug in reloading bolted SLB entries. Unused
> bolted entries are stored with .esid=0 in the slb_shadow area, and
> that value is now used directly as the RB input to slbmte, which means
> the RB[52:63] index field is set to 0, which causes SLB entry 0 to be
> cleared.
>
> Fix this by storing the index bits in the unused bolted entries, which
> directs the slbmte to the right place.
>
> The SLB shadow area is also used by the hypervisor, but PAPR is okay
> with that, from LoPAPR v1.1, 14.11.1.3 SLB Shadow Buffer:
>
>   Note: SLB is filled sequentially starting at index 0
>   from the shadow buffer ignoring the contents of
>   RB field bits 52-63
>
> Fixes: e7e81847478 ("powerpc/64s: move machine check SLB flushing to
> mm/slb.c")

Yep I'll take that, thanks.

cheers