[U-Boot] [PATCH v1] arm: remove unneeded symbol offsets and _TEXT_BASE

[Benoît Thébaudeau]

Hi Albert,

On Sunday, October 13, 2013 7:16:33 PM, Albert ARIBAUD wrote:
> Hi Benoît,
> 
> On Sun, 13 Oct 2013 17:00:25 +0200 (CEST), Benoît Thébaudeau
> <benoit.thebaudeau at advansee.com> wrote:
> 
> > Hi Albert,
> > 
> > On Sunday, October 13, 2013 9:10:28 AM, Albert ARIBAUD wrote:
> > [...]
> > 
> > > diff --git a/arch/arm/lib/board.c b/arch/arm/lib/board.c
> > > index 34f50b0..f8ac573 100644
> > > --- a/arch/arm/lib/board.c
> > > +++ b/arch/arm/lib/board.c
> > > @@ -105,8 +105,8 @@ static int display_banner(void)
> > >  {
> > >  	printf("\n\n%s\n\n", version_string);
> > >  	debug("U-Boot code: %08lX -> %08lX  BSS: -> %08lX\n",
> > > -	       _TEXT_BASE,
> > > -	       _bss_start_ofs + _TEXT_BASE, _bss_end_ofs + _TEXT_BASE);
> > > +	       (ulong)&_start,
> > > +	       (ulong)&__bss_start, (ulong)&__bss_end);
> > >  #ifdef CONFIG_MODEM_SUPPORT
> > >  	debug("Modem Support enabled\n");
> > >  #endif
> > 
> > This hunk and all the other hunks using _TEXT_BASE in the same way will
> > introduce different resulting values than the original code for targets
> > having
> > different build-time and run-time addresses.
> > 
> > This is not too much of an issue for the debug() call here, but this may be
> > more
> > damaging for things like gd->reloc_off below.
> 
> Indeed build-time and run-time values might be different. Normally,
> U-Boot starts by being loaded at its build-time address and run from
> there (or directly run there if from NOR). At that point, its build- and
> run-time addresses coincide. then we reach relocation. Then U-Boot
> runs from top of RAM, with a run-time address different from its
> build-time one, bu with all base-address-dependent locations fixed by
> relocation, so again, run-time and (relocated) build-time values are
> equal.

That's not true for all values following relocation. E.g., in start.S,
_TEXT_BASE is initialized to either CONFIG_SPL_TEXT_BASE or
CONFIG_SYS_TEXT_BASE. Those are #defined, so _TEXT_BASE has the same value
before and after relocation, and this value differs from &_start after
relocation.

> IOW, this debug() line would use true build-time values if invoked
> before relocation, and actual run-time values after relocation, because
> the &symbol constructs would have relocation entries and thus be fixed
> during relocation.
> 
> This does not preclude corner-case situations where some in-relocation
> code requires knowing both the pre- and post-relocation addresses of
> a symbol; usually it's a matter of looking at the "in-RAM" U-Boot image
> vs the "in-FLASH" U-Boot image, e.g. when relocating, we copy
> U-Bootfrom "source" to "destination" and then fix relocation using
> the "source" relocation table, because there is no "destination"
> relocation table. Normally these corner-case situations only arise near
> the relocation code itself.
> 
> However, if e.g. some fields of this debug() call should be the
> "initial" addresses while the debug() is executed at "final" location,
> just point them to me.

board_init_f() is called before relocation, so as long as there is no target
having different build- and run-time addresses at this point (which AFAIK is the
case), the changes made to this function are safe.

And display_banner() is called from board_init_f(), so everything should be fine
here.

Hence, we basically just have to make sure that there is no exception board
here.

> > > @@ -277,13 +277,13 @@ void board_init_f(ulong bootflag)
> > >  
> > >  	memset((void *)gd, 0, sizeof(gd_t));
> > >  
> > > -	gd->mon_len = _bss_end_ofs;
> > > +	gd->mon_len = (ulong)&__bss_end - (ulong)_start;
> > >  #ifdef CONFIG_OF_EMBED
> > >  	/* Get a pointer to the FDT */
> > >  	gd->fdt_blob = _binary_dt_dtb_start;
> > >  #elif defined CONFIG_OF_SEPARATE
> > >  	/* FDT is at end of image */
> > > -	gd->fdt_blob = (void *)(_end_ofs + _TEXT_BASE);
> > > +	gd->fdt_blob = &_end;
> > >  #endif
> > >  	/* Allow the early environment to override the fdt address */
> > >  	gd->fdt_blob = (void *)getenv_ulong("fdtcontroladdr", 16,
> > > @@ -451,7 +451,7 @@ void board_init_f(ulong bootflag)
> > >  
> > >  	gd->relocaddr = addr;
> > >  	gd->start_addr_sp = addr_sp;
> > > -	gd->reloc_off = addr - _TEXT_BASE;
> > > +	gd->reloc_off = addr - (ulong)&_start;
> > >  	debug("relocation Offset is: %08lx\n", gd->reloc_off);
> > >  	if (new_fdt) {
> > >  		memcpy(new_fdt, gd->fdt_blob, fdt_size);
> > > @@ -516,7 +516,7 @@ void board_init_r(gd_t *id, ulong dest_addr)
> > >  	gd->flags |= GD_FLG_RELOC;	/* tell others: relocation done */
> > >  	bootstage_mark_name(BOOTSTAGE_ID_START_UBOOT_R, "board_init_r");
> > >  
> > > -	monitor_flash_len = _end_ofs;
> > > +	monitor_flash_len = (ulong)&__rel_dyn_end - (ulong)_start;
> > >  
> > >  	/* Enable caches */
> > >  	enable_caches();
> > 
> > [...]

Best regards,
Benoît