[PATCH v2 0/9] ARM: Thumb-2: Various fixes for building Thumb-2 kernels

[PATCH v2 0/9] ARM: Thumb-2: Various fixes for building Thumb-2 kernels

[Dave Martin]

This series fixes various minor problems affecting building with
CONFIG_THUMB2_KERNEL.

v2: 

  * Improve zImage header layout patch based on feedback.
  * Disable kprobes via Kconfig when using CONFIG_THUMB2_KERNEL (for now)
  * Force longer-range Thumb-2 branch encoding for some branches in head.S,
	to avoid fixup errors when vmlinux is large.

Applies to 2.6.37-rc3


v1:

Fix misaligned data risks resulting from .long/.word without .align:
 * ARM: kexec: Correct data alignment for CONFIG_THUMB2_KERNEL
 * ARM: vfp: Correct data alignment for CONFIG_THUMB2_KERNEL
 * ARM: Thumb-2: Correct data alignment for CONFIG_THUMB2_KERNEL in bootp/init.S
 * ARM: Thumb-2: Correct data alignment for CONFIG_THUMB2_KERNEL in kernel/head.S
 * ARM: Thumb-2: Correct data alignment for CONFIG_THUMB2_KERNEL in mm/proc-v7.S

Fix Thumb-2 incompatibilities in compressed kernel entry code,
to avoid build failures:
 * ARM: Thumb-2: Fix CONFIG_THUMB2_KERNEL breakage in compressed/head.S

Fix strange zImage header layout when building Thumb-2 kernels:
 * ARM: Thumb-2: Restore sensible zImage header layout for CONFIG_THUMB2_KERNEL

----

Dave Martin (9):
  ARM: kexec: Correct data alignment for CONFIG_THUMB2_KERNEL
  ARM: vfp: Correct data alignment for CONFIG_THUMB2_KERNEL
  ARM: Thumb-2: Correct data alignment for CONFIG_THUMB2_KERNEL in
    bootp/init.S
  ARM: Thumb-2: Correct data alignment for CONFIG_THUMB2_KERNEL in
    kernel/head.S
  ARM: Thumb-2: Correct data alignment for CONFIG_THUMB2_KERNEL in
    mm/proc-v7.S
  ARM: Thumb-2: Fix CONFIG_THUMB2_KERNEL breakage in compressed/head.S
  ARM: Thumb-2: Restore sensible zImage header layout for
    CONFIG_THUMB2_KERNEL
  ARM: Thumb-2: Fix long-distance conditional branches in head.S for
    Thumb-2.
  ARM: kprobes: Don't HAVE_KPROBES when CONFIG_THUMB2_KERNEL is
    selected

 arch/arm/Kconfig                  |    2 +-
 arch/arm/boot/Makefile            |    5 -----
 arch/arm/boot/bootp/init.S        |    2 ++
 arch/arm/boot/compressed/head.S   |   15 +++++++++++----
 arch/arm/kernel/head.S            |    7 +++++++
 arch/arm/kernel/relocate_kernel.S |    2 ++
 arch/arm/mm/proc-v7.S             |    4 ++--
 arch/arm/vfp/vfphw.S              |    1 +
 8 files changed, 26 insertions(+), 12 deletions(-)

[PATCH v2 1/9] ARM: kexec: Correct data alignment for CONFIG_THUMB2_KERNEL

[Dave Martin]

Directives such as .long and .word do not magically cause the
assembler location counter to become aligned in gas.  As a
result, using these directives in code sections can result in
misaligned data words when building a Thumb-2 kernel
(CONFIG_THUMB2_KERNEL).

This is a Bad Thing, since the ABI permits the compiler to
assume that fundamental types of word size or above are word-
aligned when accessing them from C.  If the data is not really
word-aligned, this can cause impaired performance and stray
alignment faults in some circumstances.

In general, the following rules should be applied when using
data word declaration directives inside code sections:

    * .quad and .double:
         .align 3

    * .long, .word, .single, .float:
         .align (or .align 2)

    * .short:
        No explicit alignment required, since Thumb-2
        instructions are always 2 or 4 bytes in size.
        immediately after an instruction.

Applies cleanly on v2.6.37-rc1.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/relocate_kernel.S |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/relocate_kernel.S b/arch/arm/kernel/relocate_kernel.S
index fd26f8d..9cf4cbf 100644
--- a/arch/arm/kernel/relocate_kernel.S
+++ b/arch/arm/kernel/relocate_kernel.S
@@ -59,6 +59,8 @@ relocate_new_kernel:
 	ldr r2,kexec_boot_atags
 	mov pc,lr
 
+	.align
+
 	.globl kexec_start_address
 kexec_start_address:
 	.long	0x0
-- 
1.7.1

[PATCH v2 2/9] ARM: vfp: Correct data alignment for CONFIG_THUMB2_KERNEL

[Dave Martin]

Directives such as .long and .word do not magically cause the
assembler location counter to become aligned in gas.  As a
result, using these directives in code sections can result in
misaligned data words when building a Thumb-2 kernel
(CONFIG_THUMB2_KERNEL).

This is a Bad Thing, since the ABI permits the compiler to
assume that fundamental types of word size or above are word-
aligned when accessing them from C.  If the data is not really
word-aligned, this can cause impaired performance and stray
alignment faults in some circumstances.

In general, the following rules should be applied when using
data word declaration directives inside code sections:

    * .quad and .double:
         .align 3

    * .long, .word, .single, .float:
         .align (or .align 2)

    * .short:
        No explicit alignment required, since Thumb-2
        instructions are always 2 or 4 bytes in size.
        immediately after an instruction.

Applies cleanly on v2.6.37-rc1.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/vfp/vfphw.S |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
index d66cead..9897dcf 100644
--- a/arch/arm/vfp/vfphw.S
+++ b/arch/arm/vfp/vfphw.S
@@ -206,6 +206,7 @@ ENTRY(vfp_save_state)
 	mov	pc, lr
 ENDPROC(vfp_save_state)
 
+	.align
 last_VFP_context_address:
 	.word	last_VFP_context
 
-- 
1.7.1

[PATCH v2 3/9] ARM: Thumb-2: Correct data alignment for CONFIG_THUMB2_KERNEL in bootp/init.S

[Dave Martin]

Directives such as .long and .word do not magically cause the
assembler location counter to become aligned in gas.  As a
result, using these directives in code sections can result in
misaligned data words when building a Thumb-2 kernel
(CONFIG_THUMB2_KERNEL).

This is a Bad Thing, since the ABI permits the compiler to
assume that fundamental types of word size or above are word-
aligned when accessing them from C.  If the data is not really
word-aligned, this can cause impaired performance and stray
alignment faults in some circumstances.

In general, the following rules should be applied when using
data word declaration directives inside code sections:

    * .quad and .double:
         .align 3

    * .long, .word, .single, .float:
         .align (or .align 2)

    * .short:
        No explicit alignment required, since Thumb-2
        instructions are always 2 or 4 bytes in size.
        immediately after an instruction.

Applies cleanly on v2.6.37-rc1.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/boot/bootp/init.S |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/arm/boot/bootp/init.S b/arch/arm/boot/bootp/init.S
index 8b0de41..78b5080 100644
--- a/arch/arm/boot/bootp/init.S
+++ b/arch/arm/boot/bootp/init.S
@@ -73,6 +73,8 @@ move:		ldmia	r4!, {r7 - r10}		@ move 32-bytes at a time
 
 		.size	_start, . - _start
 
+		.align
+
 		.type	data,#object
 data:		.word	initrd_start		@ source initrd address
 		.word	initrd_phys		@ destination initrd address
-- 
1.7.1

[PATCH v2 4/9] ARM: Thumb-2: Correct data alignment for CONFIG_THUMB2_KERNEL in kernel/head.S

[Dave Martin]

Directives such as .long and .word do not magically cause the
assembler location counter to become aligned in gas.  As a
result, using these directives in code sections can result in
misaligned data words when building a Thumb-2 kernel
(CONFIG_THUMB2_KERNEL).

This is a Bad Thing, since the ABI permits the compiler to
assume that fundamental types of word size or above are word-
aligned when accessing them from C.  If the data is not really
word-aligned, this can cause impaired performance and stray
alignment faults in some circumstances.

In general, the following rules should be applied when using
data word declaration directives inside code sections:

    * .quad and .double:
         .align 3

    * .long, .word, .single, .float:
         .align (or .align 2)

    * .short:
        No explicit alignment required, since Thumb-2
        instructions are always 2 or 4 bytes in size.
        immediately after an instruction.

Applies cleanly on v2.6.37-rc1.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/kernel/head.S |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index dd6b369..591c097 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -262,6 +262,7 @@ __create_page_tables:
 	mov	pc, lr
 ENDPROC(__create_page_tables)
 	.ltorg
+	.align
 __enable_mmu_loc:
 	.long	.
 	.long	__enable_mmu
@@ -308,6 +309,8 @@ ENTRY(__secondary_switched)
 	b	secondary_start_kernel
 ENDPROC(__secondary_switched)
 
+	.align
+
 	.type	__secondary_data, %object
 __secondary_data:
 	.long	.
@@ -413,6 +416,7 @@ __fixup_smp_on_up:
 	mov	pc, lr
 ENDPROC(__fixup_smp)
 
+	.align
 1:	.word	.
 	.word	__smpalt_begin
 	.word	__smpalt_end
-- 
1.7.1

[PATCH v2 5/9] ARM: Thumb-2: Correct data alignment for CONFIG_THUMB2_KERNEL in mm/proc-v7.S

[Dave Martin]

Directives such as .long and .word do not magically cause the
assembler location counter to become aligned in gas.  As a
result, using these directives in code sections can result in
misaligned data words when building a Thumb-2 kernel
(CONFIG_THUMB2_KERNEL).

This is a Bad Thing, since the ABI permits the compiler to
assume that fundamental types of word size or above are word-
aligned when accessing them from C.  If the data is not really
word-aligned, this can cause impaired performance and stray
alignment faults in some circumstances.

In general, the following rules should be applied when using
data word declaration directives inside code sections:

    * .quad and .double:
         .align 3

    * .long, .word, .single, .float:
         .align (or .align 2)

    * .short:
        No explicit alignment required, since Thumb-2
        instructions are always 2 or 4 bytes in size.
        immediately after an instruction.

In this specific case, we can achieve the desired alignment by
forcing a 32-bit branch instruction using the W() macro,
since the assembler location counter is already 32-bit aligned in this case.

Applies cleanly on v2.6.37-rc1.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/mm/proc-v7.S |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index 53cbe22..9b9ff5d 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -381,7 +381,7 @@ __v7_ca9mp_proc_info:
 		PMD_SECT_XN | \
 		PMD_SECT_AP_WRITE | \
 		PMD_SECT_AP_READ
-	b	__v7_ca9mp_setup
+	W(b)	__v7_ca9mp_setup
 	.long	cpu_arch_name
 	.long	cpu_elf_name
 	.long	HWCAP_SWP|HWCAP_HALF|HWCAP_THUMB|HWCAP_FAST_MULT|HWCAP_EDSP|HWCAP_TLS
@@ -413,7 +413,7 @@ __v7_proc_info:
 		PMD_SECT_XN | \
 		PMD_SECT_AP_WRITE | \
 		PMD_SECT_AP_READ
-	b	__v7_setup
+	W(b)	__v7_setup
 	.long	cpu_arch_name
 	.long	cpu_elf_name
 	.long	HWCAP_SWP|HWCAP_HALF|HWCAP_THUMB|HWCAP_FAST_MULT|HWCAP_EDSP|HWCAP_TLS
-- 
1.7.1

[PATCH v2 6/9] ARM: Thumb-2: Fix CONFIG_THUMB2_KERNEL breakage in compressed/head.S

[Dave Martin]

Some instruction operand combinations are used here
which are nor permitted in Thumb-2.

In particular, most uses of pc as an operand are
disallowed in Thumb-2, and deprecated in ARM from
ARMv7 onwards.

The modified code introduced by this patch should be
compatible with all architecture versions >= v3, with
or without CONFIG_THUMB2_KERNEL.

Applies cleanly on v2.6.37-rc1.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Reviewed-by: Will Deacon <will.deacon@arm.com>
---
 arch/arm/boot/compressed/head.S |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 6825c34..1f65080 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -174,7 +174,8 @@ not_angel:
 		ldr	sp, [r0, #28]
 #ifdef CONFIG_AUTO_ZRELADDR
 		@ determine final kernel image address
-		and	r4, pc, #0xf8000000
+		mov	r4, pc
+		and	r4, r4, #0xf8000000
 		add	r4, r4, #TEXT_OFFSET
 #else
 		ldr	r4, =zreladdr
@@ -445,7 +446,8 @@ __setup_mmu:	sub	r3, r4, #16384		@ Page directory size
  */
 		mov	r1, #0x1e
 		orr	r1, r1, #3 << 10
-		mov	r2, pc, lsr #20
+		mov	r2, pc
+		mov	r2, r2, lsr #20
 		orr	r1, r1, r2, lsl #20
 		add	r0, r3, r2, lsl #2
 		str	r1, [r0], #4
-- 
1.7.1

[PATCH v2 7/9] ARM: Thumb-2: Restore sensible zImage header layout for CONFIG_THUMB2_KERNEL

[Dave Martin]

The code which makes up the zImage header intends to leave a
32-byte gap followed by a branch to the real entry point, a
magic number, and a word containing the absolute entry point
address.

This gets messed up with with CONFIG_THUMB2_KERNEL, because the
size of the initial padding NOPs changes.

Instead, the header can be made fully compatible by restoring
it to ARM.

In the Thumb-2 case, we can replace the initial NOPs with a
sequence which switches to Thumb and jumps to the real entry
point.

As a consequence, the zImage entry point is now always ARM, so
no special magic is needed any more for the uImage rules in the
Thumb-2 case.

Applies on v2.6.37-rc3.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 arch/arm/boot/Makefile          |    5 -----
 arch/arm/boot/compressed/head.S |    9 +++++++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/boot/Makefile b/arch/arm/boot/Makefile
index 4a590f4..4d26f2c 100644
--- a/arch/arm/boot/Makefile
+++ b/arch/arm/boot/Makefile
@@ -70,12 +70,7 @@ else
 $(obj)/uImage: LOADADDR=$(ZRELADDR)
 endif
 
-ifeq ($(CONFIG_THUMB2_KERNEL),y)
-# Set bit 0 to 1 so that "mov pc, rx" switches to Thumb-2 mode
-$(obj)/uImage: STARTADDR=$(shell echo $(LOADADDR) | sed -e "s/.$$/1/")
-else
 $(obj)/uImage: STARTADDR=$(LOADADDR)
-endif
 
 $(obj)/uImage:	$(obj)/zImage FORCE
 	$(call if_changed,uimage)
diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 1f65080..b342670 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -125,16 +125,21 @@ wait:		mrc	p14, 0, pc, c0, c1, 0
  * sort out different calling conventions
  */
 		.align
+		.arm				@ Always enter in ARM state
 start:
 		.type	start,#function
-		.rept	8
+ THUMB(		adr	r12, BSYM(1f)	)
+ THUMB(		bx	r12		)
+ THUMB(		.rept	6		)
+ ARM(		.rept	8		)
 		mov	r0, r0
 		.endr
 
-		b	1f
+		nop				@ Pad magic number to 0x24
 		.word	0x016f2818		@ Magic numbers to help the loader
 		.word	start			@ absolute load/run zImage address
 		.word	_edata			@ zImage end address
+ THUMB(		.thumb			)
 1:		mov	r7, r1			@ save architecture ID
 		mov	r8, r2			@ save atags pointer
 
-- 
1.7.1

[PATCH v2 8/9] ARM: Thumb-2: Fix long-distance conditional branches in head.S for Thumb-2.

[Dave Martin]

The 32-bit conditional branches in Thumb-2 have a shorter range (+/-512K)
than their ARM counterparts (+/-32MB).  The linker does not currently
generate trampolines to extend the range of these Thumb-2 conditional
branches, resulting in link errors when vmlinux is sufficiently large, e.g.:

head.o:(.text+0x464): relocation truncated to fit: R_ARM_THM_JUMP19

This patch forces the longer-range, unconditional branch encoding by
use of an explicit IT instruction.  The resulting branches are triggered on
the same conditions as before.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/kernel/head.S |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 591c097..6bd82d2 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -85,9 +85,11 @@ ENTRY(stext)
 	mrc	p15, 0, r9, c0, c0		@ get processor id
 	bl	__lookup_processor_type		@ r5=procinfo r9=cpuid
 	movs	r10, r5				@ invalid processor (r5=0)?
+ THUMB( it	eq )		@ force fixup-able long branch encoding
 	beq	__error_p			@ yes, error 'p'
 	bl	__lookup_machine_type		@ r5=machinfo
 	movs	r8, r5				@ invalid machine (r5=0)?
+ THUMB( it	eq )		@ force fixup-able long branch encoding
 	beq	__error_a			@ yes, error 'a'
 	bl	__vet_atags
 #ifdef CONFIG_SMP_ON_UP
@@ -283,6 +285,7 @@ ENTRY(secondary_startup)
 	bl	__lookup_processor_type
 	movs	r10, r5				@ invalid processor?
 	moveq	r0, #'p'			@ yes, error 'p'
+ THUMB( it	eq )		@ force fixup-able long branch encoding
 	beq	__error_p
 
 	/*
-- 
1.7.1

[PATCH v2 9/9] ARM: kprobes: Don't HAVE_KPROBES when CONFIG_THUMB2_KERNEL is selected

[Dave Martin]

Currently, the kprobes implementation for ARM only supports
the ARM instruction set, so it only works if
CONFIG_THUMB2_KERNEL is not enabled.

Until kprobes is updated to work with Thumb-2, turning it on
will cause horrible things to happen, so this patch disables it
for now.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/Kconfig |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index db524e7..f1d9297 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -9,7 +9,7 @@ config ARM
 	select GENERIC_ATOMIC64 if (!CPU_32v6K || !AEABI)
 	select HAVE_OPROFILE if (HAVE_PERF_EVENTS)
 	select HAVE_ARCH_KGDB
-	select HAVE_KPROBES if (!XIP_KERNEL)
+	select HAVE_KPROBES if (!XIP_KERNEL && !THUMB2_KERNEL)
 	select HAVE_KRETPROBES if (HAVE_KPROBES)
 	select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
 	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
-- 
1.7.1

[PATCH v2 7/9] ARM: Thumb-2: Restore sensible zImage header layout for CONFIG_THUMB2_KERNEL

[Nicolas Pitre]

On Mon, 22 Nov 2010, Dave Martin wrote:

> The code which makes up the zImage header intends to leave a
> 32-byte gap followed by a branch to the real entry point, a
> magic number, and a word containing the absolute entry point
> address.
> 
> This gets messed up with with CONFIG_THUMB2_KERNEL, because the
> size of the initial padding NOPs changes.
> 
> Instead, the header can be made fully compatible by restoring
> it to ARM.
> 
> In the Thumb-2 case, we can replace the initial NOPs with a
> sequence which switches to Thumb and jumps to the real entry
> point.
> 
> As a consequence, the zImage entry point is now always ARM, so
> no special magic is needed any more for the uImage rules in the
> Thumb-2 case.
> 
> Applies on v2.6.37-rc3.

Ideally you should keep that line above outside of the actual commit log 
text, as no one will care on what version it applies to once it is 
merged.  It is best to put that below the --- line, or omit it entirely 
if using 'git send-email'.

> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>

Looking at it again there is a problem with the patch as is...

> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -125,16 +125,21 @@ wait:		mrc	p14, 0, pc, c0, c1, 0
>   * sort out different calling conventions
>   */
>  		.align
> +		.arm				@ Always enter in ARM state
>  start:
>  		.type	start,#function
> -		.rept	8
> + THUMB(		adr	r12, BSYM(1f)	)
> + THUMB(		bx	r12		)
> + THUMB(		.rept	6		)
> + ARM(		.rept	8		)
>  		mov	r0, r0
>  		.endr
>  
> -		b	1f
> +		nop				@ Pad magic number to 0x24

Why is this branch removed?  With an ARM mode kernel this means we'll 
attempt to execute the magic number that follows.

>  		.word	0x016f2818		@ Magic numbers to help the loader
>  		.word	start			@ absolute load/run zImage address
>  		.word	_edata			@ zImage end address
> + THUMB(		.thumb			)
>  1:		mov	r7, r1			@ save architecture ID
>  		mov	r8, r2			@ save atags pointer
>  

Nicolas

[PATCH v2 9/9] ARM: kprobes: Don't HAVE_KPROBES when CONFIG_THUMB2_KERNEL is selected

[Nicolas Pitre]

On Mon, 22 Nov 2010, Dave Martin wrote:

> Currently, the kprobes implementation for ARM only supports
> the ARM instruction set, so it only works if
> CONFIG_THUMB2_KERNEL is not enabled.
> 
> Until kprobes is updated to work with Thumb-2, turning it on
> will cause horrible things to happen, so this patch disables it
> for now.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>

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


> ---
>  arch/arm/Kconfig |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index db524e7..f1d9297 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -9,7 +9,7 @@ config ARM
>  	select GENERIC_ATOMIC64 if (!CPU_32v6K || !AEABI)
>  	select HAVE_OPROFILE if (HAVE_PERF_EVENTS)
>  	select HAVE_ARCH_KGDB
> -	select HAVE_KPROBES if (!XIP_KERNEL)
> +	select HAVE_KPROBES if (!XIP_KERNEL && !THUMB2_KERNEL)
>  	select HAVE_KRETPROBES if (HAVE_KPROBES)
>  	select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
>  	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH v2 7/9] ARM: Thumb-2: Restore sensible zImage header layout for CONFIG_THUMB2_KERNEL

[Dave Martin]

Hi,

On Mon, Nov 22, 2010 at 6:28 PM, Nicolas Pitre <nico@fluxnic.net> wrote:

[...]

>> Applies on v2.6.37-rc3.
>
> Ideally you should keep that line above outside of the actual commit log
> text, as no one will care on what version it applies to once it is
> merged. ?It is best to put that below the --- line, or omit it entirely
> if using 'git send-email'.

OK, I'll leave it out in the future...

>> Signed-off-by: Dave Martin <dave.martin@linaro.org>
>> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
>> Acked-by: Nicolas Pitre <nicolas.pitre@linaro.org>
>
> Looking at it again there is a problem with the patch as is...
>
>> --- a/arch/arm/boot/compressed/head.S
>> +++ b/arch/arm/boot/compressed/head.S
>> @@ -125,16 +125,21 @@ wait: ? ? ? ? ? mrc ? ? p14, 0, pc, c0, c1, 0
>> ? * sort out different calling conventions
>> ? */
>> ? ? ? ? ? ? ? .align
>> + ? ? ? ? ? ? .arm ? ? ? ? ? ? ? ? ? ? ? ? ? ?@ Always enter in ARM state
>> ?start:
>> ? ? ? ? ? ? ? .type ? start,#function
>> - ? ? ? ? ? ? .rept ? 8
>> + THUMB( ? ? ? ? ? ? ?adr ? ? r12, BSYM(1f) ? )
>> + THUMB( ? ? ? ? ? ? ?bx ? ? ?r12 ? ? ? ? ? ? )
>> + THUMB( ? ? ? ? ? ? ?.rept ? 6 ? ? ? ? ? ? ? )
>> + ARM( ? ? ? ? ? ? ? ?.rept ? 8 ? ? ? ? ? ? ? )
>> ? ? ? ? ? ? ? mov ? ? r0, r0
>> ? ? ? ? ? ? ? .endr
>>
>> - ? ? ? ? ? ? b ? ? ? 1f
>> + ? ? ? ? ? ? nop ? ? ? ? ? ? ? ? ? ? ? ? ? ? @ Pad magic number to 0x24
>
> Why is this branch removed? ?With an ARM mode kernel this means we'll
> attempt to execute the magic number that follows.

Hmmm, I think I got confused during a branch merge...

I will retest and repost this patch as appropriate.

Cheers
---Dave

[PATCH v2 8/9] ARM: Thumb-2: Fix long-distance conditional branches in head.S for Thumb-2.

[Nicolas Pitre]

On Mon, 22 Nov 2010, Dave Martin wrote:

> The 32-bit conditional branches in Thumb-2 have a shorter range (+/-512K)
> than their ARM counterparts (+/-32MB).  The linker does not currently
> generate trampolines to extend the range of these Thumb-2 conditional
> branches, resulting in link errors when vmlinux is sufficiently large, e.g.:
> 
> head.o:(.text+0x464): relocation truncated to fit: R_ARM_THM_JUMP19
> 
> This patch forces the longer-range, unconditional branch encoding by
> use of an explicit IT instruction.  The resulting branches are triggered on
> the same conditions as before.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>

I'm afraid this patch is only making an actual bug visible.  There is no 
reason why __error_p or __error_a ought to be farther than 512K away 
from their call sites as they are all supposed to live in the __init 
section close together.  It looks like the refactoring done in commit 
75d90832d5 has dropped the __init attribute from those moved functions, 
and adding it back would be the real fix.


Nicolas

[PATCH v2 6/9] ARM: Thumb-2: Fix CONFIG_THUMB2_KERNEL breakage in compressed/head.S

[Nicolas Pitre]

On Mon, 22 Nov 2010, Dave Martin wrote:

> Some instruction operand combinations are used here
> which are nor permitted in Thumb-2.
> 
> In particular, most uses of pc as an operand are
> disallowed in Thumb-2, and deprecated in ARM from
> ARMv7 onwards.
> 
> The modified code introduced by this patch should be
> compatible with all architecture versions >= v3, with
> or without CONFIG_THUMB2_KERNEL.
> 
> Applies cleanly on v2.6.37-rc1.
> 
> Signed-off-by: Dave Martin <dave.martin@linaro.org>
> Acked-by: Catalin Marinas <catalin.marinas@arm.com>
> Reviewed-by: Will Deacon <will.deacon@arm.com>

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

> ---
>  arch/arm/boot/compressed/head.S |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index 6825c34..1f65080 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -174,7 +174,8 @@ not_angel:
>  		ldr	sp, [r0, #28]
>  #ifdef CONFIG_AUTO_ZRELADDR
>  		@ determine final kernel image address
> -		and	r4, pc, #0xf8000000
> +		mov	r4, pc
> +		and	r4, r4, #0xf8000000
>  		add	r4, r4, #TEXT_OFFSET
>  #else
>  		ldr	r4, =zreladdr
> @@ -445,7 +446,8 @@ __setup_mmu:	sub	r3, r4, #16384		@ Page directory size
>   */
>  		mov	r1, #0x1e
>  		orr	r1, r1, #3 << 10
> -		mov	r2, pc, lsr #20
> +		mov	r2, pc
> +		mov	r2, r2, lsr #20
>  		orr	r1, r1, r2, lsl #20
>  		add	r0, r3, r2, lsl #2
>  		str	r1, [r0], #4
> -- 
> 1.7.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH v2 8/9] ARM: Thumb-2: Fix long-distance conditional branches in head.S for Thumb-2.

[Russell King - ARM Linux]

On Mon, Nov 22, 2010 at 02:03:51PM -0500, Nicolas Pitre wrote:
> On Mon, 22 Nov 2010, Dave Martin wrote:
> 
> > The 32-bit conditional branches in Thumb-2 have a shorter range (+/-512K)
> > than their ARM counterparts (+/-32MB).  The linker does not currently
> > generate trampolines to extend the range of these Thumb-2 conditional
> > branches, resulting in link errors when vmlinux is sufficiently large, e.g.:
> > 
> > head.o:(.text+0x464): relocation truncated to fit: R_ARM_THM_JUMP19
> > 
> > This patch forces the longer-range, unconditional branch encoding by
> > use of an explicit IT instruction.  The resulting branches are triggered on
> > the same conditions as before.
> > 
> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> 
> I'm afraid this patch is only making an actual bug visible.  There is no 
> reason why __error_p or __error_a ought to be farther than 512K away 
> from their call sites as they are all supposed to live in the __init 
> section close together.  It looks like the refactoring done in commit 
> 75d90832d5 has dropped the __init attribute from those moved functions, 
> and adding it back would be the real fix.

No, you're not thinking right.  It was _intentional_ that __error_p
ended up in a different section to __init.  With CPU hotplug, as it
is called from the hotplug CPU initialization path, it needs to be
available for that path to call.

So it can't be thrown away with the init code when hotplug is enabled.

The sections which the code end up in is correct as-is.

[PATCH v2 8/9] ARM: Thumb-2: Fix long-distance conditional branches in head.S for Thumb-2.

[Dave Martin]

Hi,

On Mon, Nov 22, 2010 at 7:17 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Mon, Nov 22, 2010 at 02:03:51PM -0500, Nicolas Pitre wrote:
>> On Mon, 22 Nov 2010, Dave Martin wrote:
>>
>> > The 32-bit conditional branches in Thumb-2 have a shorter range (+/-512K)
>> > than their ARM counterparts (+/-32MB). ?The linker does not currently
>> > generate trampolines to extend the range of these Thumb-2 conditional
>> > branches, resulting in link errors when vmlinux is sufficiently large, e.g.:
>> >
>> > head.o:(.text+0x464): relocation truncated to fit: R_ARM_THM_JUMP19
>> >
>> > This patch forces the longer-range, unconditional branch encoding by
>> > use of an explicit IT instruction. ?The resulting branches are triggered on
>> > the same conditions as before.
>> >
>> > Signed-off-by: Dave Martin <dave.martin@linaro.org>
>>
>> I'm afraid this patch is only making an actual bug visible. ?There is no
>> reason why __error_p or __error_a ought to be farther than 512K away
>> from their call sites as they are all supposed to live in the __init
>> section close together. ?It looks like the refactoring done in commit
>> 75d90832d5 has dropped the __init attribute from those moved functions,
>> and adding it back would be the real fix.
>
> No, you're not thinking right. ?It was _intentional_ that __error_p
> ended up in a different section to __init. ?With CPU hotplug, as it
> is called from the hotplug CPU initialization path, it needs to be
> available for that path to call.
>
> So it can't be thrown away with the init code when hotplug is enabled.
>
> The sections which the code end up in is correct as-is.
>

I will infer from this that the patch is useful/correct in its current state...

Let me know if anything needs to change.

Cheers
---Dave

[PATCH v2 8/9] ARM: Thumb-2: Fix long-distance conditional branches in head.S for Thumb-2.

[Nicolas Pitre]

On Mon, 22 Nov 2010, Russell King - ARM Linux wrote:

> On Mon, Nov 22, 2010 at 02:03:51PM -0500, Nicolas Pitre wrote:
> > On Mon, 22 Nov 2010, Dave Martin wrote:
> > 
> > > The 32-bit conditional branches in Thumb-2 have a shorter range (+/-512K)
> > > than their ARM counterparts (+/-32MB).  The linker does not currently
> > > generate trampolines to extend the range of these Thumb-2 conditional
> > > branches, resulting in link errors when vmlinux is sufficiently large, e.g.:
> > > 
> > > head.o:(.text+0x464): relocation truncated to fit: R_ARM_THM_JUMP19
> > > 
> > > This patch forces the longer-range, unconditional branch encoding by
> > > use of an explicit IT instruction.  The resulting branches are triggered on
> > > the same conditions as before.
> > > 
> > > Signed-off-by: Dave Martin <dave.martin@linaro.org>
> > 
> > I'm afraid this patch is only making an actual bug visible.  There is no 
> > reason why __error_p or __error_a ought to be farther than 512K away 
> > from their call sites as they are all supposed to live in the __init 
> > section close together.  It looks like the refactoring done in commit 
> > 75d90832d5 has dropped the __init attribute from those moved functions, 
> > and adding it back would be the real fix.
> 
> No, you're not thinking right.  It was _intentional_ that __error_p
> ended up in a different section to __init.  With CPU hotplug, as it
> is called from the hotplug CPU initialization path, it needs to be
> available for that path to call.

Well... agreed for the hotplug CPU case.  The section selection is good 
as it is for that.

However I doubt this was intentional when commit 75d90832d5 was created, 
which is when the actual section move took place.


Nicolas

[PATCH v2 9/9] ARM: kprobes: Don't HAVE_KPROBES when CONFIG_THUMB2_KERNEL is selected

[Sergei Shtylyov]

Hello.

On 22-11-2010 21:04, Dave Martin wrote:

> Currently, the kprobes implementation for ARM only supports
> the ARM instruction set, so it only works if
> CONFIG_THUMB2_KERNEL is not enabled.

> Until kprobes is updated to work with Thumb-2, turning it on
> will cause horrible things to happen, so this patch disables it
> for now.

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

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index db524e7..f1d9297 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -9,7 +9,7 @@ config ARM
>   	select GENERIC_ATOMIC64 if (!CPU_32v6K || !AEABI)
>   	select HAVE_OPROFILE if (HAVE_PERF_EVENTS)
>   	select HAVE_ARCH_KGDB
> -	select HAVE_KPROBES if (!XIP_KERNEL)
> +	select HAVE_KPROBES if (!XIP_KERNEL && !THUMB2_KERNEL)

    These parens are useless.

WBR, Sergei

[PATCH v2 8/9] ARM: Thumb-2: Fix long-distance conditional branches in head.S for Thumb-2.

[Russell King - ARM Linux]

On Mon, Nov 22, 2010 at 03:24:11PM -0500, Nicolas Pitre wrote:
> On Mon, 22 Nov 2010, Russell King - ARM Linux wrote:
> > No, you're not thinking right.  It was _intentional_ that __error_p
> > ended up in a different section to __init.  With CPU hotplug, as it
> > is called from the hotplug CPU initialization path, it needs to be
> > available for that path to call.
> 
> Well... agreed for the hotplug CPU case.  The section selection is good 
> as it is for that.
> 
> However I doubt this was intentional when commit 75d90832d5 was created, 
> which is when the actual section move took place.

Well, T2 is not something I particularly test for.  There's already too
many combinations to test as it is.

[PATCH v2 8/9] ARM: Thumb-2: Fix long-distance conditional branches in head.S for Thumb-2.

[Nicolas Pitre]

On Mon, 22 Nov 2010, Russell King - ARM Linux wrote:

> On Mon, Nov 22, 2010 at 03:24:11PM -0500, Nicolas Pitre wrote:
> > On Mon, 22 Nov 2010, Russell King - ARM Linux wrote:
> > > No, you're not thinking right.  It was _intentional_ that __error_p
> > > ended up in a different section to __init.  With CPU hotplug, as it
> > > is called from the hotplug CPU initialization path, it needs to be
> > > available for that path to call.
> > 
> > Well... agreed for the hotplug CPU case.  The section selection is good 
> > as it is for that.
> > 
> > However I doubt this was intentional when commit 75d90832d5 was created, 
> > which is when the actual section move took place.
> 
> Well, T2 is not something I particularly test for.  There's already too
> many combinations to test as it is.

Fair enough.  My point was only about the unintentional drop of the 
__init section attribute for the __error_p and __error_a functions (and 
probably some others in head-common.S) from commit 75d9083 which is 
orthogonal to the T2 issue.  It looks that this bug has turned itself 
into a feature that the CPU hotplug support depends on, although I'd 
probably prefer if there was a __hotplug_init section that would still 
be discarded after boot when CPU hotplug support is not configured in.


Nicolas

[PATCH v2 8/9] ARM: Thumb-2: Fix long-distance conditional branches in head.S for Thumb-2.

[Nicolas Pitre]

On Mon, 22 Nov 2010, Dave Martin wrote:

> On Mon, Nov 22, 2010 at 7:17 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> 
> > With CPU hotplug, as it is called from the hotplug CPU 
> > initialization path, it needs to be available for that path to call.
> >
> > So it can't be thrown away with the init code when hotplug is enabled.
> 
> I will infer from this that the patch is useful/correct in its current state...

Right.


Nicolas

[PATCH v2 8/9] ARM: Thumb-2: Fix long-distance conditional branches in head.S for Thumb-2.

[Russell King - ARM Linux]

On Mon, Nov 22, 2010 at 07:26:51PM -0500, Nicolas Pitre wrote:
> On Mon, 22 Nov 2010, Russell King - ARM Linux wrote:
> 
> > On Mon, Nov 22, 2010 at 03:24:11PM -0500, Nicolas Pitre wrote:
> > > On Mon, 22 Nov 2010, Russell King - ARM Linux wrote:
> > > > No, you're not thinking right.  It was _intentional_ that __error_p
> > > > ended up in a different section to __init.  With CPU hotplug, as it
> > > > is called from the hotplug CPU initialization path, it needs to be
> > > > available for that path to call.
> > > 
> > > Well... agreed for the hotplug CPU case.  The section selection is good 
> > > as it is for that.
> > > 
> > > However I doubt this was intentional when commit 75d90832d5 was created, 
> > > which is when the actual section move took place.
> > 
> > Well, T2 is not something I particularly test for.  There's already too
> > many combinations to test as it is.
> 
> Fair enough.  My point was only about the unintentional drop of the 
> __init section attribute for the __error_p and __error_a functions (and 
> probably some others in head-common.S) from commit 75d9083 which is 
> orthogonal to the T2 issue.

What are you talking about Nicolas?

75d9083 is about adding nommu support, which was added some 4 years ago
well before Thumb 2 was talked about.  It moves some code out of head.S,
and creates a new file, head-common.S to contain this code.  head-common.S
is included by head.S, which means the assembler sees it as a single file.

And to prove the point, this is what head.o looks like immediately after
commit 75d9083:

00000000 l    d  .text  00000000 .text
00000000 l    d  .data  00000000 .data
00000000 l    d  .bss   00000000 .bss
00000000 l    d  .init.text     00000000 .init.text
00000000 g     F .init.text     00000000 stext
00000030 l     F .init.text     00000000 __enable_mmu
00000060 l     F .init.text     00000000 __turn_mmu_on
00000078 l     F .init.text     00000000 __create_page_tables
00000118 l     O .init.text     00000000 __switch_data
0000013c l     F .init.text     00000000 __mmap_switched
00000180 l     F .init.text     00000000 __error_p
0000018c l       .init.text     00000000 str_p1
000001c4 l     F .init.text     00000000 __error_a
00000230 l       .init.text     00000000 str_a1
00000265 l       .init.text     00000000 str_a2
00000294 l       .init.text     00000000 str_a3
000002cc l     F .init.text     00000000 __error
000002d4 l     F .init.text     00000000 __lookup_processor_type
0000030c g       .init.text     00000000 lookup_processor_type
00000334 l     F .init.text     00000000 __lookup_machine_type
00000368 g       .init.text     00000000 lookup_machine_type
c0004000 g       *ABS*  00000000 swapper_pg_dir

As you can see, everything after this commit is in the .init.text section,
including the __error_p and __error_a functions.

[PATCH v2 9/9] ARM: kprobes: Don't HAVE_KPROBES when CONFIG_THUMB2_KERNEL is selected

[Dave Martin]

Hi,

On Mon, Nov 22, 2010 at 11:01 PM, Sergei Shtylyov <sshtylyov@mvista.com> wrote:
> Hello.

[...]

>> @@ -9,7 +9,7 @@ config ARM
>> ? ? ? ?select GENERIC_ATOMIC64 if (!CPU_32v6K || !AEABI)
>> ? ? ? ?select HAVE_OPROFILE if (HAVE_PERF_EVENTS)
>> ? ? ? ?select HAVE_ARCH_KGDB
>> - ? ? ? select HAVE_KPROBES if (!XIP_KERNEL)
>> + ? ? ? select HAVE_KPROBES if (!XIP_KERNEL && !THUMB2_KERNEL)
>
> ? These parens are useless.

Maybe so, but I prefer to keep the style consistent with the rest of the file...

Cheers
---Dave

[PATCH v2 8/9] ARM: Thumb-2: Fix long-distance conditional branches in head.S for Thumb-2.

[Nicolas Pitre]

On Tue, 23 Nov 2010, Russell King - ARM Linux wrote:

> On Mon, Nov 22, 2010 at 07:26:51PM -0500, Nicolas Pitre wrote:
> > On Mon, 22 Nov 2010, Russell King - ARM Linux wrote:
> > 
> > > On Mon, Nov 22, 2010 at 03:24:11PM -0500, Nicolas Pitre wrote:
> > > > On Mon, 22 Nov 2010, Russell King - ARM Linux wrote:
> > > > > No, you're not thinking right.  It was _intentional_ that __error_p
> > > > > ended up in a different section to __init.  With CPU hotplug, as it
> > > > > is called from the hotplug CPU initialization path, it needs to be
> > > > > available for that path to call.
> > > > 
> > > > Well... agreed for the hotplug CPU case.  The section selection is good 
> > > > as it is for that.
> > > > 
> > > > However I doubt this was intentional when commit 75d90832d5 was created, 
> > > > which is when the actual section move took place.
> > > 
> > > Well, T2 is not something I particularly test for.  There's already too
> > > many combinations to test as it is.
> > 
> > Fair enough.  My point was only about the unintentional drop of the 
> > __init section attribute for the __error_p and __error_a functions (and 
> > probably some others in head-common.S) from commit 75d9083 which is 
> > orthogonal to the T2 issue.
> 
> What are you talking about Nicolas?
> 
> 75d9083 is about adding nommu support, which was added some 4 years ago
> well before Thumb 2 was talked about.

Exactly my point.

> It moves some code out of head.S,
> and creates a new file, head-common.S to contain this code.

So far so good.

> head-common.S is included by head.S, which means the assembler sees it 
> as a single file.

And that's where I screwed up with my analysis.  I somehow was under the 
impression this was a separate compilation unit.  Sorry for the noise.


Nicolas