Patchwork [U-Boot,V3,2/5] ARM: enhance u-boot.lds to detect over-sized SPL

login
register
mail settings
Submitter Stephen Warren
Date Oct. 22, 2012, 4:19 p.m.
Message ID <1350922776-30909-2-git-send-email-swarren@wwwdotorg.org>
Download mbox | patch
Permalink /patch/193206/
State Accepted
Delegated to: Tom Warren
Headers show

Comments

Stephen Warren - Oct. 22, 2012, 4:19 p.m.
From: Stephen Warren <swarren@nvidia.com>

Add an ASSERT() to u-boot.lds to detect an SPL that doesn't fit within
SPL_TEXT_BASE..SPL_MAX_SIZE.

Different .lds files implement this check in two possible ways:
1) An ASSERT() like this
2) Defining a MEMORY region of size SPL_MAX_SIZE, and re-directing all
   linker output into that region. Since u-boot.lds is used for both
   SPL and main U-Boot, this would entail only sometimes defining a
   MEMORY region, and only sometimes performing that redirection, and
   hence option (1) was deemed much simpler, and hence implemented.

Note that this causes build failures at least for NVIDIA Tegra Seaboard
and Ventana. However, these are legitimate; the SPL doesn't fit within
the required space, and this does cause runtime issues.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
Acked-by: Simon Glass <sjg@chromium.org>
Acked-by: Allen Martin <amartin@nvidia.com>
---
v3: No change.
v2: New patch; replaced checks in Makefile.
---
 arch/arm/cpu/u-boot.lds |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
Tom Rini - Oct. 22, 2012, 5:52 p.m.
On Mon, Oct 22, 2012 at 10:19:33AM -0600, Stephen Warren wrote:

> From: Stephen Warren <swarren@nvidia.com>
> 
> Add an ASSERT() to u-boot.lds to detect an SPL that doesn't fit within
> SPL_TEXT_BASE..SPL_MAX_SIZE.
> 
> Different .lds files implement this check in two possible ways:
> 1) An ASSERT() like this
> 2) Defining a MEMORY region of size SPL_MAX_SIZE, and re-directing all
>    linker output into that region. Since u-boot.lds is used for both
>    SPL and main U-Boot, this would entail only sometimes defining a
>    MEMORY region, and only sometimes performing that redirection, and
>    hence option (1) was deemed much simpler, and hence implemented.
> 
> Note that this causes build failures at least for NVIDIA Tegra Seaboard
> and Ventana. However, these are legitimate; the SPL doesn't fit within
> the required space, and this does cause runtime issues.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Acked-by: Simon Glass <sjg@chromium.org>
> Acked-by: Allen Martin <amartin@nvidia.com>

This isn't quite what I envisoned at first (see 
arch/arm/cpu/armv7/omap-common/u-boot-spl.lds) but I think for the
generic linker script, this is the least instrusive method.

Acked-by: Tom Rini <trini@ti.com>

And since parts 1 and 2 are generic code, I've assigned them to Albert
in patchwork.  It's his call if he wants to take them or have them all
come via the tegra tree.
Simon Glass - Oct. 25, 2012, 6:11 p.m.
On Mon, Oct 22, 2012 at 10:52 AM, Tom Rini <trini@ti.com> wrote:
> On Mon, Oct 22, 2012 at 10:19:33AM -0600, Stephen Warren wrote:
>
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> Add an ASSERT() to u-boot.lds to detect an SPL that doesn't fit within
>> SPL_TEXT_BASE..SPL_MAX_SIZE.
>>
>> Different .lds files implement this check in two possible ways:
>> 1) An ASSERT() like this
>> 2) Defining a MEMORY region of size SPL_MAX_SIZE, and re-directing all
>>    linker output into that region. Since u-boot.lds is used for both
>>    SPL and main U-Boot, this would entail only sometimes defining a
>>    MEMORY region, and only sometimes performing that redirection, and
>>    hence option (1) was deemed much simpler, and hence implemented.
>>
>> Note that this causes build failures at least for NVIDIA Tegra Seaboard
>> and Ventana. However, these are legitimate; the SPL doesn't fit within
>> the required space, and this does cause runtime issues.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> Acked-by: Simon Glass <sjg@chromium.org>
>> Acked-by: Allen Martin <amartin@nvidia.com>

I tested this series on seaboard.

Tested-by: Simon Glass <sjg@chromium.org>

>
> This isn't quite what I envisoned at first (see
> arch/arm/cpu/armv7/omap-common/u-boot-spl.lds) but I think for the
> generic linker script, this is the least instrusive method.
>
> Acked-by: Tom Rini <trini@ti.com>
>
> And since parts 1 and 2 are generic code, I've assigned them to Albert
> in patchwork.  It's his call if he wants to take them or have them all
> come via the tegra tree.
>
> --
> Tom
>
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

Patch

diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
index 9153c3d..ec2a273 100644
--- a/arch/arm/cpu/u-boot.lds
+++ b/arch/arm/cpu/u-boot.lds
@@ -92,3 +92,7 @@  SECTIONS
 	/DISCARD/ : { *(.interp*) }
 	/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");
+#endif