[PATCH] powerpc/tm: Print 64-bits MSR

On a kernel TM Bad thing program exception, the MSR is not being properly
displayed, since it dumps a 32-bits value. MSR is a 64 bits register for
all platforms that have HTM enabled.

This patch dumps the MSR value as 64-bits instead of 32 bits.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/kernel/traps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 0e17dcb48720..cd561fd89532 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c

@@ -1402,7 +1402,7 @@ void program_check_exception(struct pt_regs *regs)
 			goto bail;
 		} else {
 			printk(KERN_EMERG "Unexpected TM Bad Thing exception "
-			       "at %lx (msr 0x%x)\n", regs->nip, reason);
+			       "at %lx (msr 0x%lx)\n", regs->nip, regs->msr);
 			die("Unrecoverable exception", regs, SIGABRT);
 		}
 	}

-- 
2.16.3

On Mon, 2018-08-06 at 21:32 -0300, Breno Leitao wrote:

On a kernel TM Bad thing program exception, the MSR is not being properly
displayed, since it dumps a 32-bits value. MSR is a 64 bits register for
all platforms that have HTM enabled.
=20
This patch dumps the MSR value as 64-bits instead of 32 bits.

(sorry I was distracted when you asked me about this before offline...)

I think you might need to clear up in the description why you are changing
  reason -> msr.

Mikey

On a kernel TM Bad thing program exception, the Machine State Register
(MSR) is not being properly displayed. The exception code dumps a 32-bits
value but MSR is a 64 bits register for all platforms that have HTM
enabled.

This patch dumps the MSR value as a 64-bits value instead of 32 bits. In
order to do so, the 'reason' variable could not be used, since it trimmed
MSR to 32-bits (int).

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/kernel/traps.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 0e17dcb48720..cd561fd89532 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c

@@ -1402,7 +1402,7 @@ void program_check_exception(struct pt_regs *regs)
 			goto bail;
 		} else {
 			printk(KERN_EMERG "Unexpected TM Bad Thing exception "
-			       "at %lx (msr 0x%x)\n", regs->nip, reason);
+			       "at %lx (msr 0x%lx)\n", regs->nip, regs->msr);
 			die("Unrecoverable exception", regs, SIGABRT);
 		}
 	}

-- 
2.16.3

On Tue, Aug 07, 2018 at 10:35:00AM -0300, Breno Leitao wrote:

On a kernel TM Bad thing program exception, the Machine State Register
(MSR) is not being properly displayed. The exception code dumps a 32-bits
value but MSR is a 64 bits register for all platforms that have HTM
enabled.

This patch dumps the MSR value as a 64-bits value instead of 32 bits. In
order to do so, the 'reason' variable could not be used, since it trimmed
MSR to 32-bits (int).

So maybe reason should be a long instead of an int?


Segher

Le 07/08/2018 à 15:35, Breno Leitao a écrit :

On a kernel TM Bad thing program exception, the Machine State Register
(MSR) is not being properly displayed. The exception code dumps a 32-bits
value but MSR is a 64 bits register for all platforms that have HTM
enabled.

This patch dumps the MSR value as a 64-bits value instead of 32 bits. In
order to do so, the 'reason' variable could not be used, since it trimmed
MSR to 32-bits (int).

reason is not always regs->msr, see get_reason(), allthough in your case 
it is.

I think it would be better to change 'reason' to 'unsigned long' instead 
of replacing it by regs->msr for the printk.

Christophe

Hi,

On 08/07/2018 02:15 PM, Christophe LEROY wrote:

Le 07/08/2018 à 15:35, Breno Leitao a écrit :

quoted

On a kernel TM Bad thing program exception, the Machine State Register
(MSR) is not being properly displayed. The exception code dumps a 32-bits
value but MSR is a 64 bits register for all platforms that have HTM
enabled.

This patch dumps the MSR value as a 64-bits value instead of 32 bits. In
order to do so, the 'reason' variable could not be used, since it trimmed
MSR to 32-bits (int).

reason is not always regs->msr, see get_reason(), allthough in your case it is.

I think it would be better to change 'reason' to 'unsigned long' instead of
replacing it by regs->msr for the printk.

That was my initial approach, but this code seems to run on 32 bits system,
and I do not want to change the whole 'reason' bit width without having a 32
bits to test, at least.

Also, it is a bit weird doing something as:

	printk("....(msr 0x%lx)....", reason);

I personally think that the follow code is much more readable:

	printk(".... (msr 0x%lx)...", regs->msr);

Breno Leitao [off-list ref] a =C3=A9crit=C2=A0:

Hi,

On 08/07/2018 02:15 PM, Christophe LEROY wrote:

quoted

Le 07/08/2018 =C3=A0 15:35, Breno Leitao a =C3=A9crit=C2=A0:

quoted

On a kernel TM Bad thing program exception, the Machine State Register
(MSR) is not being properly displayed. The exception code dumps a 32-bi=

ts

quoted

quoted

value but MSR is a 64 bits register for all platforms that have HTM
enabled.

This patch dumps the MSR value as a 64-bits value instead of 32 bits. I=

n

quoted

quoted

order to do so, the 'reason' variable could not be used, since it trimm=

ed

quoted

quoted

MSR to 32-bits (int).

reason is not always regs->msr, see get_reason(), allthough in your=20=

=20

quoted

=20case it is.

I think it would be better to change 'reason' to 'unsigned long' instead=

 of

quoted

replacing it by regs->msr for the printk.

That was my initial approach, but this code seems to run on 32 bits syste=

m,

and I do not want to change the whole 'reason' bit width without having a=

 32

bits to test, at least.

But 'unsigned long' is still 32 bits on ppc32, so it makes no=20=20
difference=20with 'unsigned int'
And I will test it for you if needed

Christophe

Also, it is a bit weird doing something as:

	printk("....(msr 0x%lx)....", reason);

I personally think that the follow code is much more readable:

	printk(".... (msr 0x%lx)...", regs->msr);

Hi Leroy,

On 08/07/2018 03:57 PM, LEROY Christophe wrote:

Breno Leitao [off-list ref] a écrit :

quoted

On 08/07/2018 02:15 PM, Christophe LEROY wrote:

quoted

Le 07/08/2018 à 15:35, Breno Leitao a écrit :

quoted

quoted

I think it would be better to change 'reason' to 'unsigned long' instead of
replacing it by regs->msr for the printk.

That was my initial approach, but this code seems to run on 32 bits system,
and I do not want to change the whole 'reason' bit width without having a 32
bits to test, at least.

But 'unsigned long' is still 32 bits on ppc32, so it makes no difference with
'unsigned int'
And I will test it for you if needed

Cool, I really appreciate it, and I would definitely need it once I have a
more intrusive HTM patchset I am working on.

Regarding this one, I think the change is so simple as-is that  I would
prefer to continue with the v2 patch, if you do not mind.

Thank you!

On Tue, 2018-08-07 at 13:35:00 UTC, Breno Leitao wrote:

On a kernel TM Bad thing program exception, the Machine State Register
(MSR) is not being properly displayed. The exception code dumps a 32-bits
value but MSR is a 64 bits register for all platforms that have HTM
enabled.

This patch dumps the MSR value as a 64-bits value instead of 32 bits. In
order to do so, the 'reason' variable could not be used, since it trimmed
MSR to 32-bits (int).

Signed-off-by: Breno Leitao <leitao@debian.org>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/51303113e32fd92d327b3c441c45e2

cheers