linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32
@ 2019-01-22 14:11 Christophe Leroy
  2019-01-23  8:56 ` Nicholas Piggin
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Christophe Leroy @ 2019-01-22 14:11 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin
  Cc: linux-kernel, linuxppc-dev

Looks like book3s/32 doesn't set RI on machine check, so
checking RI before calling die() will always be fatal
allthought this is not an issue in most cases.

Fixes: b96672dd840f ("powerpc: Machine check interrupt is a non-maskable interrupt")
Fixes: daf00ae71dad ("powerpc/traps: restore recoverability of machine_check interrupts")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: stable@vger.kernel.org
---
 arch/powerpc/kernel/traps.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 64936b60d521..c740f8bfccc9 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -763,15 +763,15 @@ void machine_check_exception(struct pt_regs *regs)
 	if (check_io_access(regs))
 		goto bail;
 
-	/* Must die if the interrupt is not recoverable */
-	if (!(regs->msr & MSR_RI))
-		nmi_panic(regs, "Unrecoverable Machine check");
-
 	if (!nested)
 		nmi_exit();
 
 	die("Machine check", regs, SIGBUS);
 
+	/* Must die if the interrupt is not recoverable */
+	if (!(regs->msr & MSR_RI))
+		nmi_panic(regs, "Unrecoverable Machine check");
+
 	return;
 
 bail:
-- 
2.13.3


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

* Re: [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32
  2019-01-22 14:11 [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32 Christophe Leroy
@ 2019-01-23  8:56 ` Nicholas Piggin
  2019-02-22  9:47 ` Michael Ellerman
  2020-09-11  9:15 ` [PATCH] " Michal Suchánek
  2 siblings, 0 replies; 7+ messages in thread
From: Nicholas Piggin @ 2019-01-23  8:56 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
	Paul Mackerras
  Cc: linux-kernel, linuxppc-dev

Christophe Leroy's on January 23, 2019 12:11 am:
> Looks like book3s/32 doesn't set RI on machine check, so
> checking RI before calling die() will always be fatal
> allthought this is not an issue in most cases.

Oh good catch, this is a fix for powerpc/64 as well. I think actually 
the panic was supposed to go in the bail: path, it's missing from
there.

But you may have a point, I don't know if we need to panic the box if 
we have !RI in process context... Possible we might be able to remove
those panics and just turn them into die.

Thanks,
Nick

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

* Re: powerpc/traps: fix recoverability of machine check handling on book3s/32
  2019-01-22 14:11 [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32 Christophe Leroy
  2019-01-23  8:56 ` Nicholas Piggin
@ 2019-02-22  9:47 ` Michael Ellerman
  2020-09-11  9:15 ` [PATCH] " Michal Suchánek
  2 siblings, 0 replies; 7+ messages in thread
From: Michael Ellerman @ 2019-02-22  9:47 UTC (permalink / raw)
  To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Nicholas Piggin
  Cc: linuxppc-dev, linux-kernel

On Tue, 2019-01-22 at 14:11:24 UTC, Christophe Leroy wrote:
> Looks like book3s/32 doesn't set RI on machine check, so
> checking RI before calling die() will always be fatal
> allthought this is not an issue in most cases.
> 
> Fixes: b96672dd840f ("powerpc: Machine check interrupt is a non-maskable interrupt")
> Fixes: daf00ae71dad ("powerpc/traps: restore recoverability of machine_check interrupts")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: stable@vger.kernel.org

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/0bbea75c476b77fa7d7811d6be911cc7

cheers

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

* Re: [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32
  2019-01-22 14:11 [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32 Christophe Leroy
  2019-01-23  8:56 ` Nicholas Piggin
  2019-02-22  9:47 ` Michael Ellerman
@ 2020-09-11  9:15 ` Michal Suchánek
  2020-09-11  9:54   ` Christophe Leroy
  2020-09-11 13:23   ` Michael Ellerman
  2 siblings, 2 replies; 7+ messages in thread
From: Michal Suchánek @ 2020-09-11  9:15 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, linuxppc-dev, linux-kernel

Hello,

does this logic apply to "Unrecoverable System Reset" as well?

Thanks

Michal

On Tue, Jan 22, 2019 at 02:11:24PM +0000, Christophe Leroy wrote:
> Looks like book3s/32 doesn't set RI on machine check, so
> checking RI before calling die() will always be fatal
> allthought this is not an issue in most cases.
> 
> Fixes: b96672dd840f ("powerpc: Machine check interrupt is a non-maskable interrupt")
> Fixes: daf00ae71dad ("powerpc/traps: restore recoverability of machine_check interrupts")
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: stable@vger.kernel.org
> ---
>  arch/powerpc/kernel/traps.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 64936b60d521..c740f8bfccc9 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -763,15 +763,15 @@ void machine_check_exception(struct pt_regs *regs)
>  	if (check_io_access(regs))
>  		goto bail;
>  
> -	/* Must die if the interrupt is not recoverable */
> -	if (!(regs->msr & MSR_RI))
> -		nmi_panic(regs, "Unrecoverable Machine check");
> -
>  	if (!nested)
>  		nmi_exit();
>  
>  	die("Machine check", regs, SIGBUS);
>  
> +	/* Must die if the interrupt is not recoverable */
> +	if (!(regs->msr & MSR_RI))
> +		nmi_panic(regs, "Unrecoverable Machine check");
> +
>  	return;
>  
>  bail:
> -- 
> 2.13.3
> 

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

* Re: [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32
  2020-09-11  9:15 ` [PATCH] " Michal Suchánek
@ 2020-09-11  9:54   ` Christophe Leroy
  2020-09-11 13:23   ` Michael Ellerman
  1 sibling, 0 replies; 7+ messages in thread
From: Christophe Leroy @ 2020-09-11  9:54 UTC (permalink / raw)
  To: Michal Suchánek
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Nicholas Piggin, linuxppc-dev, linux-kernel

Hello,

Le 11/09/2020 à 11:15, Michal Suchánek a écrit :
> Hello,
> 
> does this logic apply to "Unrecoverable System Reset" as well?

I don't know, I don't think I have any way the generate a System Reset 
on my board to check it.

Christophe

> 
> Thanks
> 
> Michal
> 
> On Tue, Jan 22, 2019 at 02:11:24PM +0000, Christophe Leroy wrote:
>> Looks like book3s/32 doesn't set RI on machine check, so
>> checking RI before calling die() will always be fatal
>> allthought this is not an issue in most cases.
>>
>> Fixes: b96672dd840f ("powerpc: Machine check interrupt is a non-maskable interrupt")
>> Fixes: daf00ae71dad ("powerpc/traps: restore recoverability of machine_check interrupts")
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> Cc: stable@vger.kernel.org
>> ---
>>   arch/powerpc/kernel/traps.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
>> index 64936b60d521..c740f8bfccc9 100644
>> --- a/arch/powerpc/kernel/traps.c
>> +++ b/arch/powerpc/kernel/traps.c
>> @@ -763,15 +763,15 @@ void machine_check_exception(struct pt_regs *regs)
>>   	if (check_io_access(regs))
>>   		goto bail;
>>   
>> -	/* Must die if the interrupt is not recoverable */
>> -	if (!(regs->msr & MSR_RI))
>> -		nmi_panic(regs, "Unrecoverable Machine check");
>> -
>>   	if (!nested)
>>   		nmi_exit();
>>   
>>   	die("Machine check", regs, SIGBUS);
>>   
>> +	/* Must die if the interrupt is not recoverable */
>> +	if (!(regs->msr & MSR_RI))
>> +		nmi_panic(regs, "Unrecoverable Machine check");
>> +
>>   	return;
>>   
>>   bail:
>> -- 
>> 2.13.3
>>

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

* Re: [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32
  2020-09-11  9:15 ` [PATCH] " Michal Suchánek
  2020-09-11  9:54   ` Christophe Leroy
@ 2020-09-11 13:23   ` Michael Ellerman
  2020-09-14 12:46     ` Michal Suchánek
  1 sibling, 1 reply; 7+ messages in thread
From: Michael Ellerman @ 2020-09-11 13:23 UTC (permalink / raw)
  To: Michal Suchánek, Christophe Leroy
  Cc: Benjamin Herrenschmidt, Paul Mackerras, Nicholas Piggin,
	linuxppc-dev, linux-kernel

Michal Suchánek <msuchanek@suse.de> writes:
> Hello,
>
> does this logic apply to "Unrecoverable System Reset" as well?

Which logic do you mean?

We do call die() before checking MSR_RI in system_reset_exception():

  	/*
  	 * No debugger or crash dump registered, print logs then
  	 * panic.
  	 */
  	die("System Reset", regs, SIGABRT);
  
  	mdelay(2*MSEC_PER_SEC); /* Wait a little while for others to print */
  	add_taint(TAINT_DIE, LOCKDEP_NOW_UNRELIABLE);
  	nmi_panic(regs, "System Reset");
  
  out:
  #ifdef CONFIG_PPC_BOOK3S_64
  	BUG_ON(get_paca()->in_nmi == 0);
  	if (get_paca()->in_nmi > 1)
  		die("Unrecoverable nested System Reset", regs, SIGABRT);
  #endif
  	/* Must die if the interrupt is not recoverable */
  	if (!(regs->msr & MSR_RI))
  		die("Unrecoverable System Reset", regs, SIGABRT);


So you should see the output from die("System Reset", ...) even if
MSR[RI] was clear when you took the system reset.

cheers

> On Tue, Jan 22, 2019 at 02:11:24PM +0000, Christophe Leroy wrote:
>> Looks like book3s/32 doesn't set RI on machine check, so
>> checking RI before calling die() will always be fatal
>> allthought this is not an issue in most cases.
>> 
>> Fixes: b96672dd840f ("powerpc: Machine check interrupt is a non-maskable interrupt")
>> Fixes: daf00ae71dad ("powerpc/traps: restore recoverability of machine_check interrupts")
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>> Cc: stable@vger.kernel.org
>> ---
>>  arch/powerpc/kernel/traps.c | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
>> index 64936b60d521..c740f8bfccc9 100644
>> --- a/arch/powerpc/kernel/traps.c
>> +++ b/arch/powerpc/kernel/traps.c
>> @@ -763,15 +763,15 @@ void machine_check_exception(struct pt_regs *regs)
>>  	if (check_io_access(regs))
>>  		goto bail;
>>  
>> -	/* Must die if the interrupt is not recoverable */
>> -	if (!(regs->msr & MSR_RI))
>> -		nmi_panic(regs, "Unrecoverable Machine check");
>> -
>>  	if (!nested)
>>  		nmi_exit();
>>  
>>  	die("Machine check", regs, SIGBUS);
>>  
>> +	/* Must die if the interrupt is not recoverable */
>> +	if (!(regs->msr & MSR_RI))
>> +		nmi_panic(regs, "Unrecoverable Machine check");
>> +
>>  	return;
>>  
>>  bail:
>> -- 
>> 2.13.3
>> 

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

* Re: [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32
  2020-09-11 13:23   ` Michael Ellerman
@ 2020-09-14 12:46     ` Michal Suchánek
  0 siblings, 0 replies; 7+ messages in thread
From: Michal Suchánek @ 2020-09-14 12:46 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
	Nicholas Piggin, linuxppc-dev, linux-kernel

On Fri, Sep 11, 2020 at 11:23:57PM +1000, Michael Ellerman wrote:
> Michal Suchánek <msuchanek@suse.de> writes:
> > Hello,
> >
> > does this logic apply to "Unrecoverable System Reset" as well?
> 
> Which logic do you mean?
> 
> We do call die() before checking MSR_RI in system_reset_exception():
> 
>   	/*
>   	 * No debugger or crash dump registered, print logs then
>   	 * panic.
>   	 */
>   	die("System Reset", regs, SIGABRT);
>   
>   	mdelay(2*MSEC_PER_SEC); /* Wait a little while for others to print */
>   	add_taint(TAINT_DIE, LOCKDEP_NOW_UNRELIABLE);
>   	nmi_panic(regs, "System Reset");
>   
>   out:
>   #ifdef CONFIG_PPC_BOOK3S_64
>   	BUG_ON(get_paca()->in_nmi == 0);
>   	if (get_paca()->in_nmi > 1)
>   		die("Unrecoverable nested System Reset", regs, SIGABRT);
>   #endif
>   	/* Must die if the interrupt is not recoverable */
>   	if (!(regs->msr & MSR_RI))
>   		die("Unrecoverable System Reset", regs, SIGABRT);
> 
> 
> So you should see the output from die("System Reset", ...) even if
> MSR[RI] was clear when you took the system reset.

Indeed, replied to the wrong patch. I was looking at daf00ae71dad
("powerpc/traps: restore recoverability of machine_check interrupts")
which has very similar commit message.

Sorry about the confusion.

Thanks

Michal

> 
> cheers
> 
> > On Tue, Jan 22, 2019 at 02:11:24PM +0000, Christophe Leroy wrote:
> >> Looks like book3s/32 doesn't set RI on machine check, so
> >> checking RI before calling die() will always be fatal
> >> allthought this is not an issue in most cases.
> >> 
> >> Fixes: b96672dd840f ("powerpc: Machine check interrupt is a non-maskable interrupt")
> >> Fixes: daf00ae71dad ("powerpc/traps: restore recoverability of machine_check interrupts")
> >> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >> Cc: stable@vger.kernel.org
> >> ---
> >>  arch/powerpc/kernel/traps.c | 8 ++++----
> >>  1 file changed, 4 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> >> index 64936b60d521..c740f8bfccc9 100644
> >> --- a/arch/powerpc/kernel/traps.c
> >> +++ b/arch/powerpc/kernel/traps.c
> >> @@ -763,15 +763,15 @@ void machine_check_exception(struct pt_regs *regs)
> >>  	if (check_io_access(regs))
> >>  		goto bail;
> >>  
> >> -	/* Must die if the interrupt is not recoverable */
> >> -	if (!(regs->msr & MSR_RI))
> >> -		nmi_panic(regs, "Unrecoverable Machine check");
> >> -
> >>  	if (!nested)
> >>  		nmi_exit();
> >>  
> >>  	die("Machine check", regs, SIGBUS);
> >>  
> >> +	/* Must die if the interrupt is not recoverable */
> >> +	if (!(regs->msr & MSR_RI))
> >> +		nmi_panic(regs, "Unrecoverable Machine check");
> >> +
> >>  	return;
> >>  
> >>  bail:
> >> -- 
> >> 2.13.3
> >> 

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

end of thread, other threads:[~2020-09-14 12:51 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-22 14:11 [PATCH] powerpc/traps: fix recoverability of machine check handling on book3s/32 Christophe Leroy
2019-01-23  8:56 ` Nicholas Piggin
2019-02-22  9:47 ` Michael Ellerman
2020-09-11  9:15 ` [PATCH] " Michal Suchánek
2020-09-11  9:54   ` Christophe Leroy
2020-09-11 13:23   ` Michael Ellerman
2020-09-14 12:46     ` Michal Suchánek

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