Patchwork [U-Boot] armv7: do not relocate _start twice

login
register
mail settings
Submitter Vincent Stehlé
Date March 15, 2013, 4:54 p.m.
Message ID <1363366440-13227-1-git-send-email-v-stehle@ti.com>
Download mbox | patch
Permalink /patch/228097/
State Accepted
Delegated to: Albert ARIBAUD
Headers show

Comments

Vincent Stehlé - March 15, 2013, 4:54 p.m.
The _start symbol is already relocated, so do not add the relocation the second
time in c_runtime_cpu_setup.

This fixes e.g. the abort exception handling path, which ended in double fault
due to bad address in VBAR.

Signed-off-by: Vincent Stehlé <v-stehle@ti.com>
Reported-by: Lubomir Popov <lpopov@mm-sol.com>
---


Hello,

Here is a fix for a bug reported by Lubomir. He noticed that exceptions were
not handled correctly anymore. This can be seen with e.g. the 'dhcp' command on
some OMAP platforms.

Looking at the code, I would says the fix applies to all armv7 platforms except
Tegra but I did only test on OMAP5. On this platform at least the abort is now
handled:

  OMAP5430 EVM # dhcp
  data abort

      MAYBE you should read doc/README.arm-unaligned-accesses

  pc : [<fef9bd78>]          lr : [<fefa1790>]
  sp : feef9dc4  ip : fefed0f8     fp : 00000000
  r10: 00000001  r9 : 00000001     r8 : feef9f48
  r7 : feef9fe0  r6 : 00000000     r5 : 00000000  r4 : 00000014
  r3 : 00000000  r2 : 00000002     r1 : 00000014  r0 : fefed0f4
  Flags: Nzcv  IRQs off  FIQs off  Mode SVC_32
  Resetting CPU ...

  resetting ...

It would be appreciated if folks could verify on other ARMv7 platforms, when
running from flash for example (where relocation may differ?)


 arch/arm/cpu/armv7/start.S |    1 -
 1 file changed, 1 deletion(-)
Albert ARIBAUD - March 28, 2013, 9:16 a.m.
Hi Vincent,

On Fri, 15 Mar 2013 17:54:00 +0100, Vincent Stehlé <v-stehle@ti.com>
wrote:

> The _start symbol is already relocated, so do not add the relocation the second
> time in c_runtime_cpu_setup.
> 
> This fixes e.g. the abort exception handling path, which ended in double fault
> due to bad address in VBAR.
> 
> Signed-off-by: Vincent Stehlé <v-stehle@ti.com>
> Reported-by: Lubomir Popov <lpopov@mm-sol.com>
> ---
> 
> 
> Hello,
> 
> Here is a fix for a bug reported by Lubomir. He noticed that exceptions were
> not handled correctly anymore. This can be seen with e.g. the 'dhcp' command on
> some OMAP platforms.
> 
> Looking at the code, I would says the fix applies to all armv7 platforms except
> Tegra but I did only test on OMAP5. On this platform at least the abort is now
> handled:
> 
>   OMAP5430 EVM # dhcp
>   data abort
> 
>       MAYBE you should read doc/README.arm-unaligned-accesses
> 
>   pc : [<fef9bd78>]          lr : [<fefa1790>]
>   sp : feef9dc4  ip : fefed0f8     fp : 00000000
>   r10: 00000001  r9 : 00000001     r8 : feef9f48
>   r7 : feef9fe0  r6 : 00000000     r5 : 00000000  r4 : 00000014
>   r3 : 00000000  r2 : 00000002     r1 : 00000014  r0 : fefed0f4
>   Flags: Nzcv  IRQs off  FIQs off  Mode SVC_32
>   Resetting CPU ...
> 
>   resetting ...
> 
> It would be appreciated if folks could verify on other ARMv7 platforms, when
> running from flash for example (where relocation may differ?)
> 
> 
>  arch/arm/cpu/armv7/start.S |    1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 6b59529d..d06b35f 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -254,7 +254,6 @@ ENTRY(c_runtime_cpu_setup)
>  #if !defined(CONFIG_TEGRA)
>  	/* Set vector address in CP15 VBAR register */
>  	ldr     r0, =_start
> -	add     r0, r0, r9
>  	mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
>  #endif /* !Tegra */

That's a very good catch!

For the record, the issue crept in when I applied the patch set to
remove all R_ARM_ABS32 relocation record types; after that, the only
manual relocations that should have remained were the three ones in each
relocate_code() routine, yet in armv7 there was this fourth one which
had totally escaped my attention.

I have verified in the ELF dump of omap5_evm that the "=_start"
reference is indeed relocated as part of relocate_code() execution.

As this is a bugfix, applied to u-boot-arm/master,

thanks!

Amicalement,

Patch

diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
index 6b59529d..d06b35f 100644
--- a/arch/arm/cpu/armv7/start.S
+++ b/arch/arm/cpu/armv7/start.S
@@ -254,7 +254,6 @@  ENTRY(c_runtime_cpu_setup)
 #if !defined(CONFIG_TEGRA)
 	/* Set vector address in CP15 VBAR register */
 	ldr     r0, =_start
-	add     r0, r0, r9
 	mcr     p15, 0, r0, c12, c0, 0  @Set VBAR
 #endif /* !Tegra */