[PATCH v2 09/18] armv8: Support not having separate BSS

[Ilias Apalodimas]

On Fri, 27 Sept 2024 at 00:53, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Ilias,
>
> On Wed, 25 Sept 2024 at 15:40, Ilias Apalodimas
> <ilias.apalodimas at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Wed, 25 Sept 2024 at 15:48, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Ilias,
> > >
> > > On Mon, 23 Sept 2024 at 11:49, Ilias Apalodimas
> > > <ilias.apalodimas at linaro.org> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Fri, 20 Sept 2024 at 10:25, Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Separate BSS is current mandatory on armv8 but this is not useful for
> > > > > early boot phases. Add support for the combined BSS.
> > > > >
> > > > > Use an #ifdef to avoid using CONFIG_SPL_BSS_START_ADDR which is not
> > > > > valid in this case.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > > > ---
> > > > >
> > > > > (no changes since v1)
> > > > >
> > > > >  arch/arm/cpu/armv8/u-boot-spl.lds | 12 ++++++++++++
> > > > >  1 file changed, 12 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds
> > > > > index 215cedd69a8..fed69644b55 100644
> > > > > --- a/arch/arm/cpu/armv8/u-boot-spl.lds
> > > > > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
> > > > > @@ -13,8 +13,10 @@
> > > > >
> > > > >  MEMORY { .sram : ORIGIN = IMAGE_TEXT_BASE,
> > > > >                 LENGTH = IMAGE_MAX_SIZE }
> > > > > +#ifdef CONFIG_SPL_SEPARATE_BSS
> > > > >  MEMORY { .sdram : ORIGIN = CONFIG_SPL_BSS_START_ADDR,
> > > > >                 LENGTH = CONFIG_SPL_BSS_MAX_SIZE }
> > > > > +#endif
> > > > >
> > > > >  OUTPUT_FORMAT("elf64-littleaarch64", "elf64-littleaarch64", "elf64-littleaarch64")
> > > > >  OUTPUT_ARCH(aarch64)
> > > > > @@ -56,12 +58,22 @@ SECTIONS
> > > > >         _end = .;
> > > > >         _image_binary_end = .;
> > > > >
> > > > > +#ifdef CONFIG_SPL_SEPARATE_BSS
> > > > >         .bss : {
> > > > >                 __bss_start = .;
> > > > >                 *(.bss*)
> > > > >                 . = ALIGN(8);
> > > > >                 __bss_end = .;
> > > > >         } >.sdram
> > > > > +#else
> > > > > +       .bss (NOLOAD) : {
> > > > > +               __bss_start = .;
> > > > > +               *(.bss*)
> > > > > +                . = ALIGN(8);
> > > > > +               __bss_end = .;
> > > > > +       } >.sram
> > > > > +#endif
> > > >
> > > > This is still going to be separate. The only difference isn't that
> > > > it's not loaded. If you want to combine it with a region, you got to
> > > > do something similar to what we have in armv7, where it overlaps with
> > > > rel.dyn
> > >
> > > I don't mind about combining it with a region, But SPL doesn't have
> > > relocation data, right?
> > >
> > > My meaning of 'separate' is as described in CONFIG_SPL_SEPARATE_BSS,
> > > i.e. being kept in SDRAM.
> >
> > Ok, is the NOLOAD really needed? I would prefer the ifdef around .sdram/.sram
> >
> > In any case
> > Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>
> Oh I see... well the NOLOAD stops it writing zero data into the image,
> as I understand it.

It doesn't load that section into memory.

>  Actually I wonder why we don't have NOLOAD for the
> separate-BSS case? If we can use NOLOAD for both then we can indeed
> shrink the #ifdef

If .bss is zero'ed out properly (which I think it is) we can use NOLOAD on both

Thanks
/Ilias
>
> >
> >
> > >
> > > >
> > > > Thanks
> > > > /Ilias
> > > > > +       __bss_size = __bss_end - __bss_start;
> > > > >
> > > > >         /DISCARD/ : { *(.rela*) }
> > > > >         /DISCARD/ : { *(.dynsym) }
> > > > > --
> > > > > 2.43.0
> > > > >
>
> Regards,
> Simon