[U-Boot] [PATCH] ARM: Fix __bss_start and __bss_end in linker scripts

[Stephen Warren]

On 04/05/2013 07:53 AM, Tom Rini wrote:
> On Fri, Apr 05, 2013 at 08:00:43AM +0200, Albert ARIBAUD wrote:
>> Hi Beno??t,
>> 
>> CC:ing Stephen Warren who wrote commit 2b7818d4 which git blame
>> tells me added the ASSERT() to arch/arm/cpu/u-boot.lds, and Tom
>> Rini to help decide what to do.
> [snip]
>>>>>> Looks good, but what about the __bss_end in 
>>>>>> ASSERT(__bss_end < (CONFIG_SPL_TEXT_BASE +
>>>>>> CONFIG_SPL_MAX_SIZE), "SPL image too big"); ?
>>>>>> 
>>>>>> Shouldn't it be replaced with __bss_limit, or even
>>>>>> better, with __image_copy_end (why should BSS be taken
>>>>>> into account for SPL image size?)?
>>>>> 
>>>>> I meant __bss_base, not __image_copy_end.
>>>> 
>>>> Part of SPL can use BSS, once board_init_f() has handed
>>>> things over to board_init_r(),
>>> 
>>> I agree with that.
>>> 
>>>> and this test is meant to ensure that .text+.data+.bss fits
>>>> in the RAM available to SPL. This ASSERT() compares the
>>>> upper limit of this RAM to the upper limit of the SPL needs,
>>>> i.e. __bss_end.
>>>> 
>>>> IOW, this ASSERT is about how much memory SPL will use when
>>>> it runs,
>>> 
>>> This is where there might be an issue with the definition /
>>> usage of CONFIG_SPL_MAX_SIZE.
>>> 
>>> I had understood that this is the purpose of this assert, but
>>> this usage of CONFIG_SPL_MAX_SIZE, while frequent on arm,
>>> conflicts with its definition in README, as well as with some
>>> arm and other usages, e.g. in include/configs/am335x_evm.h or
>>> include/configs/p1_p2_rdb_pc.h.
>>> 
>>> The README's definition is also how I understood this config
>>> when discussing it with Scott about my 16/30, which led to the
>>> #error test that was added at some point to this patch.
>> 
>> Sorry, I'd missed that. Adding Stephen for any comment regarding
>> the ASSERT() and CONFIG_SPL_MAX_SIZE semantics.
> 
> Yes.  I'm a little confused about this one as well now.
...
> If I read all of the tegra files correctly, the way they work is
> that SPL and U-Boot are both loaded into memory, and thus they're
> ensuring that the BSS won't overlap with the full U-Boot that's
> right behind.

Yes exactly.

I'll explain the whole situation just so there's something to refer to
in the archives.

Tegra has two different types of CPU. As far as boot process goes, the
AVP (Audio/Video Processor) is what is released from reset at system
boot time, and what runs the built-in Tegra boot ROM and the U-Boot
SPL. The AVP is an ARM7TDMI. The SPL initializes the main CPU complex
(A9/A15), and causes it to execute the main U-Boot.

The boot process is:

* Entire U-Boot binary (SPL+pad+U-Boot) sits in boot flash.
* Boot ROM code running on AVP:
** Initializes DRAM controller.
** Copies entire SPL+pad+U-Boot to DRAM byte-for-byte.
** Jumps to U-Boot SPL.
* U-Boot SPL running on AVP:
** Initializes clocks/... required to boot A9 CPUs.
** Set up the A9 reset vector to point at the main U-Boot code,
   and releases A9 CPUs from reset.
* Main U-Boot running on A9:
** Runs in the typical fashion, including regular relocation.

Thus, the in-flash layout of SPL+U-Boot must match the in-DRAM layout,
since the boot ROM just copies it byte-for-byte, and the SPL does
nothing to move the appended main U-Boot out-of-the-way before executing.

The gap between SPL and concatenated U-Boot must be large enough to
hold the BSS (since SPL will use BSS without moving U-boot), but is
also larger so that we can hard-code the TEXT_BASE of the main U-Boot
to some stable value, without having to move it about often if SPL
changes size.

So the U-Boot binary layout in flash/DRAM is:

+---------------------+
| SPL                 |
+---------------------+
| Pad, including      |
| space for BSS       |
+---------------------+
| Main U-Boot         |
+---------------------+

The assert in question ensures that the start of the "Main U-Boot" in
the diagram above is at a large enough address that it doesn't overlap
the SPL's BSS usage in RAM.

It's possible that I chose the wrong defines to validate when writing
that assert. As long as the assert continues to ensure the above image
layout in the generated u-boot*.bin, I have no problems with it being
changed.