public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Albert ARIBAUD <albert.u.boot@aribaud.net>
To: u-boot@lists.denx.de
Subject: [U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation
Date: Wed, 21 Sep 2011 12:51:33 +0200	[thread overview]
Message-ID: <4E79C1B5.7040101@aribaud.net> (raw)
In-Reply-To: <BC0A2F434D4F39448D24A68EA6EFFB9F0194DA79@EU-FR-EXBE07.eu.corp.airliquide.com>

Le 21/09/2011 11:29, GROYER, Anthony a ?crit :

>> However, since start.S has a code path to handle the non-relocating
>> case, this path ought to be bug-free. But then, I want it to be
>> consistent: if the relocation offset is computed in r9, then testing
>> whether relocation is needed would be done on r9 once computed, not
>> before, by replacing
>>
>> 	adr	r0, _start
>> 	cmp	r0, r6
>> 	beq	clear_bss		/* skip relocation */
>>
>> With
>>
>> 	adr	r0, _start
>> 	sub	r9, r6, r0
>> 	cmp	r0, #0
>> 	beq	clear_bss		/* skip relocation */
>
>
> I guess the code is this:
>   	adr	r0, _start
>   	sub	r9, r6, r0
>   	cmp	r9, #0
>   	beq	clear_bss		/* skip relocation */
>
> What is the difference between _start and _TEXT_BASE ? I do not see
> any differences and the former relocation offset calculation was
> using _TEXT_BASE.

The diff?rence is that when using "ldr r0,_TEXT_BASE", you depend on 
U-Boot having been actually loaded at the address specified by 
CONFIG_SYS_TEXT_BASE -- that *should* be the case, but if it is not, the 
code won't work.

OTOH, using "adr r0,_start" is actually translated into "sub r0, pc, 
#nnn", thus always setting r0 to the actual base address of U-Boot -- it 
will work even when U-Boot is not run at CONFIG_SYS_TEXT_BASE.

> May I remove the following code in all arch/arm/cpu/*/start.S ?
>      ldr r0, _TEXT_BASE      /* r0<- Text base */
>      sub r9, r6, r0      /* r9<- relocation offset */
> and expect than the "adr	r0, _start" is sufficient ?

Upon *third* look, it is a bit more complicated.

Granted, you want r9 set even if you don't relocate.
But the current relocation code *trashes* r9 in the copy loop, before 
setting it right before the relocation loop.
And *then* it is used for jumping to board_init_r.

So the correct fix would be to

1) set r9 early, before checking if relocation is needed so that a 
direct jump to clear_bss has the correct r9 whichever way.

2) remove the late setting of r9 between the copy and relocation loop, 
as it was already set up in 1)

3) replace use of r9-r10 with e.g. r10-r11 in the copy loop, to preserve 
r9 during relocation.

4) Replace all "ldr rNN, _TEXT_BASE" with "adr rNN, _start".

Best is you submit a new RFC patch and when it is OK and you have 
validated through JTAG that it works, then re-submit it as a non-RFC 
patch to all start.S files in arch/arm/cpu/*.

Amicalement,
-- 
Albert.

  parent reply	other threads:[~2011-09-21 10:51 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-20 14:22 [U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation GROYER, Anthony
2011-09-20 18:09 ` Wolfgang Denk
2011-09-20 19:13   ` Albert ARIBAUD
2011-09-21  9:29     ` GROYER, Anthony
2011-09-21 10:45       ` Wolfgang Denk
2011-09-21 11:44         ` Albert ARIBAUD
2011-09-21 10:51       ` Albert ARIBAUD [this message]
2011-09-21 11:20         ` Andreas Bießmann
2011-09-21 12:03           ` Albert ARIBAUD
2011-09-21 12:31             ` Andreas Bießmann
2011-09-21 14:23               ` Albert ARIBAUD
2011-09-22  7:10                 ` Andreas Bießmann
2011-09-29 16:14               ` Andreas Bießmann
2011-09-30  7:21                 ` Simon Schwarz
2011-10-01  6:48                   ` Albert ARIBAUD
2011-09-20 21:34 ` Simon Glass
2011-09-21 14:21   ` Aneesh V
2011-09-23 16:04     ` Simon Glass
2011-10-01  7:01       ` Albert ARIBAUD
2011-10-03  3:34         ` Simon Glass
  -- strict thread matches above, loose matches on Subject: below --
2011-03-25 13:12 Aneesh V
2011-03-25 13:27 ` Aneesh V
2011-03-25 14:12 ` Wolfgang Denk
2011-03-25 16:12   ` Aneesh V
2011-03-25 18:35     ` Albert ARIBAUD
2011-04-20 18:34       ` Simon Glass
2011-04-21  6:56         ` Aneesh V
2011-04-21 15:18           ` Simon Glass

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=4E79C1B5.7040101@aribaud.net \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox