Hi Eugen
On Tue, 28 May 2024 at 09:40, Eugen Hristev <eugen.hris...@collabora.com> wrote:
>
> On 5/28/24 09:18, Ilias Apalodimas wrote:
> > commit 6e2228fb052b ("Merge patch series "Clean up arm linker scripts")
> > was cleaning up linker scripts for armv7 and v8 but was leaving
> > _end and __secure_stack_start/end.
> >
> > commit d0b5d9da5de2 ("arm: make _end compiler-generated")
> > was moving _end to be compiler generated. _end is defined as c variable
> > in its own section to force the compiler emit relative a reference.
> > However, defining those in the linker script will do the same thing
> > since [0].
> >
> > So let's remove the special sections from the linker scripts, the
> > variable definitions from sections.c and define them as a symbols.
> > It's worth noting that _image_binary_end symbol is now redundant and
> > can be removed in the future.
> >
> > - SPL
> >
> > The .end section has been removed from the new binary
> > [ 5] .end
> > PROGBITS 00000000fffdf488 000000000002f488 0
> > 0000000000000000 0000000000000000 0 1
> > [0000000000000003]: WRITE, ALLOC
> >
> > $~ bloat-o-meter kria_old/spl/u-boot-spl krina_new/spl/u-boot-spl
> > add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> > Function old new delta
> > Total: Before=115980, After=115980, chg +0.00%
> >
> > $~ readelf -sW kria_old/u-boot kria_new/u-boot | grep -w _end
> > 12047: 000000000813a0f0 0 OBJECT GLOBAL DEFAULT 11 _end
> > 12047: 000000000813a118 0 NOTYPE GLOBAL DEFAULT 11 _end
> >
> > $~ readelf -sW kria_old/spl/u-boot-spl kria_new/spl/u-boot-spl | grep -w
> > _end
> > 1605: 00000000fffdf488 0 OBJECT GLOBAL DEFAULT 5 _end
> > 1603: 00000000fffdf498 0 NOTYPE GLOBAL DEFAULT 4 _end
> >
> > $~ readelf -sW old/u-boot new/u-boot | grep -w _end
> > 8847: 0000000000103710 0 OBJECT GLOBAL DEFAULT 11 _end
> > 8847: 0000000000103738 0 NOTYPE GLOBAL DEFAULT 11 _end
> >
> > $~ readelf -sW old_v7/u-boot new_v7/u-boot | grep -w _end
> > 10638: 000da824 0 OBJECT GLOBAL DEFAULT 10 _end
> > 10637: 000da84c 0 NOTYPE GLOBAL DEFAULT 10 _end
> >
> > - For both QEMU instances
> > $~ bloat-o-meter old/u-boot new/u-boot
> > add/remove: 0/0 grow/shrink: 1/0 up/down: 20/0 (20)
> > Function old new delta
> > version_string 50 70 +20
> > Total: Before=656915, After=656935, chg +0.00%
> >
> > [0] binutils commit 6b3b0ab89663 ("Make linker assigned symbol dynamic only
> > for shared object")
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodi...@linaro.org>
> > ---
> > arch/arm/cpu/arm1136/u-boot-spl.lds | 6 +-----
> > arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds | 6 +-----
> > arch/arm/cpu/armv8/u-boot-spl.lds | 7 +------
> > arch/arm/cpu/u-boot-spl.lds | 6 +-----
> > arch/arm/cpu/u-boot.lds | 6 +-----
> > arch/arm/lib/sections.c | 1 -
> > arch/arm/mach-aspeed/ast2600/u-boot-spl.lds | 6 +-----
> > arch/arm/mach-at91/arm926ejs/u-boot-spl.lds | 7 +------
> > arch/arm/mach-at91/armv7/u-boot-spl.lds | 7 +------
> > arch/arm/mach-omap2/u-boot-spl.lds | 7 +------
> > arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 7 +------
> > arch/arm/mach-zynq/u-boot.lds | 6 +-----
> > board/davinci/da8xxevm/u-boot-spl-da850evm.lds | 7 +------
> > board/samsung/common/exynos-uboot-spl.lds | 6 +-----
> > 14 files changed, 13 insertions(+), 72 deletions(-)
> >
> > diff --git a/arch/arm/cpu/arm1136/u-boot-spl.lds
> > b/arch/arm/cpu/arm1136/u-boot-spl.lds
> > index f83988fd7e6a..b7af29183a97 100644
> > --- a/arch/arm/cpu/arm1136/u-boot-spl.lds
> > +++ b/arch/arm/cpu/arm1136/u-boot-spl.lds
> > @@ -33,11 +33,7 @@ SECTIONS
> > .data : { *(SORT_BY_ALIGNMENT(.data*)) } >.sram
> > . = ALIGN(4);
> > __image_copy_end = .;
> > -
> > - .end :
> > - {
> > - *(.__end)
> > - }
> > + _end = .;
> >
> > .bss :
> > {
> > diff --git a/arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds
> > b/arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds
> > index 7e20448f8108..7c6309246f8b 100644
> > --- a/arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds
> > +++ b/arch/arm/cpu/arm926ejs/mxs/u-boot-spl.lds
> > @@ -49,11 +49,7 @@ SECTIONS
> > __bss_end = .;
> > }
> >
> > - .end :
> > - {
> > - *(.__end)
> > - }
> > -
> > + _end = .;
> > _image_binary_end = .;
> >
> > .dynsym _image_binary_end : { *(.dynsym) }
> > diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds
> > b/arch/arm/cpu/armv8/u-boot-spl.lds
> > index ef8af67e11c3..215cedd69a8c 100644
> > --- a/arch/arm/cpu/armv8/u-boot-spl.lds
> > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds
> > @@ -53,12 +53,7 @@ SECTIONS
> >
> > . = ALIGN(8);
> > __image_copy_end = .;
> > -
> > - .end : {
> > - . = ALIGN(8);
> > - *(.__end)
> > - } >.sram
> > -
> > + _end = .;
> > _image_binary_end = .;
> >
> > .bss : {
> > diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
> > index 9ed62395a9c5..eee463a1b1c6 100644
> > --- a/arch/arm/cpu/u-boot-spl.lds
> > +++ b/arch/arm/cpu/u-boot-spl.lds
> > @@ -53,12 +53,8 @@ SECTIONS
> > __rel_dyn_end = .;
> > }
> >
> > - .end :
> > - {
> > - *(.__end)
> > - }
> > -
> > _image_binary_end = .;
> > + _end = .;
> >
> > .bss __rel_dyn_start (OVERLAY) : {
> > __bss_start = .;
> > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> > index 707b19795f08..2f50087f57a9 100644
> > --- a/arch/arm/cpu/u-boot.lds
> > +++ b/arch/arm/cpu/u-boot.lds
> > @@ -166,11 +166,7 @@ SECTIONS
> > __rel_dyn_end = .;
> > }
> >
> > - .end :
> > - {
> > - *(.__end)
> > - }
> > -
> > + _end = .;
> > _image_binary_end = .;
> >
> > /*
> > diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
> > index db5463b2bbbc..07efabaa7dc8 100644
> > --- a/arch/arm/lib/sections.c
> > +++ b/arch/arm/lib/sections.c
> > @@ -23,4 +23,3 @@ char __secure_start[0] __section(".__secure_start");
> > char __secure_end[0] __section(".__secure_end");
> > char __secure_stack_start[0] __section(".__secure_stack_start");
> > char __secure_stack_end[0] __section(".__secure_stack_end");
> > -char _end[0] __section(".__end");
> > diff --git a/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds
> > b/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds
> > index ada6570d9712..9502a7384b53 100644
> > --- a/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds
> > +++ b/arch/arm/mach-aspeed/ast2600/u-boot-spl.lds
> > @@ -61,11 +61,7 @@ SECTIONS
> > __rel_dyn_end = .;
> > } > .nor
> >
> > - .end :
> > - {
> > - *(.__end)
> > - } > .nor
> > -
> > + _end = .;
> > _image_binary_end = .;
> >
> > .bss : {
> > diff --git a/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds
> > b/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds
> > index 1a8bf94dee0c..09cf838cf96e 100644
> > --- a/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds
> > +++ b/arch/arm/mach-at91/arm926ejs/u-boot-spl.lds
> > @@ -33,12 +33,7 @@ SECTIONS
> >
> > . = ALIGN(4);
> > __image_copy_end = .;
> > -
> > - .end :
> > - {
> > - *(.__end)
> > - } >.sram
> > -
> > + _end = .;
> > _image_binary_end = .;
> >
> > .bss :
> > diff --git a/arch/arm/mach-at91/armv7/u-boot-spl.lds
> > b/arch/arm/mach-at91/armv7/u-boot-spl.lds
> > index 6ca725fc4ce0..460a91d93ec4 100644
> > --- a/arch/arm/mach-at91/armv7/u-boot-spl.lds
> > +++ b/arch/arm/mach-at91/armv7/u-boot-spl.lds
> > @@ -40,12 +40,7 @@ SECTIONS
> >
> > . = ALIGN(4);
> > __image_copy_end = .;
> > -
> > - .end :
> > - {
> > - *(.__end)
> > - } >.sram
> > -
> > + _end = .;
>
> Does this have any kind of consequence in terms of checking the offset of
> .end vs
> the SRAM size ? (.sram)
The value of _end does grow by 16b on the SPL for the kria boards, but
it doesn't seem to cause issues.
I can only find a single assert on those checks in
arch/microblaze/cpu/u-boot-spl.lds. Anything else you spotted?
Keeping in mind that the code used to be like that before that linker
bug -- defining a linker symbol instead of a section, I don't think
anything will blow up
/Ilias
>
> > _image_binary_end = .;
> >
> > .bss :
>
