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

Benoît Thébaudeau
2013-10-15T11:07:53Z

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@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