Albert, > -----Original Message----- > From: Stephen Warren [mailto:swar...@wwwdotorg.org] > Sent: Friday, April 05, 2013 8:59 AM > To: Tom Rini > Cc: Albert ARIBAUD; Scott Wood; u-boot@lists.denx.de; > step...@theia.denx.de; Tom Warren; Simon Glass; Allen Martin > Subject: Re: [U-Boot] [PATCH] ARM: Fix __bss_start and __bss_end in linker > scripts
I just fetched & merged TOT u-boot-arm (getting ready for a PR from u-boot-tegra -> u-boot-arm), and did a test build of all Tegra boards. I get the following linker (ld) error on all builds due to this commit (abbecf4c8, ARM: Fix __bss_start and __bss_end in linker scripts): arm-linux-gnueabi-ld.bfd:u-boot.lds:44: syntax error This is pointing to the 'HIDDEN(__bss_base = .);' lines added in this change (in the generated u-boot.lds in the root dir). I'm seeing this with both gcc 4.4.1/ld 2.19.51 and gcc 4.6.2/ld 2.22 tools. What compiler/binutils did you use to build/test this? Tom > > 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. -- nvpublic _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
