[U-Boot,3/4] ARM: tegra: check for SPL size overflow in makefile

From: Stephen Warren <swarren@nvidia.com>

If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will likely
corrupt the main U-Boot binary during execution, causing the main U-Boot
binary to fail. Check for this situation during the build to avoid
extremely annoying and hard-to-find bugs. Note that checking the size of
u-boot-spl.bin is not enough, since BSS size doesn't affect the size of
u-boot-spl.bin.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 Makefile |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

Hi Stephen,

On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will likely
> corrupt the main U-Boot binary during execution, causing the main U-Boot
> binary to fail. Check for this situation during the build to avoid
> extremely annoying and hard-to-find bugs. Note that checking the size of
> u-boot-spl.bin is not enough, since BSS size doesn't affect the size of
> u-boot-spl.bin.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  Makefile |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 425adf4..2c2d8b6 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -522,6 +522,11 @@ dtbfile=
>  endif
>
>  $(obj)u-boot-$(nodtb)-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin $(dtbfile)
> +               ebss=`${CROSS_COMPILE}objdump -t spl/u-boot-spl|awk '/__bss_end__/ {print "0x"$$1}'` ; \
> +               if [ $$(($$ebss)) -gt $$(($(CONFIG_SYS_TEXT_BASE))) ]; then \
> +                       echo ERROR: SPL BSS ends beyond CONFIG_SYS_TEXT_BASE > /dev/stderr; \
> +                       exit 1; \
> +               fi

Just wanted to check that this works ok for hex values?

>                 $(OBJCOPY) ${OBJCFLAGS} --pad-to=$(CONFIG_SYS_TEXT_BASE) -O binary $(obj)spl/u-boot-spl $(obj)spl/u-boot-spl-pad.bin
>                 cat $(obj)spl/u-boot-spl-pad.bin $(obj)u-boot.bin $(dtbfile) > $@
>                 rm $(obj)spl/u-boot-spl-pad.bin
> --
> 1.7.0.4
>

Regards,
Simon

On 10/17/2012 06:03 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Tue, Oct 16, 2012 at 2:50 PM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will likely
>> corrupt the main U-Boot binary during execution, causing the main U-Boot
>> binary to fail. Check for this situation during the build to avoid
>> extremely annoying and hard-to-find bugs. Note that checking the size of
>> u-boot-spl.bin is not enough, since BSS size doesn't affect the size of
>> u-boot-spl.bin.

>> diff --git a/Makefile b/Makefile

>>  $(obj)u-boot-$(nodtb)-tegra.bin: $(obj)spl/u-boot-spl.bin $(obj)u-boot.bin $(dtbfile)
>> +               ebss=`${CROSS_COMPILE}objdump -t spl/u-boot-spl|awk '/__bss_end__/ {print "0x"$$1}'` ; \
>> +               if [ $$(($$ebss)) -gt $$(($(CONFIG_SYS_TEXT_BASE))) ]; then \
>> +                       echo ERROR: SPL BSS ends beyond CONFIG_SYS_TEXT_BASE > /dev/stderr; \
>> +                       exit 1; \
>> +               fi
> 
> Just wanted to check that this works ok for hex values?

Yes, the reason I used $(( )) around the value is because it was hex, so
I needed to convert it to an integer for -gt.

On Tue, Oct 16, 2012 at 03:50:08PM -0600, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will likely
> corrupt the main U-Boot binary during execution, causing the main U-Boot
> binary to fail. Check for this situation during the build to avoid
> extremely annoying and hard-to-find bugs. Note that checking the size of
> u-boot-spl.bin is not enough, since BSS size doesn't affect the size of
> u-boot-spl.bin.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Can't you do this in the linker script like we do for other SPL size
constraints?  Or am I just mis-reading how this is unique and that
link-time check can't be used?  Thanks!

On 10/18/2012 10:27 AM, Tom Rini wrote:
> On Tue, Oct 16, 2012 at 03:50:08PM -0600, Stephen Warren wrote:
> 
>> From: Stephen Warren <swarren@nvidia.com>
>> 
>> If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will
>> likely corrupt the main U-Boot binary during execution, causing
>> the main U-Boot binary to fail. Check for this situation during
>> the build to avoid extremely annoying and hard-to-find bugs. Note
>> that checking the size of u-boot-spl.bin is not enough, since BSS
>> size doesn't affect the size of u-boot-spl.bin.
>> 
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> 
> Can't you do this in the linker script like we do for other SPL
> size constraints?  Or am I just mis-reading how this is unique and
> that link-time check can't be used?  Thanks!

Ah, there aren't any such checks in the linker script I looked at, so
I wasn't aware of this capability. I found the following in a
different linker script:

ASSERT(__bss_end__ <= 0xfff01000, "NAND bootstrap too big");

I agree using that technique would make sense; I'll try it out.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/18/12 13:45, Stephen Warren wrote:
> On 10/18/2012 10:27 AM, Tom Rini wrote:
>> On Tue, Oct 16, 2012 at 03:50:08PM -0600, Stephen Warren wrote:
>> 
>>> From: Stephen Warren <swarren@nvidia.com>
>>> 
>>> If the SPL extends beyond CONFIG_SYS_TEXT_BASE, then it will 
>>> likely corrupt the main U-Boot binary during execution,
>>> causing the main U-Boot binary to fail. Check for this
>>> situation during the build to avoid extremely annoying and
>>> hard-to-find bugs. Note that checking the size of
>>> u-boot-spl.bin is not enough, since BSS size doesn't affect the
>>> size of u-boot-spl.bin.
>>> 
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> 
>> Can't you do this in the linker script like we do for other SPL 
>> size constraints?  Or am I just mis-reading how this is unique
>> and that link-time check can't be used?  Thanks!
> 
> Ah, there aren't any such checks in the linker script I looked at,
> so I wasn't aware of this capability. I found the following in a 
> different linker script:
> 
> ASSERT(__bss_end__ <= 0xfff01000, "NAND bootstrap too big");
> 
> I agree using that technique would make sense; I'll try it out.

There's a few different ones, sadly.  grep around on CONFIG_SPL_MAX_SIZE.

- -- 
Tom
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQIcBAEBAgAGBQJQgGucAAoJENk4IS6UOR1WmZwP/jFzQPiYD4w33IMRtuGe2ip9
QOZUIsvVYUx7ufwhQeUUMjf/JQ/c+qw+GZeVRsj1pdsw0frjuIk1Z1smgx3QAa7w
yXAlQ7bEWhfl2DsXZeVcLjaidTIkeZl6azVOfNEV6pLDlW330z9cOun3AjtV0a1f
kXVGnmyEOsaLURI1DYfkoLD0p+uTNED1VFpTctPWMrCwBoS10M27bwAWkaIjFScu
8Keg+ThAa7Vac8+YKqNPtYuyAO2ZMVXISDe1LLvplcmCtow9g1qm7YnV2QsWqhrM
U8BfYwJdKDf3aQcqiTN5rQD0x7pIHC+3wP2BMACgN3cNu//RY373+D3Ouq3MVEEi
sUw0aMmKyc0Nehj2ONn+tgguZxKrMPNokLYz++/kTOijPZ1AtI1mObsfeT+aV1HW
vVBs5ThKILWOj+pGak7WKhO+yfWiRhmk9JgqAmvf/+uq5vKVr2bF8i2AGc88eAbN
TL8Ct8/u5BDJsNJpY1GMFw1mIR9ZxBBCEbqt3RjRLR3sZLX86qrbRFzVGF1BzPBl
xFYJMRObl4xpgTjf/r9JdROL0c6rizlHvNKwfJCjjea/F8Q0WFaWtlvx15SZQT6Y
s8Xf9iCUPTLLN6hMf0cgN42WCg0THqTbINpa66BsDYPxrVPbBf1hdOb5dhOOMILo
v5VNdn4zdR/d58JLBa0Z
=2haV
-----END PGP SIGNATURE-----