-----Message d'origine----- De : u-boot-bounces@lists.denx.de [mailto:u-boot- bounces@lists.denx.de] De la part de Albert ARIBAUD Envoyé : mardi 20 septembre 2011 21:14 À : u-boot@lists.denx.de Objet : Re: [U-Boot] [RFC PATCH] arm: provide a CONFIG flag for disabling relocation
Le 20/09/2011 20:09, Wolfgang Denk a écrit :
Dear "GROYER, Anthony",
In message<BC0A2F434D4F39448D24A68EA6EFFB9F0194D534@EU-FR-
EXBE07.eu.corp.airliquide.com> you wrote:
The use of the initial patches for the
CONFIG_SYS_SKIP_ARM_RELOCATION featu
res has revealed two issues.
Could you please restict your line length to some 70 characters or
so?
Thanks.
First issue: the calculation of the relocation offset was done
only if the
relocation is actually done. So we could reach a point where r9
has a wrong
value, since it has never been used before (in my case, this
bug happens w
This is a configuration error then, isn't it? The relocation
offset
should be either the intended value, or eventually zero, if no relocation is intended.
Actually, even though "revision 1083" and "revision 1113" are not git references (and thus I can't be sure Anthony is referring to up-to- date mainline code), there is a point to what Anthony says: in the case where relocation is unneeded (r0 equals r6) then r9 is not set, but is still used when branching to board_init_r().
for this bug to have any effect, relocation would have to be unneeded, which is a rare case, *and* r9 has to be nonzero, which may or may not happen depending on the code executed until relocate_code() is called, and thus makes the whole condition rarer yet; probably the rarity of these two conjunct conditions explains why it was not noticed until now.
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. 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 ?
Anthony
BTW: your patch has a number ofd coduing style errors, and the Signed-off-by: line is missing.
Plus it did not have the commit message separator either. I suspect it was not produced using git format-patch / git send-email.
Anthony, please submit a proper [PATCH], without [RFC], and with the setting of r9 done as shown above, and applied to all relevant start.S files in arch/arm/cpu/*/ -- in next merge window we should try and factorize those start.S files, BTW.
Best regards,
Wolfgang Denk
Amicalement,
Albert. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot