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

Benoît Thébaudeau
2013-04-09T17:39:29Z

Hi Albert,

On Tuesday, April 9, 2013 7:43:06 PM, Albert ARIBAUD wrote:
> 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.

That's perfect.

Best regards,
Beno?t