Skip to contentSkip to conversation

Fix Thumb-2 undef handling for mixed-arch kernels

PATCH Superseded Dave Martin 3 participants 9 messages Aug 18, 2011 – Aug 19, 2011 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

v2 cover Fix Thumb-2 undef handling for mixed-arch kernels

In kernels which support v6 and v7 platforms in a single binary such as omap2plus_defconfig for example, undefined instruction exceptions taken on Thumb instructions are not processed correctly, because the kernel assumes at build-time that Thumb-2 won't be supported in userspace.

This series implements a workaround, by allowing the __und_usr handler to check the CPU architecture at runtime, in affected kernels.

Changes since v1:
* Because cpu_architecture() is no longer trivial, it has been moveed back out of line, into setup.c.

Changes since RFC:
* Renamed the global variable to __cpu_architecture and provided cpu_architecture() as in inline function in <asm/system.h>. This brings back the old API, but more efficiently.
* Dropped all the patches required to handle the reverted API change.
* cpu_architecture is now declared __pure, which may lead to slightly more efficient code in some cases.
* Added BUG_ON() to cpu_architecture() so that if __cpu_architecture is not set early enough, it gets noticed.

Thanks to Tixy and Nicolas Pitre for their helpful feedback which helped slim down this series.

Dave Martin (3):
ARM: Make cpu_architecture into a global variable
ARM: entry: Remove unnecessary masking when decoding Thumb-2
instructions
ARM: entry: Fix Thumb-2 undef handling for multi-CPU kernels

 arch/arm/include/asm/system.h |    3 +-
 arch/arm/kernel/entry-armv.S  |   44 ++++++++++++++++++++++++++++++++++++----
 arch/arm/kernel/setup.c       |   20 +++++++++++++++++-
 3 files changed, 60 insertions(+), 7 deletions(-)
Signature · 1 line
1.7.4.1
#1 lore raw

PATCH v2 1/3 ARM: Make cpu_architecture into a global variable

The CPU architecture really should not be changing at runtime, so make it a global variable instead of a function.

The cpu_architecture() function declared in <asm/system.h> remains the correct way to read this variable from C code.

---
 arch/arm/include/asm/system.h |    3 ++-
 arch/arm/kernel/setup.c       |   20 +++++++++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)
arch/arm/include/asm/system.h +2 -1
index 832888d..4adf71b 100644
@@ -57,6 +57,7 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/compiler.h>
 #include <linux/linkage.h>
 #include <linux/irqflags.h>
 
@@ -104,7 +105,7 @@ struct mm_struct;
 extern void show_pte(struct mm_struct *mm, unsigned long addr);
 extern void __show_regs(struct pt_regs *);
 
-extern int cpu_architecture(void);
+extern int __pure cpu_architecture(void);
 extern void cpu_init(void);
 
 void arm_machine_restart(char mode, const char *cmd);
arch/arm/kernel/setup.c +19 -1
index 70bca64..1f11472 100644
@@ -29,6 +29,8 @@
 #include <linux/fs.h>
 #include <linux/proc_fs.h>
 #include <linux/memblock.h>
+#include <linux/bug.h>
+#include <linux/compiler.h>
 
 #include <asm/unified.h>
 #include <asm/cpu.h>
@@ -42,6 +44,7 @@
 #include <asm/cacheflush.h>
 #include <asm/cachetype.h>
 #include <asm/tlbflush.h>
+#include <asm/system.h>
 
 #include <asm/prom.h>
 #include <asm/mach/arch.h>
@@ -115,6 +118,13 @@ struct outer_cache_fns outer_cache __read_mostly;
 EXPORT_SYMBOL(outer_cache);
 #endif
 
+/*
+ * Cached cpu_architecture() result for use by assembler code.
+ * C code should use the cpu_architecture() function instead of accessing this
+ * variable directly.
+ */
+int __cpu_architecture __read_mostly = CPU_ARCH_UNKNOWN;
+
 struct stack {
 	u32 irq[3];
 	u32 abt[3];
@@ -210,7 +220,7 @@ static const char *proc_arch[] = {
 	"?(17)",
 };
 
-int cpu_architecture(void)
+static int __get_cpu_architecture(void)
 {
 	int cpu_arch;
 
@@ -243,6 +253,13 @@ int cpu_architecture(void)
 	return cpu_arch;
 }
 
+int __pure cpu_architecture(void)
+{
+	BUG_ON(__cpu_architecture == CPU_ARCH_UNKNOWN);
+
+	return __cpu_architecture;
+}
+
 static int cpu_has_aliasing_icache(unsigned int arch)
 {
 	int aliasing_icache;
@@ -413,6 +430,7 @@ static void __init setup_processor(void)
 	}
 
 	cpu_name = list->cpu_name;
+	__cpu_architecture = __get_cpu_architecture();
 
 #ifdef MULTI_CPU
 	processor = *list->proc;
Signature · 1 line
1.7.4.1
#2 lore raw

PATCH v2 2/3 ARM: entry: Remove unnecessary masking when decoding Thumb-2 instructions

In reply to Dave Martin

When testing whether a Thumb-2 instruction is 32 bits long or not, the masking done in order to test bits 11-15 of the first instruction halfword won't affect the result of the comparison, so remove it.

---
 arch/arm/kernel/entry-armv.S |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
arch/arm/kernel/entry-armv.S +2 -4
index a87cbf8..b7236d4 100644
@@ -262,8 +262,7 @@ __und_svc:
 	ldr	r0, [r4, #-4]
 #else
 	ldrh	r0, [r4, #-2]			@ Thumb instruction at LR - 2
-	and	r9, r0, #0xf800
-	cmp	r9, #0xe800			@ 32-bit instruction if xx >= 0
+	cmp	r0, #0xe800			@ 32-bit instruction if xx >= 0
 	ldrhhs	r9, [r4]			@ bottom 16 bits
 	orrhs	r0, r9, r0, lsl #16
 #endif
@@ -445,8 +444,7 @@ __und_usr:
  ARM(	ldrht	r5, [r4], #2	)
  THUMB(	ldrht	r5, [r4]	)
  THUMB(	add	r4, r4, #2	)
-	and	r0, r5, #0xf800			@ mask bits 111x x... .... ....
-	cmp	r0, #0xe800			@ 32bit instruction if xx != 0
+	cmp	r5, #0xe800			@ 32bit instruction if xx != 0
 	blo	__und_usr_unknown
 3:	ldrht	r0, [r4]
 	add	r2, r2, #2			@ r2 is PC + 2, make it PC + 4
Signature · 1 line
1.7.4.1
#3 lore raw

PATCH v2 3/3 ARM: entry: Fix Thumb-2 undef handling for multi-CPU kernels

In reply to Dave Martin

When v6 and >=v7 boards are supported in the same kernel, the __und_usr code currently makes a build-time assumption that Thumb-2 instructions occurring in userspace don't need to be supported. Strictly speaking this is incorrect.

This patch fixes the above case by doing a run-time check on the CPU architecture in these cases. This only affects kernels which support v6 and >=v7 CPUs together: plain v6 and plain v7 kernels are unaffected.

---
 arch/arm/kernel/entry-armv.S |   38 +++++++++++++++++++++++++++++++++++++-
 1 files changed, 37 insertions(+), 1 deletions(-)
arch/arm/kernel/entry-armv.S +37 -1
index b7236d4..9ad50c4 100644
@@ -24,6 +24,7 @@
 #include <asm/unwind.h>
 #include <asm/unistd.h>
 #include <asm/tls.h>
+#include <asm/system.h>
 
 #include "entry-header.S"
 #include <asm/entry-macro-multi.S>
@@ -439,7 +440,27 @@ __und_usr:
 #endif
 	beq	call_fpe
 	@ Thumb instruction
-#if __LINUX_ARM_ARCH__ >= 7
+#if CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7
+/*
+ * Thumb-2 instruction handling.  Note that because pre-v6 and >= v6 platforms
+ * can never be supported in a single kernel, this code is not applicable at
+ * all when __LINUX_ARM_ARCH__ < 6.  This allows simplifying assumptions to be
+ * made about .arch directives.
+ */
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target CPU may not be Thumb-2-capable, a run-time check is needed: */
+#define NEED_CPU_ARCHITECTURE
+	ldr	r5, .LCcpu_architecture
+	ldr	r5, [r5]
+	cmp	r5, #CPU_ARCH_ARMv7
+	blo	__und_usr_unknown
+/*
+ * The following code won't get run unless the running CPU really is v7, so
+ * coding round the lack of ldrht on older arches is pointless.  Temporarily
+ * override the assembler target arch with the minimum required instead:
+ */
+	.arch	armv6t2
+#endif
 2:
  ARM(	ldrht	r5, [r4], #2	)
  THUMB(	ldrht	r5, [r4]	)
@@ -449,7 +470,16 @@ __und_usr:
 3:	ldrht	r0, [r4]
 	add	r2, r2, #2			@ r2 is PC + 2, make it PC + 4
 	orr	r0, r0, r5, lsl #16
+
+#if __LINUX_ARM_ARCH__ < 7
+/* If the target arch was overridden, change it back: */
+#ifdef CONFIG_CPU_32v6K
+	.arch	armv6k
 #else
+	.arch	armv6
+#endif
+#endif /* __LINUX_ARM_ARCH__ < 7 */
+#else /* !(CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7) */
 	b	__und_usr_unknown
 #endif
  UNWIND(.fnend		)
@@ -576,6 +606,12 @@ call_fpe:
 	movw_pc	lr				@ CP#14 (Debug)
 	movw_pc	lr				@ CP#15 (Control)
 
+#ifdef NEED_CPU_ARCHITECTURE
+	.align	2
+.LCcpu_architecture:
+	.word	__cpu_architecture
+#endif
+
 #ifdef CONFIG_NEON
 	.align	6
 
Signature · 1 line
1.7.4.1
#4 lore raw

v2 cover Fix Thumb-2 undef handling for mixed-arch kernels

In reply to Dave Martin
Quoted 5 lines · Dave Martin

In kernels which support v6 and v7 platforms in a single binary such as omap2plus_defconfig for example, undefined instruction exceptions taken on Thumb instructions are not processed correctly, because the kernel assumes at build-time that Thumb-2 won't be supported in userspace.

Unless there's any further comments from anyone, I think this is now ready. Thanks.

#5 lore raw

v2 cover Fix Thumb-2 undef handling for mixed-arch kernels

Quoted 9 lines · Russell King - ARM Linux
Quoted 5 lines · Dave Martin

In kernels which support v6 and v7 platforms in a single binary such as omap2plus_defconfig for example, undefined instruction exceptions taken on Thumb instructions are not processed correctly, because the kernel assumes at build-time that Thumb-2 won't be supported in userspace.

Unless there's any further comments from anyone, I think this is now ready. Thanks.

I've no further comments.

#6 lore raw

v2 cover Fix Thumb-2 undef handling for mixed-arch kernels

Quoted 8 lines · Tixy
Quoted 7 lines · Russell King - ARM Linux
Quoted 5 lines · Dave Martin

In kernels which support v6 and v7 platforms in a single binary such as omap2plus_defconfig for example, undefined instruction exceptions taken on Thumb instructions are not processed correctly, because the kernel assumes at build-time that Thumb-2 won't be supported in userspace.

Quoted 4 lines
Quoted 2 lines

Unless there's any further comments from anyone, I think this is now ready. Thanks.

I've no further comments.

Are you happy for me to transfer your reviewed-by to the new version of patch 1? I removed the tags beacuse it's changed, though perhaps unnecessarily -- the changes are fairly minor compared with the last version.

Cheers
---Dave

#7 lore raw

v2 cover Fix Thumb-2 undef handling for mixed-arch kernels

Quoted 9 lines · Dave Martin
Quoted 8 lines · Tixy
Quoted 7 lines · Russell King - ARM Linux
Quoted 5 lines · Dave Martin

In kernels which support v6 and v7 platforms in a single binary such as omap2plus_defconfig for example, undefined instruction exceptions taken on Thumb instructions are not processed correctly, because the kernel assumes at build-time that Thumb-2 won't be supported in userspace.

Quoted 7 lines
Quoted 4 lines
Quoted 2 lines

Unless there's any further comments from anyone, I think this is now ready. Thanks.

I've no further comments.

Are you happy for me to transfer your reviewed-by to the new version of patch 1?

Yes, I compared the new patch to the old one.

#8 lore raw

v2 cover Fix Thumb-2 undef handling for mixed-arch kernels

In reply to Russell King - ARM Linux
Quoted 9 lines · Russell King - ARM Linux
Quoted 5 lines · Dave Martin

In kernels which support v6 and v7 platforms in a single binary such as omap2plus_defconfig for example, undefined instruction exceptions taken on Thumb instructions are not processed correctly, because the kernel assumes at build-time that Thumb-2 won't be supported in userspace.

Unless there's any further comments from anyone, I think this is now ready. Thanks.

OK, I'll send it to the patch system if nobody comes back with anything.

Cheers
---Dave