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

[GROYER, Anthony]

> -----Message d'origine-----
> De : u-boot-bounces at lists.denx.de [mailto:u-boot-
> bounces at lists.denx.de] De la part de Albert ARIBAUD
> Envoyé : mardi 20 septembre 2011 21:14
> À : u-boot at 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 at 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot