All of lore.kernel.org
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] armv7: do not relocate _start twice
Date: Thu, 28 Mar 2013 10:16:02 +0100	[thread overview]
Message-ID: <20130328101602.251c732a@lilith> (raw)
In-Reply-To: <1363366440-13227-1-git-send-email-v-stehle@ti.com>

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,
-- 
Albert.

      reply	other threads:[~2013-03-28  9:16 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-15 16:54 [U-Boot] [PATCH] armv7: do not relocate _start twice Vincent Stehlé
2013-03-28  9:16 ` Albert ARIBAUD [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130328101602.251c732a@lilith \
    --to=albert.u.boot@aribaud.net \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.