[U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation

[Albert ARIBAUD]

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.