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

[PATCH 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 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_alignment 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 |   11 +++++++++-
 arch/arm/kernel/entry-armv.S  |   44 ++++++++++++++++++++++++++++++++++++----
 arch/arm/kernel/setup.c       |    5 +++-
 3 files changed, 53 insertions(+), 7 deletions(-)

--
1.7.4.1

[PATCH 1/3] ARM: Make cpu_alignment 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 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 |   11 ++++++++++-
 arch/arm/kernel/setup.c       |    5 ++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index 832888d..b0445f7 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -57,6 +57,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <linux/bug.h>
+#include <linux/compiler.h>
 #include <linux/linkage.h>
 #include <linux/irqflags.h>
 
@@ -104,7 +106,14 @@ 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 __cpu_architecture;
+
+static inline int __pure cpu_architecture(void)
+{
+	BUG_ON(__cpu_architecture == CPU_ARCH_UNKNOWN);
+	return __cpu_architecture;
+}
+
 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..1e0c1b3 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -42,6 +42,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>
@@ -114,6 +115,7 @@ struct cpu_cache_fns cpu_cache __read_mostly;
 struct outer_cache_fns outer_cache __read_mostly;
 EXPORT_SYMBOL(outer_cache);
 #endif
+int __cpu_architecture __read_mostly = CPU_ARCH_UNKNOWN;
 
 struct stack {
 	u32 irq[3];
@@ -210,7 +212,7 @@ static const char *proc_arch[] = {
 	"?(17)",
 };
 
-int cpu_architecture(void)
+static int __init __get_cpu_architecture(void)
 {
 	int cpu_arch;
 
@@ -413,6 +415,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 1/3] ARM: Make cpu_alignment into a global variable

[Nicolas Pitre]

On Tue, 16 Aug 2011, Dave Martin wrote:

> The CPU architecture really should not be changing at runtime, so
> make it a global variable instead of a function.
> 
> The cpu_architecture() function in <asm/system.h> remains the
> correct way to read this variable from C code.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>

Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

> ---
>  arch/arm/include/asm/system.h |   11 ++++++++++-
>  arch/arm/kernel/setup.c       |    5 ++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> index 832888d..b0445f7 100644
> --- a/arch/arm/include/asm/system.h
> +++ b/arch/arm/include/asm/system.h
> @@ -57,6 +57,8 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +#include <linux/bug.h>
> +#include <linux/compiler.h>
>  #include <linux/linkage.h>
>  #include <linux/irqflags.h>
>  
> @@ -104,7 +106,14 @@ 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 __cpu_architecture;
> +
> +static inline int __pure cpu_architecture(void)
> +{
> +	BUG_ON(__cpu_architecture == CPU_ARCH_UNKNOWN);
> +	return __cpu_architecture;
> +}
> +
>  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..1e0c1b3 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -42,6 +42,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>
> @@ -114,6 +115,7 @@ struct cpu_cache_fns cpu_cache __read_mostly;
>  struct outer_cache_fns outer_cache __read_mostly;
>  EXPORT_SYMBOL(outer_cache);
>  #endif
> +int __cpu_architecture __read_mostly = CPU_ARCH_UNKNOWN;
>  
>  struct stack {
>  	u32 irq[3];
> @@ -210,7 +212,7 @@ static const char *proc_arch[] = {
>  	"?(17)",
>  };
>  
> -int cpu_architecture(void)
> +static int __init __get_cpu_architecture(void)
>  {
>  	int cpu_arch;
>  
> @@ -413,6 +415,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 1/3] ARM: Make cpu_alignment into a global variable

[Dave Martin]

On Tue, Aug 16, 2011 at 12:15:59PM -0400, Nicolas Pitre wrote:
> On Tue, 16 Aug 2011, Dave Martin wrote:
> 
> > The CPU architecture really should not be changing at runtime, so
> > make it a global variable instead of a function.
> > 
> > The cpu_architecture() function in <asm/system.h> remains the
> > correct way to read this variable from C code.
> > 
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> 
> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

Thanks
---Dave

> 
> > ---
> >  arch/arm/include/asm/system.h |   11 ++++++++++-
> >  arch/arm/kernel/setup.c       |    5 ++++-
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
> > index 832888d..b0445f7 100644
> > --- a/arch/arm/include/asm/system.h
> > +++ b/arch/arm/include/asm/system.h
> > @@ -57,6 +57,8 @@
> >  
> >  #ifndef __ASSEMBLY__
> >  
> > +#include <linux/bug.h>
> > +#include <linux/compiler.h>
> >  #include <linux/linkage.h>
> >  #include <linux/irqflags.h>
> >  
> > @@ -104,7 +106,14 @@ 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 __cpu_architecture;
> > +
> > +static inline int __pure cpu_architecture(void)
> > +{
> > +	BUG_ON(__cpu_architecture == CPU_ARCH_UNKNOWN);
> > +	return __cpu_architecture;
> > +}
> > +
> >  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..1e0c1b3 100644
> > --- a/arch/arm/kernel/setup.c
> > +++ b/arch/arm/kernel/setup.c
> > @@ -42,6 +42,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>
> > @@ -114,6 +115,7 @@ struct cpu_cache_fns cpu_cache __read_mostly;
> >  struct outer_cache_fns outer_cache __read_mostly;
> >  EXPORT_SYMBOL(outer_cache);
> >  #endif
> > +int __cpu_architecture __read_mostly = CPU_ARCH_UNKNOWN;
> >  
> >  struct stack {
> >  	u32 irq[3];
> > @@ -210,7 +212,7 @@ static const char *proc_arch[] = {
> >  	"?(17)",
> >  };
> >  
> > -int cpu_architecture(void)
> > +static int __init __get_cpu_architecture(void)
> >  {
> >  	int cpu_arch;
> >  
> > @@ -413,6 +415,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 1/3] ARM: Make cpu_alignment into a global variable

[Sergei Shtylyov]

Hello.

On 16-08-2011 18:14, Dave Martin wrote:

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

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

> Signed-off-by: Dave Martin<dave.martin@linaro.org>

    You're talking of the CPU architecture here but the subject has 
"cpu_alignment"...

WBR, Sergei

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

[Dave Martin]

On Wed, Aug 17, 2011 at 03:31:21PM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 16-08-2011 18:14, Dave Martin wrote:
> 
> >The CPU architecture really should not be changing at runtime, so
> >make it a global variable instead of a function.
> 
> >The cpu_architecture() function in<asm/system.h>  remains the
> >correct way to read this variable from C code.
> 
> >Signed-off-by: Dave Martin<dave.martin@linaro.org>
> 
>    You're talking of the CPU architecture here but the subject has
> "cpu_alignment"...

Hmmm, I guess my brain was on autopilot when I wrote that...

You're right, it should say cpu_architecture in the subject.
I'll fix this before I repost.

Thanks
---Dave

[PATCH 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>
---
 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 2/3] ARM: entry: Remove unnecessary masking when decoding Thumb-2 instructions

[Nicolas Pitre]

On Tue, 16 Aug 2011, Dave Martin wrote:

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

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 2/3] ARM: entry: Remove unnecessary masking when decoding Thumb-2 instructions

[Dave Martin]

On Tue, Aug 16, 2011 at 12:17:35PM -0400, Nicolas Pitre wrote:
> On Tue, 16 Aug 2011, Dave Martin wrote:
> 
> > 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>
> 
> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> 

Thanks
---Dave

> 
> > ---
> >  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 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>
---
 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 3/3] ARM: entry: Fix Thumb-2 undef handling for multi-CPU kernels

[Tixy]

On Tue, 2011-08-16 at 15:14 +0100, Dave Martin wrote:
[...]
> +#ifdef NEED_CPU_ARCHITECTURE
> +	.align	2
> +.LCcpu_architecture:
> +	.word	__cpu_architecture
> +#endif

What's the convention about the prefix '.LC' as opposed to just '.L'?

-- 
Tixy

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

[Nicolas Pitre]

On Tue, 16 Aug 2011, Tixy wrote:

> On Tue, 2011-08-16 at 15:14 +0100, Dave Martin wrote:
> [...]
> > +#ifdef NEED_CPU_ARCHITECTURE
> > +	.align	2
> > +.LCcpu_architecture:
> > +	.word	__cpu_architecture
> > +#endif
> 
> What's the convention about the prefix '.LC' as opposed to just '.L'?

The .Lfoo format is more widely used than .LCfoo in the tree.


Nicolas

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

[Dave Martin]

On Tue, Aug 16, 2011 at 4:47 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Tue, 16 Aug 2011, Tixy wrote:
>
>> On Tue, 2011-08-16 at 15:14 +0100, Dave Martin wrote:
>> [...]
>> > +#ifdef NEED_CPU_ARCHITECTURE
>> > + ? .align ?2
>> > +.LCcpu_architecture:
>> > + ? .word ? __cpu_architecture
>> > +#endif
>>
>> What's the convention about the prefix '.LC' as opposed to just '.L'?
>
> The .Lfoo format is more widely used than .LCfoo in the tree.

...but .LCfoo is more widely used in entry-armv.S, for exactly the
same kind of thing.

I thought it best to blend in...  anyway, apart from the magic prefix
'.L', it's just a name.

Any concerns about this, or can we leave it as-is?

Cheers
---Dave

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

[Nicolas Pitre]

On Tue, 16 Aug 2011, Dave Martin wrote:

> On Tue, Aug 16, 2011 at 4:47 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > On Tue, 16 Aug 2011, Tixy wrote:
> >
> >> On Tue, 2011-08-16 at 15:14 +0100, Dave Martin wrote:
> >> [...]
> >> > +#ifdef NEED_CPU_ARCHITECTURE
> >> > + ? .align ?2
> >> > +.LCcpu_architecture:
> >> > + ? .word ? __cpu_architecture
> >> > +#endif
> >>
> >> What's the convention about the prefix '.LC' as opposed to just '.L'?
> >
> > The .Lfoo format is more widely used than .LCfoo in the tree.
> 
> ...but .LCfoo is more widely used in entry-armv.S, for exactly the
> same kind of thing.

Just use the same style then.


Nicolas

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

[Dave Martin]

On Tue, Aug 16, 2011 at 12:14:01PM -0400, Nicolas Pitre wrote:
> On Tue, 16 Aug 2011, Dave Martin wrote:
> 
> > On Tue, Aug 16, 2011 at 4:47 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> > > On Tue, 16 Aug 2011, Tixy wrote:
> > >
> > >> On Tue, 2011-08-16 at 15:14 +0100, Dave Martin wrote:
> > >> [...]
> > >> > +#ifdef NEED_CPU_ARCHITECTURE
> > >> > + ? .align ?2
> > >> > +.LCcpu_architecture:
> > >> > + ? .word ? __cpu_architecture
> > >> > +#endif
> > >>
> > >> What's the convention about the prefix '.LC' as opposed to just '.L'?
> > >
> > > The .Lfoo format is more widely used than .LCfoo in the tree.
> > 
> > ...but .LCfoo is more widely used in entry-armv.S, for exactly the
> > same kind of thing.
> 
> Just use the same style then.

OK, done.

Cheers
---Dave

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

[Tixy]

On Tue, 2011-08-16 at 15:14 +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.
> 
> This series implements a workaround, by allowing the __und_usr
> handler to check the CPU architecture at runtime, in affected
> kernels.
> 
> 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_alignment 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 |   11 +++++++++-
>  arch/arm/kernel/entry-armv.S  |   44 ++++++++++++++++++++++++++++++++++++----
>  arch/arm/kernel/setup.c       |    5 +++-
>  3 files changed, 53 insertions(+), 7 deletions(-)
> 

Looks good to me.

-- 
Tixy

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

[Dave Martin]

On Tue, Aug 16, 2011 at 3:58 PM, Tixy <tixy@yxit.co.uk> wrote:
> On Tue, 2011-08-16 at 15:14 +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.
>>
>> This series implements a workaround, by allowing the __und_usr
>> handler to check the CPU architecture at runtime, in affected
>> kernels.
>>
>> 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_alignment 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 | ? 11 +++++++++-
>> ?arch/arm/kernel/entry-armv.S ?| ? 44 ++++++++++++++++++++++++++++++++++++----
>> ?arch/arm/kernel/setup.c ? ? ? | ? ?5 +++-
>> ?3 files changed, 53 insertions(+), 7 deletions(-)
>>
>
> Looks good to me.

Thanks for looking over it.

Is that a Reviewed-by, or will you go for an Ack?

Cheers
---Dave

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

[Tixy]

On Tue, 2011-08-16 at 17:06 +0100, Dave Martin wrote:
> On Tue, Aug 16, 2011 at 3:58 PM, Tixy <tixy@yxit.co.uk> wrote:

> > Looks good to me.
> 
> Thanks for looking over it.
> 
> Is that a Reviewed-by, or will you go for an Ack?

It's certainly a Reviewed-by: Jon Medhurst <tixy@yxit.co.uk> 
for all three patches.

I've been re-reading Documentation/SubmittingPatches to try and work out
when it's appropriate to use Acked-by, but it seems a bit woolly -
mostly for maintainers?

-- 
Tixy