[PATCH v3 2/7] arm: clean up v7 and v8 linker scripts for bss_start/end

[Ilias Apalodimas]

Hi Richard,


On Thu, 14 Mar 2024 at 00:43, Richard Henderson
<richard.henderson at linaro.org> wrote:
>
> On 3/13/24 11:43, Ilias Apalodimas wrote:
> > Hi Richard,
> >
> > On Wed, 13 Mar 2024 at 22:19, Richard Henderson
> > <richard.henderson at linaro.org> wrote:
> >>
> >> On 3/13/24 06:23, Ilias Apalodimas wrote:
> >>> +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
> >>> @@ -63,18 +63,11 @@ SECTIONS
> >>>
> >>>        _image_binary_end = .;
> >>>
> >>> -     .bss_start (NOLOAD) : {
> >>> -             . = ALIGN(8);
> >>> -             KEEP(*(.__bss_start));
> >>> -     } >.sdram
> >>> -
> >>> -     .bss (NOLOAD) : {
> >>> +     .bss : {
> >>> +             __bss_start = .;
> >>>                *(.bss*)
> >>> -              . = ALIGN(8);
> >>> -     } >.sdram
> >>> -
> >>> -     .bss_end (NOLOAD) : {
> >>> -             KEEP(*(.__bss_end));
> >>> +             . = ALIGN(8);
> >>> +             __bss_end = .;
> >>
> >> Still missing the alignment on .bss, previously in .bss_start.
> >
> > Since this is emitted in .sdram memory I can't define it as
> > .bss ALIGN(8) : {....} since the calculated memory will be outside the
> > sdram-defined region
> >
> > I could define it as
> > .bss : {
> >          . = ALIGN(8);
> >          __bss_start = .;
> >          ......
> > }
> >
> > But instead, I added an assert at the bottom which will break the
> > linking if the __bss_start is not 8byte aligned.
>
> I think it would be clearer to assert on __bss_start address rather than
> CONFIG_SPL_BSS_START_ADDR, if that's possible.  If not, then the constant will have to do.


Fair enough and it is possible. I can convert that assertion to
ASSERT(ADDR(.bss) % 8 == 0, ".bss must be 8-byte aligned");

Keep in mind that .bss, due to the linker aligning the section to the
strictest input section requirement will end up aligning that to 8b
regardless. IOW compiling with CONFIG_SPL_BSS_START_ADDR=0x1001, won't
break the linking since .bss will end up at 0x1008.

Testing the assertion with a section definition which looks like this
will trigger the assert.
        .bss 0x1001 : {
                __bss_start = .;
                *(.bss*)
                . = ALIGN(8);
                __bss_end = .;
        } >.sdram

I don't have a really strong opinion here. I think the assertion above
is ok. What we could also do is

        .bss  : {
                . = ALIGN(8);
                __bss_start = .;
                *(.bss*)
                . = ALIGN(8);
                __bss_end = .;
        } >.sdram

which would also fix the __bss_start alignment regardless of the .bss
output address.
For example with the linker entry below __bss_start will end up at 0x1008
        .bss  0x1001 : {
                . = ALIGN(8);
                __bss_start = .;
                *(.bss*)
                . = ALIGN(8);
                __bss_end = .;
        } >.sdram

As you already mentioned, the bss_end - bss_start % 8 == 0 can be
fixed by using memset. So since I plan to continue working on this to
get RO, RW^x etc mappings I think I'll just go for the assertion for
now -- but test the .bss address instead of the config option.

Anyone objects?

Thanks
/Ilias

>
>
> r~