On Tue, 6 Feb 2024 at 16:44, Ilias Apalodimas ilias.apalodimas@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+renesas@mailbox.org
Cc: Simon Glass sjg@chromium.org Cc: Tom Rini trini@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