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

[PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 4/7] ARM: Thumb-2: Correct data alignment for CONFIG_THUMB2_KERNEL in kernel/head.S

[Russell King - ARM Linux]

On Tue, Nov 16, 2010 at 01:14:34PM +0000, Dave Martin wrote:
> 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).

> @@ -413,6 +416,7 @@ __fixup_smp_on_up:
>  	mov	pc, lr
>  ENDPROC(__fixup_smp)
>  
> +	.align
>  1:	.word	.
>  	.word	__smpalt_begin
>  	.word	__smpalt_end

This one is technically bogus because this feature is disabled (and
incompatible) with Thumb-2 kernels.  So it never gets built for a T2
kernel.

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

[Catalin Marinas]

On Tue, 2010-11-16 at 18:22 +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 16, 2010 at 01:14:34PM +0000, Dave Martin wrote:
> > 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).
> 
> > @@ -413,6 +416,7 @@ __fixup_smp_on_up:
> >       mov     pc, lr
> >  ENDPROC(__fixup_smp)
> > 
> > +     .align
> >  1:   .word   .
> >       .word   __smpalt_begin
> >       .word   __smpalt_end
> 
> This one is technically bogus because this feature is disabled (and
> incompatible) with Thumb-2 kernels.  So it never gets built for a T2
> kernel.

Well, I think it doesn't cause any harm to have it in. It's on our to-do
list to allow T2 support for SMP-on-UP.

-- 
Catalin

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

[Dave Martin]

On Tue, Nov 16, 2010 at 6:22 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Nov 16, 2010 at 01:14:34PM +0000, Dave Martin wrote:
>> 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).
>
>> @@ -413,6 +416,7 @@ __fixup_smp_on_up:
>> ? ? ? mov ? ? pc, lr
>> ?ENDPROC(__fixup_smp)
>>
>> + ? ? .align
>> ?1: ? .word ? .
>> ? ? ? .word ? __smpalt_begin
>> ? ? ? .word ? __smpalt_end
>
> This one is technically bogus because this feature is disabled (and
> incompatible) with Thumb-2 kernels. ?So it never gets built for a T2
> kernel.
>

For now, this is true -- I'll soon be looking at making SMP_ON_UP work
for Thumb-2.

When and if that is implemented, I believe this fix will be appropriate.
Would it be better to hold this patch back and re-propose it with the
rest of the SMP_ON_UP changes?

Cheers
---Dave

[PATCH 5/7] 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 6/7] 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 7/7] ARM: Thumb-2: Restore sensible zImage header layout for CONFIG_THUMB2_KERNEL

[Dave Martin]

The code which makes up the zImage header clearly intends to
leave a vector-table-sized gap of 8 words (NOPs, in fact),
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:

    * The NOPs making up the vector table become halfword-
      sized.

    * The magic number and absolute entry point occur too early
      and become misaligned.

    * The absolute entry point fails to indicate that the entry
      point is Thumb code, which will cause incorrect execution
      if the bootloader uses this to enter the kernel.

This patch makes sure the same layout is generated in the
CONFIG_THUMB2_KERNEL case as in the traditional ARM case, and
sets the Thumb bit (bit 0) in the entry point field.  The ARM
case is unaffected.

This is my best guess as to how the zImage should be laid out
for Thumb-2.  U-Boot in particular uses its own metadata and
ignores the zImage header fields.

If someone knows of a bootloader or other utility which relies
on the strange existing Thumb-2 zImage header layout then this
may require more careful thought.

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, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
index 1f65080..1e36e2a 100644
--- a/arch/arm/boot/compressed/head.S
+++ b/arch/arm/boot/compressed/head.S
@@ -128,12 +128,12 @@ wait:		mrc	p14, 0, pc, c0, c1, 0
 start:
 		.type	start,#function
 		.rept	8
-		mov	r0, r0
+		W(nop)				@ use 32-bit NOPs for correct padding
 		.endr
 
-		b	1f
+		W(b)	1f
 		.word	0x016f2818		@ Magic numbers to help the loader
-		.word	start			@ absolute load/run zImage address
+		.word	BSYM(start)		@ absolute load/run zImage address
 		.word	_edata			@ zImage end address
 1:		mov	r7, r1			@ save architecture ID
 		mov	r8, r2			@ save atags pointer
-- 
1.7.1

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

[Russell King - ARM Linux]

On Tue, Nov 16, 2010 at 01:14:37PM +0000, Dave Martin wrote:
> The code which makes up the zImage header clearly intends to
> leave a vector-table-sized gap of 8 words (NOPs, in fact),
> followed by a branch to the real entry point, a magic number,
> and a word containing the absolute entry point address.

That's an incorrect assumption.  The set of 8 words have nothing to do
with the CPUs vector table at all - it has more to do with compatibility
with a.out built kernels, where the a.out header was 32 bytes.

> This gets messed up with with CONFIG_THUMB2_KERNEL:
> 
>     * The NOPs making up the vector table become halfword-
>       sized.

Doesn't matter.

>     * The magic number and absolute entry point occur too early
>       and become misaligned.

Not used anymore - it's practically zero (and unused) for most cases
now anyway.

>     * The absolute entry point fails to indicate that the entry
>       point is Thumb code, which will cause incorrect execution
>       if the bootloader uses this to enter the kernel.

And as such...

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

[Nicolas Pitre]

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

> On Tue, Nov 16, 2010 at 01:14:37PM +0000, Dave Martin wrote:
> > This gets messed up with with CONFIG_THUMB2_KERNEL:
> > 
> >     * The NOPs making up the vector table become halfword-
> >       sized.
> 
> Doesn't matter.
> 
> >     * The magic number and absolute entry point occur too early
> >       and become misaligned.
> 
> Not used anymore - it's practically zero (and unused) for most cases
> now anyway.

I think it is worth preserving this layout regardless.  First of all 
this is really cheap to do, and if whatever bootloader out there is 
relying on it, at least the magic number, then better not break it 
freely.

> >     * The absolute entry point fails to indicate that the entry
> >       point is Thumb code, which will cause incorrect execution
> >       if the bootloader uses this to enter the kernel.
> 
> And as such...

On the other hand... we could simply decide _not_ to fix it on the 
basis that this will create a different header for a pure Thumb2 
image.

Maybe it is a better idea to preserve the current header as is with the 
ARM mode nops and perform the switch to Thumb mode using the branch 
instruction after those 8 nops.  This way the kernel image format will 
remain compatible with existing bootloaders without a need for 
bootloaders to know if the image is ARM or Thumb mode.


Nicolas

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

[Russell King - ARM Linux]

On Tue, Nov 16, 2010 at 03:28:44PM -0500, Nicolas Pitre wrote:
> On Tue, 16 Nov 2010, Russell King - ARM Linux wrote:
> > Not used anymore - it's practically zero (and unused) for most cases
> > now anyway.
> 
> I think it is worth preserving this layout regardless.  First of all 
> this is really cheap to do, and if whatever bootloader out there is 
> relying on it, at least the magic number, then better not break it 
> freely.

It's already broken by the relocatable format - which has zero as the
start address.  That's been in for a few years now, and no one even
noticed that this header ended up with zero as the entry address.
Therefore, I suggest that no one at all is using it.

> On the other hand... we could simply decide _not_ to fix it on the 
> basis that this will create a different header for a pure Thumb2 
> image.
> 
> Maybe it is a better idea to preserve the current header as is with the 
> ARM mode nops and perform the switch to Thumb mode using the branch 
> instruction after those 8 nops.  This way the kernel image format will 
> remain compatible with existing bootloaders without a need for 
> bootloaders to know if the image is ARM or Thumb mode.

That is definitely worth doing, and sounds like a better idea.

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

[Uwe Kleine-König]

Hi Russell,

On Tue, Nov 16, 2010 at 08:35:20PM +0000, Russell King - ARM Linux wrote:
> On Tue, Nov 16, 2010 at 03:28:44PM -0500, Nicolas Pitre wrote:
> > On Tue, 16 Nov 2010, Russell King - ARM Linux wrote:
> > > Not used anymore - it's practically zero (and unused) for most cases
> > > now anyway.
> > 
> > I think it is worth preserving this layout regardless.  First of all 
> > this is really cheap to do, and if whatever bootloader out there is 
> > relying on it, at least the magic number, then better not break it 
> > freely.
> 
> It's already broken by the relocatable format - which has zero as the
> start address.  That's been in for a few years now, and no one even
> noticed that this header ended up with zero as the entry address.
> Therefore, I suggest that no one at all is using it.
I remember me creating a patch for U-Boot that ignored the uImage
header's load and entry address if it was an ARM-Linux image and the
linux header had the right magic and the entry address was zero.  I
don't know anything about it's current state, but it might still ship
with Digi's BSP.

Best regards
Uwe

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

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

[Dave Martin]

On Tue, Nov 16, 2010 at 6:26 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Nov 16, 2010 at 01:14:37PM +0000, Dave Martin wrote:
>> The code which makes up the zImage header clearly intends to
>> leave a vector-table-sized gap of 8 words (NOPs, in fact),
>> followed by a branch to the real entry point, a magic number,
>> and a word containing the absolute entry point address.
>
> That's an incorrect assumption.  The set of 8 words have nothing to do
> with the CPUs vector table at all - it has more to do with compatibility
> with a.out built kernels, where the a.out header was 32 bytes.

[...]

Fair enough -- that was just guesswork on my part.

[...]

On Tue, Nov 16, 2010 at 8:35 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Nov 16, 2010 at 03:28:44PM -0500, Nicolas Pitre wrote:
>> On Tue, 16 Nov 2010, Russell King - ARM Linux wrote:
>> > Not used anymore - it's practically zero (and unused) for most cases
>> > now anyway.
>>
>> I think it is worth preserving this layout regardless. ?First of all
>> this is really cheap to do, and if whatever bootloader out there is
>> relying on it, at least the magic number, then better not break it
>> freely.
>
> It's already broken by the relocatable format - which has zero as the
> start address. ?That's been in for a few years now, and no one even
> noticed that this header ended up with zero as the entry address.
> Therefore, I suggest that no one at all is using it.
>
>> On the other hand... we could simply decide _not_ to fix it on the
>> basis that this will create a different header for a pure Thumb2
>> image.
>>
>> Maybe it is a better idea to preserve the current header as is with the
>> ARM mode nops and perform the switch to Thumb mode using the branch
>> instruction after those 8 nops. ?This way the kernel image format will
>> remain compatible with existing bootloaders without a need for
>> bootloaders to know if the image is ARM or Thumb mode.
>
> That is definitely worth doing, and sounds like a better idea.
>

OK, that sounds reasonable -- I can try and propose a patch for this.
If the contents of the first 32 bytes doesn't matter, we can replace
the first two words with something like

adr r12, BSYM(real_start)
bx r12

.rept 6

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

[Dave Martin]

On Wed, Nov 17, 2010 at 9:16 AM, Dave Martin <dave.martin@linaro.org> wrote:

[...]

Sorry -- the web is broken (pressing space alone should never _send_ a
mail :( ... )

Better than trying to give an example, I'll follow up soon with an
updated patch.

Cheers
---Dave

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

[Catalin Marinas]

On Wed, 2010-11-17 at 09:16 +0000, Dave Martin wrote:
> On Tue, Nov 16, 2010 at 6:26 PM, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Tue, Nov 16, 2010 at 01:14:37PM +0000, Dave Martin wrote:
> >> Maybe it is a better idea to preserve the current header as is with the
> >> ARM mode nops and perform the switch to Thumb mode using the branch
> >> instruction after those 8 nops.  This way the kernel image format will
> >> remain compatible with existing bootloaders without a need for
> >> bootloaders to know if the image is ARM or Thumb mode.
> >
> > That is definitely worth doing, and sounds like a better idea.
> 
> OK, that sounds reasonable -- I can try and propose a patch for this.
> If the contents of the first 32 bytes doesn't matter, we can replace
> the first two words with something like
> 
> adr r12, BSYM(real_start)
> bx r12

Would these be compiled to ARM or Thumb-2?

-- 
Catalin

[PATCH 7/7 v1.1] 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 cleanly on v2.6.37-rc2.

Signed-off-by: Dave Martin <dave.martin@linaro.org>
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
---
 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..ab95391 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
+ THUMB(		.arm			)
 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 7/7 v1.1] ARM: Thumb-2: Restore sensible zImage header layout for CONFIG_THUMB2_KERNEL

[Nicolas Pitre]

On Wed, 17 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 cleanly on v2.6.37-rc2.
> 
> 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>

Only a nanonit below that is not that important.

> index 1f65080..ab95391 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
> + THUMB(		.arm			)

Maybe the THUMB() can be omitted here and .arm be unconditional?

>  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 7/7 v1.1] ARM: Thumb-2: Restore sensible zImage header layout for CONFIG_THUMB2_KERNEL

[Dave Martin]

On Wed, Nov 17, 2010 at 4:17 PM, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

[...]

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

Thanks

> Only a nanonit below that is not that important.

[...]

>> + THUMB( ? ? ? ? ? ? ?.arm ? ? ? ? ? ? ? ? ? ?)
>
> Maybe the THUMB() can be omitted here and .arm be unconditional?

Agreed, that would be less crufty -- I'll make that change locally for
now and repost when I have other changes to push.

Cheers
---Dave