| Message ID | 20240304090113.1410575-6-ilias.apalodimas@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Clean up arm linker scripts | expand |
On 3/4/24 02:01, Ilias Apalodimas wrote: > __efi_runtime_start/end are defined as c variables for arm7 only in > order to force the compiler emit relative references. However, defining > those within a section definition will do the same thing. On top of that > the v8 linker scripts define it as a symbol. > > So let's remove the special sections from the linker scripts, the > variable definitions from sections.c and define them as a symbols within > the correct section. > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical Thanks for the cleanup, Sam > --- > arch/arm/cpu/u-boot.lds | 12 +++--------- > arch/arm/lib/sections.c | 2 -- > arch/arm/mach-zynq/u-boot.lds | 12 +++--------- > include/asm-generic/sections.h | 1 + > 4 files changed, 7 insertions(+), 20 deletions(-) > > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > index 7c6e7891d360..df55bb716e35 100644 > --- a/arch/arm/cpu/u-boot.lds > +++ b/arch/arm/cpu/u-boot.lds > @@ -43,18 +43,12 @@ SECTIONS > } > > /* This needs to come before *(.text*) */ > - .__efi_runtime_start : { > - *(.__efi_runtime_start) > - } > - > - .efi_runtime : { > + .efi_runtime ALIGN(4) : { Do we truly require the ALIGN(4)? If I understand correctly, by default, the linker calculates the alignment of an output section as the least common multiple of the input sections' alignment requirements -- meaning most (perhaps all) of our ALIGN()s today are redundant. For the time being, I'm in favor of merging existing `. = ALIGN(x)` into each following section for clarity and to avoid the testing overhead of removing them in the same patch as other changes. However, in the near future (perhaps even as "near" as v2 of this series?), I'd also like to see a patch that eliminates unnecessary ALIGN()s altogether. Introducing additional ALIGN()s where we already know they aren't needed may be a step away from that goal. > + __efi_runtime_start = .; > *(.text.efi_runtime*) > *(.rodata.efi_runtime*) > *(.data.efi_runtime*) > - } > - > - .__efi_runtime_stop : { > - *(.__efi_runtime_stop) > + __efi_runtime_stop = .; > } > > .text_rest : > diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c > index 1ee3dd3667ba..a4d4202e99f5 100644 > --- a/arch/arm/lib/sections.c > +++ b/arch/arm/lib/sections.c > @@ -25,6 +25,4 @@ 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 __efi_runtime_start[0] __section(".__efi_runtime_start"); > -char __efi_runtime_stop[0] __section(".__efi_runtime_stop"); > char _end[0] __section(".__end"); > diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds > index 71dea4a1f60a..fcd0f42a7106 100644 > --- a/arch/arm/mach-zynq/u-boot.lds > +++ b/arch/arm/mach-zynq/u-boot.lds > @@ -22,18 +22,12 @@ SECTIONS > } > > /* This needs to come before *(.text*) */ > - .__efi_runtime_start : { > - *(.__efi_runtime_start) > - } > - > - .efi_runtime : { > + .efi_runtime ALIGN(4) : { Ditto above > + __efi_runtime_start = .; > *(.text.efi_runtime*) > *(.rodata.efi_runtime*) > *(.data.efi_runtime*) > - } > - > - .__efi_runtime_stop : { > - *(.__efi_runtime_stop) > + __efi_runtime_stop = .; > } > > .text_rest : > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > index 60949200dd93..b6bca53db10d 100644 > --- a/include/asm-generic/sections.h > +++ b/include/asm-generic/sections.h > @@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[]; > extern char __ctors_start[], __ctors_end[]; > > extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; > +extern char __efi_runtime_start[], __efi_runtime_stop[]; > > /* function descriptor handling (if any). Override > * in asm/sections.h */
Hi Sam, Again thank you for the elaborate review. This really helps a lot. On Wed, 6 Mar 2024 at 10:14, Sam Edwards <cfsworks@gmail.com> wrote: > > > > On 3/4/24 02:01, Ilias Apalodimas wrote: > > __efi_runtime_start/end are defined as c variables for arm7 only in > > order to force the compiler emit relative references. However, defining > > those within a section definition will do the same thing. On top of that > > the v8 linker scripts define it as a symbol. > > > > So let's remove the special sections from the linker scripts, the > > variable definitions from sections.c and define them as a symbols within > > the correct section. > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical > > Thanks for the cleanup, > Sam > > > --- > > arch/arm/cpu/u-boot.lds | 12 +++--------- > > arch/arm/lib/sections.c | 2 -- > > arch/arm/mach-zynq/u-boot.lds | 12 +++--------- > > include/asm-generic/sections.h | 1 + > > 4 files changed, 7 insertions(+), 20 deletions(-) > > > > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > > index 7c6e7891d360..df55bb716e35 100644 > > --- a/arch/arm/cpu/u-boot.lds > > +++ b/arch/arm/cpu/u-boot.lds > > @@ -43,18 +43,12 @@ SECTIONS > > } > > > > /* This needs to come before *(.text*) */ > > - .__efi_runtime_start : { > > - *(.__efi_runtime_start) > > - } > > - > > - .efi_runtime : { > > + .efi_runtime ALIGN(4) : { > > Do we truly require the ALIGN(4)? If I understand correctly, by default, > the linker calculates the alignment of an output section as the least > common multiple of the input sections' alignment requirements -- meaning > most (perhaps all) of our ALIGN()s today are redundant. I don't think we do. But I preserved those for a few reasons. > For the time > being, I'm in favor of merging existing `. = ALIGN(x)` into each > following section for clarity and to avoid the testing overhead of > removing them in the same patch as other changes. However, in the near > future (perhaps even as "near" as v2 of this series?), I'd also like to > see a patch that eliminates unnecessary ALIGN()s altogether. Introducing > additional ALIGN()s where we already know they aren't needed may be a > step away from that goal. So as you already mentioned the reason I preserved this is: - Reduce the testing overhead by preserving the same layout for now - In the future, I am playing around with the idea of mapping U-Boot (not SPL but the relocated full U-Boot) in sections with proper memory permissions (R), RW^X etc). In that case, we will need a 4k section alignment and we can repurpose the ALIGN(4/8) to ALIGN(CONSTANT(COMMONPAGESIZE)). Thoughts? Thanks /Ilias > > > + __efi_runtime_start = .; > > *(.text.efi_runtime*) > > *(.rodata.efi_runtime*) > > *(.data.efi_runtime*) > > - } > > - > > - .__efi_runtime_stop : { > > - *(.__efi_runtime_stop) > > + __efi_runtime_stop = .; > > } > > > > .text_rest : > > diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c > > index 1ee3dd3667ba..a4d4202e99f5 100644 > > --- a/arch/arm/lib/sections.c > > +++ b/arch/arm/lib/sections.c > > @@ -25,6 +25,4 @@ 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 __efi_runtime_start[0] __section(".__efi_runtime_start"); > > -char __efi_runtime_stop[0] __section(".__efi_runtime_stop"); > > char _end[0] __section(".__end"); > > diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds > > index 71dea4a1f60a..fcd0f42a7106 100644 > > --- a/arch/arm/mach-zynq/u-boot.lds > > +++ b/arch/arm/mach-zynq/u-boot.lds > > @@ -22,18 +22,12 @@ SECTIONS > > } > > > > /* This needs to come before *(.text*) */ > > - .__efi_runtime_start : { > > - *(.__efi_runtime_start) > > - } > > - > > - .efi_runtime : { > > + .efi_runtime ALIGN(4) : { > > Ditto above > > > + __efi_runtime_start = .; > > *(.text.efi_runtime*) > > *(.rodata.efi_runtime*) > > *(.data.efi_runtime*) > > - } > > - > > - .__efi_runtime_stop : { > > - *(.__efi_runtime_stop) > > + __efi_runtime_stop = .; > > } > > > > .text_rest : > > diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > > index 60949200dd93..b6bca53db10d 100644 > > --- a/include/asm-generic/sections.h > > +++ b/include/asm-generic/sections.h > > @@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[]; > > extern char __ctors_start[], __ctors_end[]; > > > > extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; > > +extern char __efi_runtime_start[], __efi_runtime_stop[]; > > > > /* function descriptor handling (if any). Override > > * in asm/sections.h */
On 3/6/24 02:13, Ilias Apalodimas wrote: > Hi Sam, > > Again thank you for the elaborate review. This really helps a lot. > > On Wed, 6 Mar 2024 at 10:14, Sam Edwards <cfsworks@gmail.com> wrote: >> >> >> >> On 3/4/24 02:01, Ilias Apalodimas wrote: >>> __efi_runtime_start/end are defined as c variables for arm7 only in >>> order to force the compiler emit relative references. However, defining >>> those within a section definition will do the same thing. On top of that >>> the v8 linker scripts define it as a symbol. >>> >>> So let's remove the special sections from the linker scripts, the >>> variable definitions from sections.c and define them as a symbols within >>> the correct section. >>> >>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> >> Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical >> >> Thanks for the cleanup, >> Sam >> >>> --- >>> arch/arm/cpu/u-boot.lds | 12 +++--------- >>> arch/arm/lib/sections.c | 2 -- >>> arch/arm/mach-zynq/u-boot.lds | 12 +++--------- >>> include/asm-generic/sections.h | 1 + >>> 4 files changed, 7 insertions(+), 20 deletions(-) >>> >>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds >>> index 7c6e7891d360..df55bb716e35 100644 >>> --- a/arch/arm/cpu/u-boot.lds >>> +++ b/arch/arm/cpu/u-boot.lds >>> @@ -43,18 +43,12 @@ SECTIONS >>> } >>> >>> /* This needs to come before *(.text*) */ >>> - .__efi_runtime_start : { >>> - *(.__efi_runtime_start) >>> - } >>> - >>> - .efi_runtime : { >>> + .efi_runtime ALIGN(4) : { >> >> Do we truly require the ALIGN(4)? If I understand correctly, by default, >> the linker calculates the alignment of an output section as the least >> common multiple of the input sections' alignment requirements -- meaning >> most (perhaps all) of our ALIGN()s today are redundant. > > I don't think we do. But I preserved those for a few reasons. > >> For the time >> being, I'm in favor of merging existing `. = ALIGN(x)` into each >> following section for clarity and to avoid the testing overhead of >> removing them in the same patch as other changes. However, in the near >> future (perhaps even as "near" as v2 of this series?), I'd also like to >> see a patch that eliminates unnecessary ALIGN()s altogether. Introducing >> additional ALIGN()s where we already know they aren't needed may be a >> step away from that goal. > > So as you already mentioned the reason I preserved this is: > - Reduce the testing overhead by preserving the same layout for now > - In the future, I am playing around with the idea of mapping U-Boot > (not SPL but the relocated full U-Boot) in sections with proper memory > permissions (R), RW^X etc). In that case, we will need a 4k section > alignment and we can repurpose the ALIGN(4/8) to > ALIGN(CONSTANT(COMMONPAGESIZE)). > > Thoughts? Ah, right: I had forgotten for a moment that the ultimate goal was to clean up the memory protection bit situation. Okay, as long as that future cleanup will also remove all unnecessary uses of ALIGN() (i.e. any that don't result in an actual change in memory layout) then that sounds great to me. Reviewed-by: Sam Edwards <CFSworks@gmail.com> Cheers, Sam > > Thanks > /Ilias > >> >>> + __efi_runtime_start = .; >>> *(.text.efi_runtime*) >>> *(.rodata.efi_runtime*) >>> *(.data.efi_runtime*) >>> - } >>> - >>> - .__efi_runtime_stop : { >>> - *(.__efi_runtime_stop) >>> + __efi_runtime_stop = .; >>> } >>> >>> .text_rest : >>> diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c >>> index 1ee3dd3667ba..a4d4202e99f5 100644 >>> --- a/arch/arm/lib/sections.c >>> +++ b/arch/arm/lib/sections.c >>> @@ -25,6 +25,4 @@ 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 __efi_runtime_start[0] __section(".__efi_runtime_start"); >>> -char __efi_runtime_stop[0] __section(".__efi_runtime_stop"); >>> char _end[0] __section(".__end"); >>> diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds >>> index 71dea4a1f60a..fcd0f42a7106 100644 >>> --- a/arch/arm/mach-zynq/u-boot.lds >>> +++ b/arch/arm/mach-zynq/u-boot.lds >>> @@ -22,18 +22,12 @@ SECTIONS >>> } >>> >>> /* This needs to come before *(.text*) */ >>> - .__efi_runtime_start : { >>> - *(.__efi_runtime_start) >>> - } >>> - >>> - .efi_runtime : { >>> + .efi_runtime ALIGN(4) : { >> >> Ditto above >> >>> + __efi_runtime_start = .; >>> *(.text.efi_runtime*) >>> *(.rodata.efi_runtime*) >>> *(.data.efi_runtime*) >>> - } >>> - >>> - .__efi_runtime_stop : { >>> - *(.__efi_runtime_stop) >>> + __efi_runtime_stop = .; >>> } >>> >>> .text_rest : >>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h >>> index 60949200dd93..b6bca53db10d 100644 >>> --- a/include/asm-generic/sections.h >>> +++ b/include/asm-generic/sections.h >>> @@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[]; >>> extern char __ctors_start[], __ctors_end[]; >>> >>> extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; >>> +extern char __efi_runtime_start[], __efi_runtime_stop[]; >>> >>> /* function descriptor handling (if any). Override >>> * in asm/sections.h */
On Thu, 7 Mar 2024 at 00:19, Sam Edwards <cfsworks@gmail.com> wrote: > > > On 3/6/24 02:13, Ilias Apalodimas wrote: > > Hi Sam, > > > > Again thank you for the elaborate review. This really helps a lot. > > > > On Wed, 6 Mar 2024 at 10:14, Sam Edwards <cfsworks@gmail.com> wrote: > >> > >> > >> > >> On 3/4/24 02:01, Ilias Apalodimas wrote: > >>> __efi_runtime_start/end are defined as c variables for arm7 only in > >>> order to force the compiler emit relative references. However, defining > >>> those within a section definition will do the same thing. On top of > that > >>> the v8 linker scripts define it as a symbol. > >>> > >>> So let's remove the special sections from the linker scripts, the > >>> variable definitions from sections.c and define them as a symbols > within > >>> the correct section. > >>> > >>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > >> Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical > >> > >> Thanks for the cleanup, > >> Sam > >> > >>> --- > >>> arch/arm/cpu/u-boot.lds | 12 +++--------- > >>> arch/arm/lib/sections.c | 2 -- > >>> arch/arm/mach-zynq/u-boot.lds | 12 +++--------- > >>> include/asm-generic/sections.h | 1 + > >>> 4 files changed, 7 insertions(+), 20 deletions(-) > >>> > >>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > >>> index 7c6e7891d360..df55bb716e35 100644 > >>> --- a/arch/arm/cpu/u-boot.lds > >>> +++ b/arch/arm/cpu/u-boot.lds > >>> @@ -43,18 +43,12 @@ SECTIONS > >>> } > >>> > >>> /* This needs to come before *(.text*) */ > >>> - .__efi_runtime_start : { > >>> - *(.__efi_runtime_start) > >>> - } > >>> - > >>> - .efi_runtime : { > >>> + .efi_runtime ALIGN(4) : { > >> > >> Do we truly require the ALIGN(4)? If I understand correctly, by default, > >> the linker calculates the alignment of an output section as the least > >> common multiple of the input sections' alignment requirements -- meaning > >> most (perhaps all) of our ALIGN()s today are redundant. > > > > I don't think we do. But I preserved those for a few reasons. > > > >> For the time > >> being, I'm in favor of merging existing `. = ALIGN(x)` into each > >> following section for clarity and to avoid the testing overhead of > >> removing them in the same patch as other changes. However, in the near > >> future (perhaps even as "near" as v2 of this series?), I'd also like to > >> see a patch that eliminates unnecessary ALIGN()s altogether. Introducing > >> additional ALIGN()s where we already know they aren't needed may be a > >> step away from that goal. > > > > So as you already mentioned the reason I preserved this is: > > - Reduce the testing overhead by preserving the same layout for now > > - In the future, I am playing around with the idea of mapping U-Boot > > (not SPL but the relocated full U-Boot) in sections with proper memory > > permissions (R), RW^X etc). In that case, we will need a 4k section > > alignment and we can repurpose the ALIGN(4/8) to > > ALIGN(CONSTANT(COMMONPAGESIZE)). > > > > Thoughts? > > Ah, right: I had forgotten for a moment that the ultimate goal was to > clean up the memory protection bit situation. > > Okay, as long as that future cleanup will also remove all unnecessary > uses of ALIGN() (i.e. any that don't result in an actual change in > memory layout) then that sounds great to me. > Yep will do > Reviewed-by: Sam Edwards <CFSworks@gmail.com> > Thanks! > Cheers, > Sam > > > > > Thanks > > /Ilias > > > >> > >>> + __efi_runtime_start = .; > >>> *(.text.efi_runtime*) > >>> *(.rodata.efi_runtime*) > >>> *(.data.efi_runtime*) > >>> - } > >>> - > >>> - .__efi_runtime_stop : { > >>> - *(.__efi_runtime_stop) > >>> + __efi_runtime_stop = .; > >>> } > >>> > >>> .text_rest : > >>> diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c > >>> index 1ee3dd3667ba..a4d4202e99f5 100644 > >>> --- a/arch/arm/lib/sections.c > >>> +++ b/arch/arm/lib/sections.c > >>> @@ -25,6 +25,4 @@ 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 __efi_runtime_start[0] __section(".__efi_runtime_start"); > >>> -char __efi_runtime_stop[0] __section(".__efi_runtime_stop"); > >>> char _end[0] __section(".__end"); > >>> diff --git a/arch/arm/mach-zynq/u-boot.lds > b/arch/arm/mach-zynq/u-boot.lds > >>> index 71dea4a1f60a..fcd0f42a7106 100644 > >>> --- a/arch/arm/mach-zynq/u-boot.lds > >>> +++ b/arch/arm/mach-zynq/u-boot.lds > >>> @@ -22,18 +22,12 @@ SECTIONS > >>> } > >>> > >>> /* This needs to come before *(.text*) */ > >>> - .__efi_runtime_start : { > >>> - *(.__efi_runtime_start) > >>> - } > >>> - > >>> - .efi_runtime : { > >>> + .efi_runtime ALIGN(4) : { > >> > >> Ditto above > >> > >>> + __efi_runtime_start = .; > >>> *(.text.efi_runtime*) > >>> *(.rodata.efi_runtime*) > >>> *(.data.efi_runtime*) > >>> - } > >>> - > >>> - .__efi_runtime_stop : { > >>> - *(.__efi_runtime_stop) > >>> + __efi_runtime_stop = .; > >>> } > >>> > >>> .text_rest : > >>> diff --git a/include/asm-generic/sections.h > b/include/asm-generic/sections.h > >>> index 60949200dd93..b6bca53db10d 100644 > >>> --- a/include/asm-generic/sections.h > >>> +++ b/include/asm-generic/sections.h > >>> @@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[]; > >>> extern char __ctors_start[], __ctors_end[]; > >>> > >>> extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; > >>> +extern char __efi_runtime_start[], __efi_runtime_stop[]; > >>> > >>> /* function descriptor handling (if any). Override > >>> * in asm/sections.h */ >
Hi Sam, On Thu, 7 Mar 2024 at 08:50, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > > > On Thu, 7 Mar 2024 at 00:19, Sam Edwards <cfsworks@gmail.com> wrote: >> >> >> >> On 3/6/24 02:13, Ilias Apalodimas wrote: >> > Hi Sam, >> > >> > Again thank you for the elaborate review. This really helps a lot. >> > >> > On Wed, 6 Mar 2024 at 10:14, Sam Edwards <cfsworks@gmail.com> wrote: >> >> >> >> >> >> >> >> On 3/4/24 02:01, Ilias Apalodimas wrote: >> >>> __efi_runtime_start/end are defined as c variables for arm7 only in >> >>> order to force the compiler emit relative references. However, defining >> >>> those within a section definition will do the same thing. On top of that >> >>> the v8 linker scripts define it as a symbol. >> >>> >> >>> So let's remove the special sections from the linker scripts, the >> >>> variable definitions from sections.c and define them as a symbols within >> >>> the correct section. >> >>> >> >>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> >> >> Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical >> >> >> >> Thanks for the cleanup, >> >> Sam >> >> >> >>> --- >> >>> arch/arm/cpu/u-boot.lds | 12 +++--------- >> >>> arch/arm/lib/sections.c | 2 -- >> >>> arch/arm/mach-zynq/u-boot.lds | 12 +++--------- >> >>> include/asm-generic/sections.h | 1 + >> >>> 4 files changed, 7 insertions(+), 20 deletions(-) >> >>> >> >>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds >> >>> index 7c6e7891d360..df55bb716e35 100644 >> >>> --- a/arch/arm/cpu/u-boot.lds >> >>> +++ b/arch/arm/cpu/u-boot.lds >> >>> @@ -43,18 +43,12 @@ SECTIONS >> >>> } >> >>> >> >>> /* This needs to come before *(.text*) */ >> >>> - .__efi_runtime_start : { >> >>> - *(.__efi_runtime_start) >> >>> - } >> >>> - >> >>> - .efi_runtime : { >> >>> + .efi_runtime ALIGN(4) : { >> >> >> >> Do we truly require the ALIGN(4)? If I understand correctly, by default, >> >> the linker calculates the alignment of an output section as the least >> >> common multiple of the input sections' alignment requirements -- meaning >> >> most (perhaps all) of our ALIGN()s today are redundant. >> > >> > I don't think we do. But I preserved those for a few reasons. >> > >> >> For the time >> >> being, I'm in favor of merging existing `. = ALIGN(x)` into each >> >> following section for clarity and to avoid the testing overhead of >> >> removing them in the same patch as other changes. However, in the near >> >> future (perhaps even as "near" as v2 of this series?), I'd also like to >> >> see a patch that eliminates unnecessary ALIGN()s altogether. Introducing >> >> additional ALIGN()s where we already know they aren't needed may be a >> >> step away from that goal. >> > >> > So as you already mentioned the reason I preserved this is: >> > - Reduce the testing overhead by preserving the same layout for now >> > - In the future, I am playing around with the idea of mapping U-Boot >> > (not SPL but the relocated full U-Boot) in sections with proper memory >> > permissions (R), RW^X etc). In that case, we will need a 4k section >> > alignment and we can repurpose the ALIGN(4/8) to >> > ALIGN(CONSTANT(COMMONPAGESIZE)). >> > >> > Thoughts? >> >> Ah, right: I had forgotten for a moment that the ultimate goal was to >> clean up the memory protection bit situation. >> >> Okay, as long as that future cleanup will also remove all unnecessary >> uses of ALIGN() (i.e. any that don't result in an actual change in >> memory layout) then that sounds great to me. > > > Yep will do > >> >> Reviewed-by: Sam Edwards <CFSworks@gmail.com> Thanks. However, I noticed the alignment rule was wrong it needs to be '.efi_runtime : ALIGN(4)' instead of '.efi_runtime ALIGN(4) :'. The latter will define the start address instead of specifying the alignment. Due to what you mentioned above this (that we don't really need the ALIGN(4) the resulting binary is unaffected. Thanks /Ilias > > > > > Thanks! > >> >> Cheers, >> Sam >> >> > >> > Thanks >> > /Ilias >> > >> >> >> >>> + __efi_runtime_start = .; >> >>> *(.text.efi_runtime*) >> >>> *(.rodata.efi_runtime*) >> >>> *(.data.efi_runtime*) >> >>> - } >> >>> - >> >>> - .__efi_runtime_stop : { >> >>> - *(.__efi_runtime_stop) >> >>> + __efi_runtime_stop = .; >> >>> } >> >>> >> >>> .text_rest : >> >>> diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c >> >>> index 1ee3dd3667ba..a4d4202e99f5 100644 >> >>> --- a/arch/arm/lib/sections.c >> >>> +++ b/arch/arm/lib/sections.c >> >>> @@ -25,6 +25,4 @@ 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 __efi_runtime_start[0] __section(".__efi_runtime_start"); >> >>> -char __efi_runtime_stop[0] __section(".__efi_runtime_stop"); >> >>> char _end[0] __section(".__end"); >> >>> diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds >> >>> index 71dea4a1f60a..fcd0f42a7106 100644 >> >>> --- a/arch/arm/mach-zynq/u-boot.lds >> >>> +++ b/arch/arm/mach-zynq/u-boot.lds >> >>> @@ -22,18 +22,12 @@ SECTIONS >> >>> } >> >>> >> >>> /* This needs to come before *(.text*) */ >> >>> - .__efi_runtime_start : { >> >>> - *(.__efi_runtime_start) >> >>> - } >> >>> - >> >>> - .efi_runtime : { >> >>> + .efi_runtime ALIGN(4) : { >> >> >> >> Ditto above >> >> >> >>> + __efi_runtime_start = .; >> >>> *(.text.efi_runtime*) >> >>> *(.rodata.efi_runtime*) >> >>> *(.data.efi_runtime*) >> >>> - } >> >>> - >> >>> - .__efi_runtime_stop : { >> >>> - *(.__efi_runtime_stop) >> >>> + __efi_runtime_stop = .; >> >>> } >> >>> >> >>> .text_rest : >> >>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h >> >>> index 60949200dd93..b6bca53db10d 100644 >> >>> --- a/include/asm-generic/sections.h >> >>> +++ b/include/asm-generic/sections.h >> >>> @@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[]; >> >>> extern char __ctors_start[], __ctors_end[]; >> >>> >> >>> extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; >> >>> +extern char __efi_runtime_start[], __efi_runtime_stop[]; >> >>> >> >>> /* function descriptor handling (if any). Override >> >>> * in asm/sections.h */
On Fri, 8 Mar 2024 at 15:22, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > Hi Sam, > > On Thu, 7 Mar 2024 at 08:50, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > On Thu, 7 Mar 2024 at 00:19, Sam Edwards <cfsworks@gmail.com> wrote: > >> > >> > >> > >> On 3/6/24 02:13, Ilias Apalodimas wrote: > >> > Hi Sam, > >> > > >> > Again thank you for the elaborate review. This really helps a lot. > >> > > >> > On Wed, 6 Mar 2024 at 10:14, Sam Edwards <cfsworks@gmail.com> wrote: > >> >> > >> >> > >> >> > >> >> On 3/4/24 02:01, Ilias Apalodimas wrote: > >> >>> __efi_runtime_start/end are defined as c variables for arm7 only in > >> >>> order to force the compiler emit relative references. However, defining > >> >>> those within a section definition will do the same thing. On top of that > >> >>> the v8 linker scripts define it as a symbol. > >> >>> > >> >>> So let's remove the special sections from the linker scripts, the > >> >>> variable definitions from sections.c and define them as a symbols within > >> >>> the correct section. > >> >>> > >> >>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > >> >> Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical > >> >> > >> >> Thanks for the cleanup, > >> >> Sam > >> >> > >> >>> --- > >> >>> arch/arm/cpu/u-boot.lds | 12 +++--------- > >> >>> arch/arm/lib/sections.c | 2 -- > >> >>> arch/arm/mach-zynq/u-boot.lds | 12 +++--------- > >> >>> include/asm-generic/sections.h | 1 + > >> >>> 4 files changed, 7 insertions(+), 20 deletions(-) > >> >>> > >> >>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > >> >>> index 7c6e7891d360..df55bb716e35 100644 > >> >>> --- a/arch/arm/cpu/u-boot.lds > >> >>> +++ b/arch/arm/cpu/u-boot.lds > >> >>> @@ -43,18 +43,12 @@ SECTIONS > >> >>> } > >> >>> > >> >>> /* This needs to come before *(.text*) */ > >> >>> - .__efi_runtime_start : { > >> >>> - *(.__efi_runtime_start) > >> >>> - } > >> >>> - > >> >>> - .efi_runtime : { > >> >>> + .efi_runtime ALIGN(4) : { > >> >> > >> >> Do we truly require the ALIGN(4)? If I understand correctly, by default, > >> >> the linker calculates the alignment of an output section as the least > >> >> common multiple of the input sections' alignment requirements -- meaning > >> >> most (perhaps all) of our ALIGN()s today are redundant. > >> > > >> > I don't think we do. But I preserved those for a few reasons. > >> > > >> >> For the time > >> >> being, I'm in favor of merging existing `. = ALIGN(x)` into each > >> >> following section for clarity and to avoid the testing overhead of > >> >> removing them in the same patch as other changes. However, in the near > >> >> future (perhaps even as "near" as v2 of this series?), I'd also like to > >> >> see a patch that eliminates unnecessary ALIGN()s altogether. Introducing > >> >> additional ALIGN()s where we already know they aren't needed may be a > >> >> step away from that goal. > >> > > >> > So as you already mentioned the reason I preserved this is: > >> > - Reduce the testing overhead by preserving the same layout for now > >> > - In the future, I am playing around with the idea of mapping U-Boot > >> > (not SPL but the relocated full U-Boot) in sections with proper memory > >> > permissions (R), RW^X etc). In that case, we will need a 4k section > >> > alignment and we can repurpose the ALIGN(4/8) to > >> > ALIGN(CONSTANT(COMMONPAGESIZE)). > >> > > >> > Thoughts? > >> > >> Ah, right: I had forgotten for a moment that the ultimate goal was to > >> clean up the memory protection bit situation. > >> > >> Okay, as long as that future cleanup will also remove all unnecessary > >> uses of ALIGN() (i.e. any that don't result in an actual change in > >> memory layout) then that sounds great to me. > > > > > > Yep will do > > > >> > >> Reviewed-by: Sam Edwards <CFSworks@gmail.com> > > Thanks. However, I noticed the alignment rule was wrong > it needs to be '.efi_runtime : ALIGN(4)' instead of '.efi_runtime ALIGN(4) :'. > The latter will define the start address instead of specifying the alignment. > Due to what you mentioned above this (that we don't really need the > ALIGN(4) the resulting binary is unaffected. To be exact, it will work because ALIGN returns the current location counter aligned upward to the specified value. > > Thanks > /Ilias > > > > > > > > > > > Thanks! > > > >> > >> Cheers, > >> Sam > >> > >> > > >> > Thanks > >> > /Ilias > >> > > >> >> > >> >>> + __efi_runtime_start = .; > >> >>> *(.text.efi_runtime*) > >> >>> *(.rodata.efi_runtime*) > >> >>> *(.data.efi_runtime*) > >> >>> - } > >> >>> - > >> >>> - .__efi_runtime_stop : { > >> >>> - *(.__efi_runtime_stop) > >> >>> + __efi_runtime_stop = .; > >> >>> } > >> >>> > >> >>> .text_rest : > >> >>> diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c > >> >>> index 1ee3dd3667ba..a4d4202e99f5 100644 > >> >>> --- a/arch/arm/lib/sections.c > >> >>> +++ b/arch/arm/lib/sections.c > >> >>> @@ -25,6 +25,4 @@ 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 __efi_runtime_start[0] __section(".__efi_runtime_start"); > >> >>> -char __efi_runtime_stop[0] __section(".__efi_runtime_stop"); > >> >>> char _end[0] __section(".__end"); > >> >>> diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds > >> >>> index 71dea4a1f60a..fcd0f42a7106 100644 > >> >>> --- a/arch/arm/mach-zynq/u-boot.lds > >> >>> +++ b/arch/arm/mach-zynq/u-boot.lds > >> >>> @@ -22,18 +22,12 @@ SECTIONS > >> >>> } > >> >>> > >> >>> /* This needs to come before *(.text*) */ > >> >>> - .__efi_runtime_start : { > >> >>> - *(.__efi_runtime_start) > >> >>> - } > >> >>> - > >> >>> - .efi_runtime : { > >> >>> + .efi_runtime ALIGN(4) : { > >> >> > >> >> Ditto above > >> >> > >> >>> + __efi_runtime_start = .; > >> >>> *(.text.efi_runtime*) > >> >>> *(.rodata.efi_runtime*) > >> >>> *(.data.efi_runtime*) > >> >>> - } > >> >>> - > >> >>> - .__efi_runtime_stop : { > >> >>> - *(.__efi_runtime_stop) > >> >>> + __efi_runtime_stop = .; > >> >>> } > >> >>> > >> >>> .text_rest : > >> >>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > >> >>> index 60949200dd93..b6bca53db10d 100644 > >> >>> --- a/include/asm-generic/sections.h > >> >>> +++ b/include/asm-generic/sections.h > >> >>> @@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[]; > >> >>> extern char __ctors_start[], __ctors_end[]; > >> >>> > >> >>> extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; > >> >>> +extern char __efi_runtime_start[], __efi_runtime_stop[]; > >> >>> > >> >>> /* function descriptor handling (if any). Override > >> >>> * in asm/sections.h */
On Fri, 8 Mar 2024 at 16:14, Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: > > On Fri, 8 Mar 2024 at 15:22, Ilias Apalodimas > <ilias.apalodimas@linaro.org> wrote: > > > > Hi Sam, > > > > On Thu, 7 Mar 2024 at 08:50, Ilias Apalodimas > > <ilias.apalodimas@linaro.org> wrote: > > > > > > > > > > > > On Thu, 7 Mar 2024 at 00:19, Sam Edwards <cfsworks@gmail.com> wrote: > > >> > > >> > > >> > > >> On 3/6/24 02:13, Ilias Apalodimas wrote: > > >> > Hi Sam, > > >> > > > >> > Again thank you for the elaborate review. This really helps a lot. > > >> > > > >> > On Wed, 6 Mar 2024 at 10:14, Sam Edwards <cfsworks@gmail.com> wrote: > > >> >> > > >> >> > > >> >> > > >> >> On 3/4/24 02:01, Ilias Apalodimas wrote: > > >> >>> __efi_runtime_start/end are defined as c variables for arm7 only in > > >> >>> order to force the compiler emit relative references. However, defining > > >> >>> those within a section definition will do the same thing. On top of that > > >> >>> the v8 linker scripts define it as a symbol. > > >> >>> > > >> >>> So let's remove the special sections from the linker scripts, the > > >> >>> variable definitions from sections.c and define them as a symbols within > > >> >>> the correct section. > > >> >>> > > >> >>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > >> >> Tested-by: Sam Edwards <CFSworks@gmail.com> # Binary output identical > > >> >> > > >> >> Thanks for the cleanup, > > >> >> Sam > > >> >> > > >> >>> --- > > >> >>> arch/arm/cpu/u-boot.lds | 12 +++--------- > > >> >>> arch/arm/lib/sections.c | 2 -- > > >> >>> arch/arm/mach-zynq/u-boot.lds | 12 +++--------- > > >> >>> include/asm-generic/sections.h | 1 + > > >> >>> 4 files changed, 7 insertions(+), 20 deletions(-) > > >> >>> > > >> >>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > > >> >>> index 7c6e7891d360..df55bb716e35 100644 > > >> >>> --- a/arch/arm/cpu/u-boot.lds > > >> >>> +++ b/arch/arm/cpu/u-boot.lds > > >> >>> @@ -43,18 +43,12 @@ SECTIONS > > >> >>> } > > >> >>> > > >> >>> /* This needs to come before *(.text*) */ > > >> >>> - .__efi_runtime_start : { > > >> >>> - *(.__efi_runtime_start) > > >> >>> - } > > >> >>> - > > >> >>> - .efi_runtime : { > > >> >>> + .efi_runtime ALIGN(4) : { > > >> >> > > >> >> Do we truly require the ALIGN(4)? If I understand correctly, by default, > > >> >> the linker calculates the alignment of an output section as the least > > >> >> common multiple of the input sections' alignment requirements -- meaning > > >> >> most (perhaps all) of our ALIGN()s today are redundant. > > >> > > > >> > I don't think we do. But I preserved those for a few reasons. > > >> > > > >> >> For the time > > >> >> being, I'm in favor of merging existing `. = ALIGN(x)` into each > > >> >> following section for clarity and to avoid the testing overhead of > > >> >> removing them in the same patch as other changes. However, in the near > > >> >> future (perhaps even as "near" as v2 of this series?), I'd also like to > > >> >> see a patch that eliminates unnecessary ALIGN()s altogether. Introducing > > >> >> additional ALIGN()s where we already know they aren't needed may be a > > >> >> step away from that goal. > > >> > > > >> > So as you already mentioned the reason I preserved this is: > > >> > - Reduce the testing overhead by preserving the same layout for now > > >> > - In the future, I am playing around with the idea of mapping U-Boot > > >> > (not SPL but the relocated full U-Boot) in sections with proper memory > > >> > permissions (R), RW^X etc). In that case, we will need a 4k section > > >> > alignment and we can repurpose the ALIGN(4/8) to > > >> > ALIGN(CONSTANT(COMMONPAGESIZE)). > > >> > > > >> > Thoughts? > > >> > > >> Ah, right: I had forgotten for a moment that the ultimate goal was to > > >> clean up the memory protection bit situation. > > >> > > >> Okay, as long as that future cleanup will also remove all unnecessary > > >> uses of ALIGN() (i.e. any that don't result in an actual change in > > >> memory layout) then that sounds great to me. > > > > > > > > > Yep will do > > > > > >> > > >> Reviewed-by: Sam Edwards <CFSworks@gmail.com> > > > > Thanks. However, I noticed the alignment rule was wrong > > it needs to be '.efi_runtime : ALIGN(4)' instead of '.efi_runtime ALIGN(4) :'. > > The latter will define the start address instead of specifying the alignment. > > Due to what you mentioned above this (that we don't really need the > > ALIGN(4) the resulting binary is unaffected. > > To be exact, it will work because ALIGN returns the current location > counter aligned upward to the specified value. But thinking about it a bit more, I'll just get rid of the redundant ALIGN as you suggested. I don't think it makes sense to keep or force the output section alignment just because in the future we want to fix page permissions. We can always re-introduce it when stricter (4k) alignment is required for that Cheers /Ilias > > > > > > Thanks > > /Ilias > > > > > > > > > > > > > > > > > Thanks! > > > > > >> > > >> Cheers, > > >> Sam > > >> > > >> > > > >> > Thanks > > >> > /Ilias > > >> > > > >> >> > > >> >>> + __efi_runtime_start = .; > > >> >>> *(.text.efi_runtime*) > > >> >>> *(.rodata.efi_runtime*) > > >> >>> *(.data.efi_runtime*) > > >> >>> - } > > >> >>> - > > >> >>> - .__efi_runtime_stop : { > > >> >>> - *(.__efi_runtime_stop) > > >> >>> + __efi_runtime_stop = .; > > >> >>> } > > >> >>> > > >> >>> .text_rest : > > >> >>> diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c > > >> >>> index 1ee3dd3667ba..a4d4202e99f5 100644 > > >> >>> --- a/arch/arm/lib/sections.c > > >> >>> +++ b/arch/arm/lib/sections.c > > >> >>> @@ -25,6 +25,4 @@ 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 __efi_runtime_start[0] __section(".__efi_runtime_start"); > > >> >>> -char __efi_runtime_stop[0] __section(".__efi_runtime_stop"); > > >> >>> char _end[0] __section(".__end"); > > >> >>> diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds > > >> >>> index 71dea4a1f60a..fcd0f42a7106 100644 > > >> >>> --- a/arch/arm/mach-zynq/u-boot.lds > > >> >>> +++ b/arch/arm/mach-zynq/u-boot.lds > > >> >>> @@ -22,18 +22,12 @@ SECTIONS > > >> >>> } > > >> >>> > > >> >>> /* This needs to come before *(.text*) */ > > >> >>> - .__efi_runtime_start : { > > >> >>> - *(.__efi_runtime_start) > > >> >>> - } > > >> >>> - > > >> >>> - .efi_runtime : { > > >> >>> + .efi_runtime ALIGN(4) : { > > >> >> > > >> >> Ditto above > > >> >> > > >> >>> + __efi_runtime_start = .; > > >> >>> *(.text.efi_runtime*) > > >> >>> *(.rodata.efi_runtime*) > > >> >>> *(.data.efi_runtime*) > > >> >>> - } > > >> >>> - > > >> >>> - .__efi_runtime_stop : { > > >> >>> - *(.__efi_runtime_stop) > > >> >>> + __efi_runtime_stop = .; > > >> >>> } > > >> >>> > > >> >>> .text_rest : > > >> >>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h > > >> >>> index 60949200dd93..b6bca53db10d 100644 > > >> >>> --- a/include/asm-generic/sections.h > > >> >>> +++ b/include/asm-generic/sections.h > > >> >>> @@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[]; > > >> >>> extern char __ctors_start[], __ctors_end[]; > > >> >>> > > >> >>> extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; > > >> >>> +extern char __efi_runtime_start[], __efi_runtime_stop[]; > > >> >>> > > >> >>> /* function descriptor handling (if any). Override > > >> >>> * in asm/sections.h */
diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 7c6e7891d360..df55bb716e35 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -43,18 +43,12 @@ SECTIONS } /* This needs to come before *(.text*) */ - .__efi_runtime_start : { - *(.__efi_runtime_start) - } - - .efi_runtime : { + .efi_runtime ALIGN(4) : { + __efi_runtime_start = .; *(.text.efi_runtime*) *(.rodata.efi_runtime*) *(.data.efi_runtime*) - } - - .__efi_runtime_stop : { - *(.__efi_runtime_stop) + __efi_runtime_stop = .; } .text_rest : diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c index 1ee3dd3667ba..a4d4202e99f5 100644 --- a/arch/arm/lib/sections.c +++ b/arch/arm/lib/sections.c @@ -25,6 +25,4 @@ 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 __efi_runtime_start[0] __section(".__efi_runtime_start"); -char __efi_runtime_stop[0] __section(".__efi_runtime_stop"); char _end[0] __section(".__end"); diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds index 71dea4a1f60a..fcd0f42a7106 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -22,18 +22,12 @@ SECTIONS } /* This needs to come before *(.text*) */ - .__efi_runtime_start : { - *(.__efi_runtime_start) - } - - .efi_runtime : { + .efi_runtime ALIGN(4) : { + __efi_runtime_start = .; *(.text.efi_runtime*) *(.rodata.efi_runtime*) *(.data.efi_runtime*) - } - - .__efi_runtime_stop : { - *(.__efi_runtime_stop) + __efi_runtime_stop = .; } .text_rest : diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h index 60949200dd93..b6bca53db10d 100644 --- a/include/asm-generic/sections.h +++ b/include/asm-generic/sections.h @@ -35,6 +35,7 @@ extern char __priv_data_start[], __priv_data_end[]; extern char __ctors_start[], __ctors_end[]; extern char __efi_runtime_rel_start[], __efi_runtime_rel_stop[]; +extern char __efi_runtime_start[], __efi_runtime_stop[]; /* function descriptor handling (if any). Override * in asm/sections.h */
__efi_runtime_start/end are defined as c variables for arm7 only in order to force the compiler emit relative references. However, defining those within a section definition will do the same thing. On top of that the v8 linker scripts define it as a symbol. So let's remove the special sections from the linker scripts, the variable definitions from sections.c and define them as a symbols within the correct section. Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- arch/arm/cpu/u-boot.lds | 12 +++--------- arch/arm/lib/sections.c | 2 -- arch/arm/mach-zynq/u-boot.lds | 12 +++--------- include/asm-generic/sections.h | 1 + 4 files changed, 7 insertions(+), 20 deletions(-)