On Tue, 6 Feb 2024 at 16:44, Ilias Apalodimas <ilias.apalodi...@linaro.org> wrote: > > Hi Marek, > > On Sun, Dec 17, 2023 at 01:33:39AM +0100, Marek Vasut wrote: > > Avoid allocating and loading the BSS section. > > > > Can we elaborate a bit more on why we need this? AFAICT there's no code > loading those segments in memory and swapping them, so why do we need the > OVERLAY? On top of that the ALLOC flag seems to be missing? The .bss > section doesn't need to be loaded indeed, since we can memset it to 0, but > it does need proper backing memory. > > another thing I noticed is the bss_start and end are defined as sections of > their own and a bit of git history led me to 3ebd1cbc49f00050. But the > linker script will emit absolute addresses only if the symbol is defined > outside a section. IOW applying this makes the value expressed as a fixed > offset from the base of the section and the bss_start/end sections go away > > --- a/arch/arm/cpu/armv8/u-boot.lds > +++ b/arch/arm/cpu/armv8/u-boot.lds > @@ -151,17 +151,11 @@ SECTIONS > > . = ALIGN(8); > > - .bss_start : { > - KEEP(*(.__bss_start)); > - } > - > .bss : { > + .bss_start = .; > *(.bss*) > . = ALIGN(8); > - } > - > - .bss_end : { > - KEEP(*(.__bss_end)); > + .bss_end = .; > } > > /DISCARD/ : { *(.dynsym) }
Apologies the patch above was wrong. The . in bss_start/end isn't needed and I was missing a __ before the symbol definition. The diff below though should produce the same binary without the bss_start/end sections and the .bss section marked as ALLOC only diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds index fb6a30c922f7..8105cfd122af 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -151,17 +151,11 @@ SECTIONS . = ALIGN(8); - .bss_start : { - KEEP(*(.__bss_start)); - } - .bss : { + __bss_start = .; *(.bss*) . = ALIGN(8); - } - - .bss_end : { - KEEP(*(.__bss_end)); + __bss_end = .; } /DISCARD/ : { *(.dynsym) } diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c index 857879711c6a..59dfc53851a8 100644 --- a/arch/arm/lib/sections.c +++ b/arch/arm/lib/sections.c @@ -19,8 +19,8 @@ * aliasing warnings. */ -char __bss_start[0] __section(".__bss_start"); -char __bss_end[0] __section(".__bss_end"); +//char __bss_start[0] __section(".__bss_start"); +//char __bss_end[0] __section(".__bss_end"); char __image_copy_start[0] __section(".__image_copy_start"); char __image_copy_end[0] __section(".__image_copy_end"); char __rel_dyn_start[0] __section(".__rel_dyn_start"); > > > leads to this: > > 11 .bss 000090b0 0000000000102b00 0000000000102b00 00112aa0 2**7 > ALLOC > > P.S: I am playing around with rewriting the linker script and mapping > u-boot with proper permissions at least for armv8. If this patch is neeeded > *now* can someone explain why? Otherwise I'll clean it up once I test my > patches enough > > Thanks > /Ilias > > > $ aarch64-linux-gnu-objdump -Sh u-boot > > > > Before: > > 10 .bss_start 00000000 00000000000f21d8 00000000000f21d8 001021d8 > > 2**0 > > CONTENTS, ALLOC, LOAD, DATA > > 11 .bss 000068f8 00000000000f2200 00000000000f2200 001021d8 > > 2**6 > > ALLOC > > 12 .bss_end 00000000 00000000000f8af8 00000000000f8af8 00108af8 > > 2**0 > > CONTENTS, ALLOC, LOAD, DATA > > > > After: > > 10 .bss_start 00000000 00000000000bf990 00000000000bf990 001021e0 > > 2**0 > > CONTENTS > > 11 .bss 000068e8 00000000000bf990 00000000000bf990 001021e0 > > 2**4 > > CONTENTS > > 12 .bss_end 00000000 00000000000c6278 00000000000c6278 00108ac8 > > 2**0 > > CONTENTS > > > > Signed-off-by: Marek Vasut <marek.vasut+rene...@mailbox.org> > > --- > > Cc: Simon Glass <s...@chromium.org> > > Cc: Tom Rini <tr...@konsulko.com> > > --- > > V2: Replicate arch/arm/cpu/u-boot.lds BSS part verbatim > > --- > > arch/arm/cpu/armv8/u-boot.lds | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds > > index fb6a30c922f..ebdc079552d 100644 > > --- a/arch/arm/cpu/armv8/u-boot.lds > > +++ b/arch/arm/cpu/armv8/u-boot.lds > > @@ -151,16 +151,18 @@ SECTIONS > > > > . = ALIGN(8); > > > > - .bss_start : { > > + .bss_start __rel_dyn_start (OVERLAY) : { > > KEEP(*(.__bss_start)); > > + __bss_base = .; > > } > > > > - .bss : { > > + .bss __bss_base (OVERLAY) : { > > *(.bss*) > > . = ALIGN(8); > > + __bss_limit = .; > > } > > > > - .bss_end : { > > + .bss_end __bss_limit (OVERLAY) : { > > KEEP(*(.__bss_end)); > > } > > > > -- > > 2.43.0 > >