[U-Boot] [PATCH 4/4] ARM: fix CONFIG_SPL_MAX_SIZE semantics

Albert ARIBAUD
2013-04-09T17:43:06Z

Hi Beno?t,

On Tue, 9 Apr 2013 16:24:36 +0200 (CEST), Beno?t Th?baudeau
<benoit.thebaudeau@advansee.com> wrote:

> Hi Albert,
>
> On Tuesday, April 9, 2013 4:23:58 PM, Albert ARIBAUD wrote:
> > Hi Beno?t,
> >
> > On Mon, 8 Apr 2013 23:43:37 +0200 (CEST), Beno?t Th?baudeau
> > <benoit.thebaudeau@advansee.com> wrote:
> >
> > > Hi Albert,
> >
> > > > diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
> > > > index 3c0d99c..89ef9ce 100644
> > > > --- a/arch/arm/cpu/u-boot-spl.lds
> > > > +++ b/arch/arm/cpu/u-boot-spl.lds
> > > > @@ -88,6 +88,12 @@ SECTIONS
> > > > /DISCARD/ : { *(.gnu*) }
> > > > }
> > > >
> > > > -#if defined(CONFIG_SPL_TEXT_BASE) && defined(CONFIG_SPL_MAX_SIZE)
> > > > -ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE + CONFIG_SPL_MAX_SIZE), "SPL
> > > > image
> > > > too big");
> > > > +#if defined(CONFIG_SPL_MAX_SIZE)
> > > > +ASSERT(__image_copy_end - __image_copy_start < (CONFIG_SPL_MAX_SIZE), \
> > >
> > > The possible relocation and MMU data is also part of the binary image file,
> > > so
> > > that would be __bss_start rather than __image_copy_end above, and README
> > > should
> > > be updated to reflect this.
> >
> > Actually, mmutable is not used in any SPL; it is used only in targets
> > h2200, lubbock, palmtc, pxa255_idp and xaeniax, none of which use SPL.
> > I have just confirmed this with a MAKEALL -a arm and a grep on all map
> > files.
> >
> > This presence of mmutable in u-boot-spl.lds is in fact an overlook
> > that I missed when I created this file from u-boot.lds. I have just
> > finished verifying that removing the mmutable section altogether does
> > not change a single bit to any of the 309 ARM platforms currently built
> > under MAKEALL -a arm.
> >
> > I'll remove mmutable entries from u-boot-spl.lds in V2.
>
> OK, that's perfect for MMU data, but what about relocation data?

Relocation data should not exist for SPLs, which do not relocate.

Unfortunately, most tegra and some exynos have start.S code going
through the relocation loop even for their SPL; that's cardhu,
colibri_t20_iris, dalmore, harmony, medcom-wide, origen, paz00, plutux,
seaboard, smdkv310, tec, trimslice, ventana, and whistler.

Tegras do it for their arm720t; Exynos' probably do something similar.
I am not going to try and change some start.S so close in time to
release. :)

Fortunately, for all the SPLs that fail building if I remove .rel.dyn
and .dynsym, these sections are actually empty, i.e. their __end is
equal to their __image_copy_end. I have manually verified both reloc
section emptiness and equality of _end and __image_copy_end.

Therefore I'll leave the ASSERT() as written in V2, and will provide a
separate patch for fixing the Tegra / Exynos unneeded relocation data
issue.

> Best regards,
> Beno?t

Amicalement,
--
Albert.