[U-Boot] [PATCH 1/2] arm: Make reset position-independent

[U-Boot] [PATCH 1/2] arm: Make reset position-independent

[Benoît Thébaudeau]

Some boards, like mx31pdk and tx25, require the beginning of the SPL
code to be position-independent. For these two boards, this is because
they use the i.MX external NAND boot, which starts by executing the
first NAND Flash page from the NFC page buffer. The SPL then needs to
copy itself to its actual link address in order to free the NFC page
buffer and use it to load the non-SPL image from Flash before running
it. This means that the SPL runtime address differs from its link
address between the reset and the initial copy performed by
board_init_f(), so this part of the SPL binary must be
position-independent.

This requirement was broken by commit 41623c9 'arm: move exception
handling out of start.S files', which used an absolute address to branch
to the reset routine. This new commit restores the original behavior,
which just performed a relative branch. This fixes the boot of mx31pdk
and tx25.

Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
Reported-by: Helmut Raiger <helmut.raiger@hale.at>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Magnus Lilja <lilja.magnus@gmail.com>
Cc: John Rigby <jcrigby@gmail.com>
---
 arch/arm/lib/vectors.S |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
index 493f337..843b18f 100644
--- a/arch/arm/lib/vectors.S
+++ b/arch/arm/lib/vectors.S
@@ -50,7 +50,7 @@
 #endif
 
 _start:
-	ldr	pc, _reset
+	b	reset
 	ldr	pc, _undefined_instruction
 	ldr	pc, _software_interrupt
 	ldr	pc, _prefetch_abort
@@ -77,7 +77,6 @@ _start:
 	.globl	_irq
 	.globl	_fiq
 
-_reset:			.word reset
 _undefined_instruction:	.word undefined_instruction
 _software_interrupt:	.word software_interrupt
 _prefetch_abort:	.word prefetch_abort
-- 
1.7.10.4

[U-Boot] [PATCH 2/2] arm: Fix _start for CONFIG_SYS_DV_NOR_BOOT_CFG

[Benoît Thébaudeau]

The boards using CONFIG_SYS_DV_NOR_BOOT_CFG (i.e. calimain,
da850evm_direct_nor and enbw_cmc) had the _start symbol defined after
the CONFIG_SYS_DV_NOR_BOOT_CFG word rather than before it in
arch/arm/lib/vectors.S. Because of that, if by lack of luck
'gd->mon_len = (ulong)&__bss_end - (ulong)_start' (see setup_mon_len())
was a multiple of 4 kiB (see reserve_uboot()), then the last BSS word
overlapped the first word of the following reserved RAM area (or went
beyond the top of RAM without such an area) after relocation because
__image_copy_start did not match _start (see relocate_code()).

This was broken by commit 41623c9 'arm: move exception handling out of
start.S files', which defined _start twice (before and after the
CONFIG_SYS_DV_NOR_BOOT_CFG word), then by commit 0a26e1d 'arm: fix a
double-definition error of _start symbol', which kept the definition of
the _start symbol after the CONFIG_SYS_DV_NOR_BOOT_CFG word. This new
commit fixes this issue by restoring the original behavior, i.e. by
defining the _start symbol before the CONFIG_SYS_DV_NOR_BOOT_CFG word.

Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
Cc: Manfred Rudigier <manfred.rudigier@omicron.at>
Cc: Christian Riesch <christian.riesch@omicron.at>
Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
Cc: Heiko Schocher <hs@denx.de>
---
 arch/arm/lib/vectors.S |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
index 843b18f..0cb87ce 100644
--- a/arch/arm/lib/vectors.S
+++ b/arch/arm/lib/vectors.S
@@ -45,11 +45,12 @@
  *************************************************************************
  */
 
+_start:
+
 #ifdef CONFIG_SYS_DV_NOR_BOOT_CFG
 	.word	CONFIG_SYS_DV_NOR_BOOT_CFG
 #endif
 
-_start:
 	b	reset
 	ldr	pc, _undefined_instruction
 	ldr	pc, _software_interrupt
-- 
1.7.10.4

[U-Boot] [PATCH 2/2] arm: Fix _start for CONFIG_SYS_DV_NOR_BOOT_CFG

[Albert ARIBAUD]

Hi Beno?t,

On Wed,  3 Sep 2014 23:32:34 +0200, Beno?t Th?baudeau
<benoit.thebaudeau.dev@gmail.com> wrote:

> The boards using CONFIG_SYS_DV_NOR_BOOT_CFG (i.e. calimain,
> da850evm_direct_nor and enbw_cmc) had the _start symbol defined after
> the CONFIG_SYS_DV_NOR_BOOT_CFG word rather than before it in
> arch/arm/lib/vectors.S. Because of that, if by lack of luck
> 'gd->mon_len = (ulong)&__bss_end - (ulong)_start' (see setup_mon_len())
> was a multiple of 4 kiB (see reserve_uboot()), then the last BSS word
> overlapped the first word of the following reserved RAM area (or went
> beyond the top of RAM without such an area) after relocation because
> __image_copy_start did not match _start (see relocate_code()).
> 
> This was broken by commit 41623c9 'arm: move exception handling out of
> start.S files', which defined _start twice (before and after the
> CONFIG_SYS_DV_NOR_BOOT_CFG word), then by commit 0a26e1d 'arm: fix a
> double-definition error of _start symbol', which kept the definition of
> the _start symbol after the CONFIG_SYS_DV_NOR_BOOT_CFG word. This new
> commit fixes this issue by restoring the original behavior, i.e. by
> defining the _start symbol before the CONFIG_SYS_DV_NOR_BOOT_CFG word.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Manfred Rudigier <manfred.rudigier@omicron.at>
> Cc: Christian Riesch <christian.riesch@omicron.at>
> Cc: Sudhakar Rajashekhara <sudhakar.raj@ti.com>
> Cc: Heiko Schocher <hs@denx.de>
> ---
>  arch/arm/lib/vectors.S |    3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
> index 843b18f..0cb87ce 100644
> --- a/arch/arm/lib/vectors.S
> +++ b/arch/arm/lib/vectors.S
> @@ -45,11 +45,12 @@
>   *************************************************************************
>   */
>  
> +_start:
> +
>  #ifdef CONFIG_SYS_DV_NOR_BOOT_CFG
>  	.word	CONFIG_SYS_DV_NOR_BOOT_CFG
>  #endif
>  
> -_start:
>  	b	reset
>  	ldr	pc, _undefined_instruction
>  	ldr	pc, _software_interrupt

Applied (as a bug fix) to u-boot-arm/master, thanks!

Amicalement,
-- 
Albert.

[U-Boot] [PATCH 1/2] arm: Make reset position-independent

[Benoît Thébaudeau]

On Wed, Sep 3, 2014 at 11:32 PM, Beno?t Th?baudeau
<benoit.thebaudeau.dev@gmail.com> wrote:
> Some boards, like mx31pdk and tx25, require the beginning of the SPL
> code to be position-independent. For these two boards, this is because
> they use the i.MX external NAND boot, which starts by executing the
> first NAND Flash page from the NFC page buffer. The SPL then needs to
> copy itself to its actual link address in order to free the NFC page
> buffer and use it to load the non-SPL image from Flash before running
> it. This means that the SPL runtime address differs from its link
> address between the reset and the initial copy performed by
> board_init_f(), so this part of the SPL binary must be
> position-independent.
>
> This requirement was broken by commit 41623c9 'arm: move exception
> handling out of start.S files', which used an absolute address to branch
> to the reset routine. This new commit restores the original behavior,
> which just performed a relative branch. This fixes the boot of mx31pdk
> and tx25.
>
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
> Reported-by: Helmut Raiger <helmut.raiger@hale.at>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Magnus Lilja <lilja.magnus@gmail.com>
> Cc: John Rigby <jcrigby@gmail.com>
> ---
>  arch/arm/lib/vectors.S |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
> index 493f337..843b18f 100644
> --- a/arch/arm/lib/vectors.S
> +++ b/arch/arm/lib/vectors.S
> @@ -50,7 +50,7 @@
>  #endif
>
>  _start:
> -       ldr     pc, _reset
> +       b       reset
>         ldr     pc, _undefined_instruction
>         ldr     pc, _software_interrupt
>         ldr     pc, _prefetch_abort
> @@ -77,7 +77,6 @@ _start:
>         .globl  _irq
>         .globl  _fiq
>
> -_reset:                        .word reset
>  _undefined_instruction:        .word undefined_instruction
>  _software_interrupt:   .word software_interrupt
>  _prefetch_abort:       .word prefetch_abort
> --
> 1.7.10.4
>

Magnus, can I have your 'Tested-by' for mx31pdk since you said you
have tested this?

Tom, Albert, can you build-test all ARM boards with this patch (that
would take eons on my ultra slow machine)? It would also be nice if
someone could runtime-test an ARM board other than mx31pdk and tx25
with this patch.

Thanks in advance.

Best regards,
Beno?t

[U-Boot] [PATCH 1/2] arm: Make reset position-independent

[Benoît Thébaudeau]

On Wed, Sep 3, 2014 at 11:32 PM, Beno?t Th?baudeau
<benoit.thebaudeau.dev@gmail.com> wrote:
> Some boards, like mx31pdk and tx25, require the beginning of the SPL
> code to be position-independent. For these two boards, this is because
> they use the i.MX external NAND boot, which starts by executing the
> first NAND Flash page from the NFC page buffer. The SPL then needs to
> copy itself to its actual link address in order to free the NFC page
> buffer and use it to load the non-SPL image from Flash before running
> it. This means that the SPL runtime address differs from its link
> address between the reset and the initial copy performed by
> board_init_f(), so this part of the SPL binary must be
> position-independent.
>
> This requirement was broken by commit 41623c9 'arm: move exception
> handling out of start.S files', which used an absolute address to branch
> to the reset routine. This new commit restores the original behavior,
> which just performed a relative branch. This fixes the boot of mx31pdk
> and tx25.
>
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
> Reported-by: Helmut Raiger <helmut.raiger@hale.at>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Magnus Lilja <lilja.magnus@gmail.com>
> Cc: John Rigby <jcrigby@gmail.com>
> ---
>  arch/arm/lib/vectors.S |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
> index 493f337..843b18f 100644
> --- a/arch/arm/lib/vectors.S
> +++ b/arch/arm/lib/vectors.S
> @@ -50,7 +50,7 @@
>  #endif
>
>  _start:
> -       ldr     pc, _reset
> +       b       reset
>         ldr     pc, _undefined_instruction
>         ldr     pc, _software_interrupt
>         ldr     pc, _prefetch_abort
> @@ -77,7 +77,6 @@ _start:
>         .globl  _irq
>         .globl  _fiq
>
> -_reset:                        .word reset
>  _undefined_instruction:        .word undefined_instruction
>  _software_interrupt:   .word software_interrupt
>  _prefetch_abort:       .word prefetch_abort
> --
> 1.7.10.4
>

Adding Helmut to Cc.

Beno?t

[U-Boot] [PATCH 1/2] arm: Make reset position-independent

[Magnus Lilja]

Hi

On 3 September 2014 23:32, Beno?t Th?baudeau
<benoit.thebaudeau.dev@gmail.com> wrote:
> Some boards, like mx31pdk and tx25, require the beginning of the SPL
> code to be position-independent. For these two boards, this is because
> they use the i.MX external NAND boot, which starts by executing the
> first NAND Flash page from the NFC page buffer. The SPL then needs to
> copy itself to its actual link address in order to free the NFC page
> buffer and use it to load the non-SPL image from Flash before running
> it. This means that the SPL runtime address differs from its link
> address between the reset and the initial copy performed by
> board_init_f(), so this part of the SPL binary must be
> position-independent.
>
> This requirement was broken by commit 41623c9 'arm: move exception
> handling out of start.S files', which used an absolute address to branch
> to the reset routine. This new commit restores the original behavior,
> which just performed a relative branch. This fixes the boot of mx31pdk
> and tx25.
>
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
> Reported-by: Helmut Raiger <helmut.raiger@hale.at>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Magnus Lilja <lilja.magnus@gmail.com>
> Cc: John Rigby <jcrigby@gmail.com>
> ---

Tested-by: Magnus Lilja <lilja.magnus@gmail.com>

[U-Boot] [PATCH 1/2] arm: Make reset position-independent

[Fabio Estevam]

Hi Albert,

On Fri, Sep 5, 2014 at 3:41 PM, Magnus Lilja <lilja.magnus@gmail.com> wrote:
> Hi
>
> On 3 September 2014 23:32, Beno?t Th?baudeau
> <benoit.thebaudeau.dev@gmail.com> wrote:
>> Some boards, like mx31pdk and tx25, require the beginning of the SPL
>> code to be position-independent. For these two boards, this is because
>> they use the i.MX external NAND boot, which starts by executing the
>> first NAND Flash page from the NFC page buffer. The SPL then needs to
>> copy itself to its actual link address in order to free the NFC page
>> buffer and use it to load the non-SPL image from Flash before running
>> it. This means that the SPL runtime address differs from its link
>> address between the reset and the initial copy performed by
>> board_init_f(), so this part of the SPL binary must be
>> position-independent.
>>
>> This requirement was broken by commit 41623c9 'arm: move exception
>> handling out of start.S files', which used an absolute address to branch
>> to the reset routine. This new commit restores the original behavior,
>> which just performed a relative branch. This fixes the boot of mx31pdk
>> and tx25.
>>
>> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
>> Reported-by: Helmut Raiger <helmut.raiger@hale.at>
>> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
>> Cc: Magnus Lilja <lilja.magnus@gmail.com>
>> Cc: John Rigby <jcrigby@gmail.com>
>> ---
>
> Tested-by: Magnus Lilja <lilja.magnus@gmail.com>

Should this one go through your tree?

[U-Boot] [PATCH 1/2] arm: Make reset position-independent

[Albert ARIBAUD]

Hi Beno?t,

On Wed,  3 Sep 2014 23:32:33 +0200, Beno?t Th?baudeau
<benoit.thebaudeau.dev@gmail.com> wrote:

> Some boards, like mx31pdk and tx25, require the beginning of the SPL
> code to be position-independent. For these two boards, this is because
> they use the i.MX external NAND boot, which starts by executing the
> first NAND Flash page from the NFC page buffer. The SPL then needs to
> copy itself to its actual link address in order to free the NFC page
> buffer and use it to load the non-SPL image from Flash before running
> it. This means that the SPL runtime address differs from its link
> address between the reset and the initial copy performed by
> board_init_f(), so this part of the SPL binary must be
> position-independent.
> 
> This requirement was broken by commit 41623c9 'arm: move exception
> handling out of start.S files', which used an absolute address to branch
> to the reset routine. This new commit restores the original behavior,
> which just performed a relative branch. This fixes the boot of mx31pdk
> and tx25.
> 
> Signed-off-by: Beno?t Th?baudeau <benoit.thebaudeau.dev@gmail.com>
> Reported-by: Helmut Raiger <helmut.raiger@hale.at>
> Cc: Albert Aribaud <albert.u.boot@aribaud.net>
> Cc: Magnus Lilja <lilja.magnus@gmail.com>
> Cc: John Rigby <jcrigby@gmail.com>
> ---
>  arch/arm/lib/vectors.S |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/arch/arm/lib/vectors.S b/arch/arm/lib/vectors.S
> index 493f337..843b18f 100644
> --- a/arch/arm/lib/vectors.S
> +++ b/arch/arm/lib/vectors.S
> @@ -50,7 +50,7 @@
>  #endif
>  
>  _start:
> -	ldr	pc, _reset
> +	b	reset
>  	ldr	pc, _undefined_instruction
>  	ldr	pc, _software_interrupt
>  	ldr	pc, _prefetch_abort
> @@ -77,7 +77,6 @@ _start:
>  	.globl	_irq
>  	.globl	_fiq
>  
> -_reset:			.word reset
>  _undefined_instruction:	.word undefined_instruction
>  _software_interrupt:	.word software_interrupt
>  _prefetch_abort:	.word prefetch_abort

Applied (as a bug fix) to u-boot-arm/master, thanks!

Amicalement,
-- 
Albert.