[PATCH v2 0/3] Fix Thumb-2 undef handling for mixed-arch kernels

[PATCH v2 0/3] Fix Thumb-2 undef handling for mixed-arch kernels

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

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

-- 
1.7.4.1

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

[Dave Martin]

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.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/include/asm/system.h |    3 ++-
 arch/arm/kernel/setup.c       |   20 +++++++++++++++++++-
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 832888d..4adf71b 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -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);
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 70bca64..1f11472 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -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;
-- 
1.7.4.1

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

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

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Reviewed-by: Jon Medhurst <tixy@yxit.co.uk>
Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 arch/arm/kernel/entry-armv.S |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index a87cbf8..b7236d4 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -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
-- 
1.7.4.1

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

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

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Reviewed-by: Jon Medhurst <tixy@yxit.co.uk>
---
 arch/arm/kernel/entry-armv.S |   38 +++++++++++++++++++++++++++++++++++++-
 1 files changed, 37 insertions(+), 1 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index b7236d4..9ad50c4 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -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
 
-- 
1.7.4.1

[PATCH v2 0/3] Fix Thumb-2 undef handling for mixed-arch kernels

[Russell King - ARM Linux]

On Thu, Aug 18, 2011 at 06:01:51PM +0100, Dave Martin wrote:
> 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.

[PATCH v2 0/3] Fix Thumb-2 undef handling for mixed-arch kernels

[Tixy]

On Thu, 2011-08-18 at 23:29 +0100, Russell King - ARM Linux wrote:
> On Thu, Aug 18, 2011 at 06:01:51PM +0100, Dave Martin wrote:
> > 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.

[PATCH v2 0/3] Fix Thumb-2 undef handling for mixed-arch kernels

[Dave Martin]

On Thu, Aug 18, 2011 at 11:29:06PM +0100, Russell King - ARM Linux wrote:
> On Thu, Aug 18, 2011 at 06:01:51PM +0100, Dave Martin wrote:
> > 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

[PATCH v2 0/3] Fix Thumb-2 undef handling for mixed-arch kernels

[Dave Martin]

On Fri, Aug 19, 2011 at 09:44:28AM +0100, Tixy wrote:
> On Thu, 2011-08-18 at 23:29 +0100, Russell King - ARM Linux wrote:
> > On Thu, Aug 18, 2011 at 06:01:51PM +0100, Dave Martin wrote:
> > > 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.

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

[PATCH v2 0/3] Fix Thumb-2 undef handling for mixed-arch kernels

[Tixy]

On Fri, 2011-08-19 at 10:10 +0100, Dave Martin wrote: 
> On Fri, Aug 19, 2011 at 09:44:28AM +0100, Tixy wrote:
> > On Thu, 2011-08-18 at 23:29 +0100, Russell King - ARM Linux wrote:
> > > On Thu, Aug 18, 2011 at 06:01:51PM +0100, Dave Martin wrote:
> > > > 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.
> 
> 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.