> -----Message d'origine-----
> De : u-boot-boun...@lists.denx.de [mailto:u-boot-
> boun...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
