oops broken on v7-M

oops broken on v7-M

[Uwe Kleine-König]

Hello,

__show_regs does among other things the following:

        printk("Flags: %s  IRQs o%s  FIQs o%s  Mode %s  ISA %s  Segment %s\n",
                buf, interrupts_enabled(regs) ? "n" : "ff",
                fast_interrupts_enabled(regs) ? "n" : "ff",
                processor_modes[processor_mode(regs)],
                isa_modes[isa_mode(regs)],
                get_fs() == get_ds() ? "kernel" : "user");

with isa_mode being defined as follows:

	#define isa_mode(regs) \
		((((regs)->ARM_cpsr & PSR_J_BIT) >> 23) | \ 
		 (((regs)->ARM_cpsr & PSR_T_BIT) >> 5))

with

	#define V4_PSR_T_BIT    0x00000020      /* >= V4T, but not V7M */
	#define V7M_PSR_T_BIT   0x01000000
	#if defined(__KERNEL__) && defined(CONFIG_CPU_V7M)
	#define PSR_T_BIT       V7M_PSR_T_BIT
	#else
	/* for compatibility */
	#define PSR_T_BIT       V4_PSR_T_BIT
	#endif          
	...
	#define PSR_J_BIT       0x01000000      /* >= V5J, but not V7M */

so on !CPU_V7M isa_mode maps nicely into [0 .. 3], on CPU_V7M however it
maps to either 0x80000 or 0x80002 which both are bad values for indexing
into isa_modes[].

The possibilities to fix that are:
 1) #define isa_mode(regs) to be 1
    This makes all memory accesses fine, but still the other bits printed
    are bogus (IRQs, FIQs, Mode) as before.
 2) just do:
    printk("xPSR: %08x\n", regs->ARM_cpsr);
    instead of the printk above for CPU_V7M. This is informative, but an
    expert output.
 3) don't hardcode 23 and 5, but calculate these depending on PSR_J_BIT
    and PSR_T_BIT.

I think I will go for b) + c), but not today any more. What do you
think?

Best regards
Uwe


-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] ARM: make isa_mode macro more robust and fix for v7-M

[Uwe Kleine-König]

The definition of isa_mode hardcodes the values to shift PSR_J_BIT and
PSR_T_BIT to move them to bits 1 and 0 respectively. Instead use __ffs to
calculate the shift from the #define already used for masking.

This is relevant on v7-M as there PSR_T_BIT is 0x01000000 instead of
0x00000020 for V7-[AR] and earlier. Because of that isa_mode produced
values >= 0x80000 which are unsuitable to index into isa_modes[4] there
and so made __show_regs read from undefined memory which resulted in
hangs and crashes.

Moreover isa_mode is wrong for v7-M even after this robustness fix as
there is no J-bit in the PSR register. So hardcode isa_mode to "Thumb"
for v7-M.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm/include/asm/ptrace.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/ptrace.h b/arch/arm/include/asm/ptrace.h
index 04c99f3..7f51fee 100644
--- a/arch/arm/include/asm/ptrace.h
+++ b/arch/arm/include/asm/ptrace.h
@@ -27,9 +27,13 @@ struct pt_regs {
 #define thumb_mode(regs) (0)
 #endif
 
+#ifndef CONFIG_CPU_V7M
 #define isa_mode(regs) \
-	((((regs)->ARM_cpsr & PSR_J_BIT) >> 23) | \
-	 (((regs)->ARM_cpsr & PSR_T_BIT) >> 5))
+	((((regs)->ARM_cpsr & PSR_J_BIT) >> (__ffs(PSR_J_BIT) - 1)) | \
+	 (((regs)->ARM_cpsr & PSR_T_BIT) >> (__ffs(PSR_T_BIT)))
+#else
+#define isa_mode(regs) 1 /* Thumb */
+#endif
 
 #define processor_mode(regs) \
 	((regs)->ARM_cpsr & MODE_MASK)
-- 
1.8.4.4

[PATCH v2] ARM: make isa_mode macro more robust and fix for v7-M

[Uwe Kleine-König]

The definition of isa_mode hardcodes the values to shift PSR_J_BIT and
PSR_T_BIT to move them to bits 1 and 0 respectively. Instead use __ffs to
calculate the shift from the #define already used for masking.

This is relevant on v7-M as there PSR_T_BIT is 0x01000000 instead of
0x00000020 for V7-[AR] and earlier. Because of that isa_mode produced
values >= 0x80000 which are unsuitable to index into isa_modes[4] there
and so made __show_regs read from undefined memory which resulted in
hangs and crashes.

Moreover isa_mode is wrong for v7-M even after this robustness fix as
there is no J-bit in the PSR register. So hardcode isa_mode to "Thumb"
for v7-M.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1, sent with Message-Id:
1387186673-7439-1-git-send-email-u.kleine-koenig at pengutronix.de:

 - fix !CONFIG_CPU_V7M compilation because of a missing ')'
 
 arch/arm/include/asm/ptrace.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/ptrace.h b/arch/arm/include/asm/ptrace.h
index 04c99f36ff7f..627a03ebb987 100644
--- a/arch/arm/include/asm/ptrace.h
+++ b/arch/arm/include/asm/ptrace.h
@@ -27,9 +27,13 @@ struct pt_regs {
 #define thumb_mode(regs) (0)
 #endif
 
+#ifndef CONFIG_CPU_V7M
 #define isa_mode(regs) \
-	((((regs)->ARM_cpsr & PSR_J_BIT) >> 23) | \
-	 (((regs)->ARM_cpsr & PSR_T_BIT) >> 5))
+	((((regs)->ARM_cpsr & PSR_J_BIT) >> (__ffs(PSR_J_BIT) - 1)) | \
+	 (((regs)->ARM_cpsr & PSR_T_BIT) >> (__ffs(PSR_T_BIT))))
+#else
+#define isa_mode(regs) 1 /* Thumb */
+#endif
 
 #define processor_mode(regs) \
 	((regs)->ARM_cpsr & MODE_MASK)
-- 
1.8.5.2

[PATCH] ARM: show_regs: on v7-M there are no FIQs, different processor modes, ...

[Uwe Kleine-König]

no indication about irqs in PSR and only a single ISA. So skip the whole
decoding and just print the xPSR on v7-M.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 arch/arm/kernel/process.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 541843b..e9d4b42 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -276,12 +276,17 @@ void __show_regs(struct pt_regs *regs)
 	buf[3] = flags & PSR_V_BIT ? 'V' : 'v';
 	buf[4] = '\0';
 
+#ifndef CONFIG_CPU_V7M
 	printk("Flags: %s  IRQs o%s  FIQs o%s  Mode %s  ISA %s  Segment %s\n",
 		buf, interrupts_enabled(regs) ? "n" : "ff",
 		fast_interrupts_enabled(regs) ? "n" : "ff",
 		processor_modes[processor_mode(regs)],
 		isa_modes[isa_mode(regs)],
 		get_fs() == get_ds() ? "kernel" : "user");
+#else
+	printk("xPSR: %08x\n", regs->ARM_cpsr);
+#endif
+
 #ifdef CONFIG_CPU_CP15
 	{
 		unsigned int ctrl;
-- 
1.8.4.4

[PATCH v2] ARM: show_regs: on v7-M there are no FIQs, different processor modes, ...

[Uwe Kleine-König]

no indication about irqs in PSR and only a single ISA. So skip the whole
decoding and just print the xPSR on v7-M.

Also mark two static variables as __maybe_unused to prevent the compiler
from emitting:

	arch/arm/kernel/process.c:51:20: warning: 'processor_modes' defined but not used [-Wunused-variable]
	arch/arm/kernel/process.c:58:20: warning: 'isa_modes' defined but not used [-Wunused-variable]

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
Changes since (implicit) v1, sent with Message-Id:
1387187296-7558-1-git-send-email-u.kleine-koenig at pengutronix.de:

 - add the __maybe_unused annotations and mention that in the commit log.

 arch/arm/kernel/process.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 92f7b15dd221..204f7d273319 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -48,14 +48,14 @@ unsigned long __stack_chk_guard __read_mostly;
 EXPORT_SYMBOL(__stack_chk_guard);
 #endif
 
-static const char *processor_modes[] = {
+static const char *processor_modes[] __maybe_unused = {
   "USER_26", "FIQ_26" , "IRQ_26" , "SVC_26" , "UK4_26" , "UK5_26" , "UK6_26" , "UK7_26" ,
   "UK8_26" , "UK9_26" , "UK10_26", "UK11_26", "UK12_26", "UK13_26", "UK14_26", "UK15_26",
   "USER_32", "FIQ_32" , "IRQ_32" , "SVC_32" , "UK4_32" , "UK5_32" , "UK6_32" , "ABT_32" ,
   "UK8_32" , "UK9_32" , "UK10_32", "UND_32" , "UK12_32", "UK13_32", "UK14_32", "SYS_32"
 };
 
-static const char *isa_modes[] = {
+static const char *isa_modes[] __maybe_unused = {
   "ARM" , "Thumb" , "Jazelle", "ThumbEE"
 };
 
@@ -276,12 +276,17 @@ void __show_regs(struct pt_regs *regs)
 	buf[3] = flags & PSR_V_BIT ? 'V' : 'v';
 	buf[4] = '\0';
 
+#ifndef CONFIG_CPU_V7M
 	printk("Flags: %s  IRQs o%s  FIQs o%s  Mode %s  ISA %s  Segment %s\n",
 		buf, interrupts_enabled(regs) ? "n" : "ff",
 		fast_interrupts_enabled(regs) ? "n" : "ff",
 		processor_modes[processor_mode(regs)],
 		isa_modes[isa_mode(regs)],
 		get_fs() == get_ds() ? "kernel" : "user");
+#else
+	printk("xPSR: %08lx\n", regs->ARM_cpsr);
+#endif
+
 #ifdef CONFIG_CPU_CP15
 	{
 		unsigned int ctrl;
-- 
1.8.5.2