[PATCH v2] arm: Add OVERLAY command to BSS section on ARM64

[Ilias Apalodimas]

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