[PATCH 6/6] arm: move image_copy_start/end to linker symbols
Ilias Apalodimas
ilias.apalodimas at linaro.org
Wed Mar 6 11:37:26 CET 2024
On Wed, 6 Mar 2024 at 11:35, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Sam,
>
>
> On Wed, 6 Mar 2024 at 10:22, Sam Edwards <cfsworks at gmail.com> wrote:
> >
> > On 3/4/24 02:01, Ilias Apalodimas wrote:
> > > image_copy_start/end are defined as c variables in order to force the compiler
> > > emit relative references. However, defining those within a section definition
> > > will do the same thing.
> > >
> > > So let's remove the special sections from the linker scripts, the
> > > variable definitions from sections.c and define them as a symbols within
> > > a section.
> > >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > Tested-by: Sam Edwards <CFSworks at gmail.com> # Binary output identical
> >
> > Since the __image_copy_* symbols are marking boundaries in the whole
> > image as opposed to a single section, I'd find it clearer if they were
> > loose in the SECTIONS { ... } block, so as not to imply that they are
> > coupled to any particular section. Thoughts?
>
> The reason I included it within a section is my understanding of the
> ld (way older) manual [0].
> Copy-pasting from that:
>
> " Assignment statements may appear:
> - as commands in their own right in an ld script; or
> - as independent statements within a SECTIONS command; or
> - as part of the contents of a section definition in a SECTIONS command.
> The first two cases are equivalent in effect--both define a symbol
> with an absolute address. The last case defines a symbol whose address
> is relative to a particular section."
> So, since we need relocatable entries, I included it within a section.
>
> Looking at the latest ld [1] the wording has changed a bit it says
> "Expressions appearing outside an output section definition treat all
> numbers as absolute addresses" and it also has the following example
> SECTIONS
> {
> . = 0x100;
> __executable_start = 0x100;
> .data :
> {
> . = 0x10;
> __data_start = 0x10;
> *(.data)
> }
> …
> }
> both . and __executable_start are set to the absolute address 0x100 in
> the first two assignments, then both . and __data_start are set to
> 0x10 relative to the .data section in the second two assignments.
> So I assume the same behavior persists?
I just tested moving image_binary_end outside the section definition
and it's still emitted properly. I'll try to figure this out and on v3
I'll move both image_copy_start/end outside the sections.
I also noticed that I forgot to change
arch/arm/cpu/armv8/u-boot-spl.lds and get rid of the .image_copy_end.
Thanks
/Ilias
>
> [0] https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_mono/ld.html#SEC13
> [1] https://sourceware.org/binutils/docs/ld/Expression-Section.html
>
> Thanks
> /Ilias
>
>
> >
> > Thanks for the cleanup,
> > Sam
> >
> > > ---
> > > arch/arm/cpu/armv8/u-boot.lds | 10 ++--------
> > > arch/arm/cpu/u-boot.lds | 10 ++--------
> > > arch/arm/lib/sections.c | 2 --
> > > arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 8 ++------
> > > arch/arm/mach-zynq/u-boot.lds | 9 ++-------
> > > 5 files changed, 8 insertions(+), 31 deletions(-)
> > >
> > > diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
> > > index e737de761a9d..340acf5c043b 100644
> > > --- a/arch/arm/cpu/armv8/u-boot.lds
> > > +++ b/arch/arm/cpu/armv8/u-boot.lds
> > > @@ -23,7 +23,7 @@ SECTIONS
> > > . = ALIGN(8);
> > > .text :
> > > {
> > > - *(.__image_copy_start)
> > > + __image_copy_start = .;
> > > CPUDIR/start.o (.text*)
> > > }
> > >
> > > @@ -120,13 +120,7 @@ SECTIONS
> > > *(.rel*.efi_runtime)
> > > *(.rel*.efi_runtime.*)
> > > __efi_runtime_rel_stop = .;
> > > - }
> > > -
> > > - . = ALIGN(8);
> > > -
> > > - .image_copy_end :
> > > - {
> > > - *(.__image_copy_end)
> > > + __image_copy_end = .;
> > > }
> > >
> > > .rela.dyn ALIGN(8) : {
> > > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
> > > index df55bb716e35..7c4f9c2d4dd3 100644
> > > --- a/arch/arm/cpu/u-boot.lds
> > > +++ b/arch/arm/cpu/u-boot.lds
> > > @@ -37,7 +37,7 @@ SECTIONS
> > > . = ALIGN(4);
> > > .text :
> > > {
> > > - *(.__image_copy_start)
> > > + __image_copy_start = .;
> > > *(.vectors)
> > > CPUDIR/start.o (.text*)
> > > }
> > > @@ -151,13 +151,7 @@ SECTIONS
> > > *(.rel*.efi_runtime)
> > > *(.rel*.efi_runtime.*)
> > > __efi_runtime_rel_stop = .;
> > > - }
> > > -
> > > - . = ALIGN(4);
> > > -
> > > - .image_copy_end :
> > > - {
> > > - *(.__image_copy_end)
> > > + __image_copy_end = .;
> > > }
> > >
> > > .rel.dyn ALIGN(4) : {
> > > diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
> > > index a4d4202e99f5..db5463b2bbbc 100644
> > > --- a/arch/arm/lib/sections.c
> > > +++ b/arch/arm/lib/sections.c
> > > @@ -19,8 +19,6 @@
> > > * aliasing warnings.
> > > */
> > >
> > > -char __image_copy_start[0] __section(".__image_copy_start");
> > > -char __image_copy_end[0] __section(".__image_copy_end");
> > > char __secure_start[0] __section(".__secure_start");
> > > char __secure_end[0] __section(".__secure_end");
> > > char __secure_stack_start[0] __section(".__secure_stack_start");
> > > diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
> > > index b7887194026e..4b480ebb0c4d 100644
> > > --- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
> > > +++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds
> > > @@ -24,7 +24,7 @@ SECTIONS
> > >
> > > .text : {
> > > . = ALIGN(8);
> > > - *(.__image_copy_start)
> > > + __image_copy_start = .;
> > > CPUDIR/start.o (.text*)
> > > *(.text*)
> > > }
> > > @@ -42,11 +42,7 @@ SECTIONS
> > > __u_boot_list : {
> > > . = ALIGN(8);
> > > KEEP(*(SORT(__u_boot_list*)));
> > > - }
> > > -
> > > - .image_copy_end : {
> > > - . = ALIGN(8);
> > > - *(.__image_copy_end)
> > > + __image_copy_end = .;
> > > }
> > >
> > > .end : {
> > > diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds
> > > index fcd0f42a7106..553bcf182272 100644
> > > --- a/arch/arm/mach-zynq/u-boot.lds
> > > +++ b/arch/arm/mach-zynq/u-boot.lds
> > > @@ -16,7 +16,7 @@ SECTIONS
> > > . = ALIGN(4);
> > > .text :
> > > {
> > > - *(.__image_copy_start)
> > > + __image_copy_start = .;
> > > *(.vectors)
> > > CPUDIR/start.o (.text*)
> > > }
> > > @@ -57,12 +57,7 @@ SECTIONS
> > > *(.rel*.efi_runtime)
> > > *(.rel*.efi_runtime.*)
> > > __efi_runtime_rel_stop = .;
> > > - }
> > > -
> > > - . = ALIGN(8);
> > > - .image_copy_end :
> > > - {
> > > - *(.__image_copy_end)
> > > + __image_copy_end = .;
> > > }
> > >
> > > .rel.dyn ALIGN(8) : {
More information about the U-Boot
mailing list