| Message ID | 20240228104811.1366694-3-ilias.apalodimas@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Clean up arm linker scripts | expand |
On 28/02/2024 10:48, Ilias Apalodimas wrote: > commit 3ebd1cbc49f0 ("arm: make __bss_start and __bss_end__ compiler-generated") > and > commit f84a7b8f54db ("ARM: Fix __bss_start and __bss_end in linker scripts") > were moving the bss_start/end on c generated variables that were > injected in their own sections. The reason was that we needed relative > relocations for position independent code. > > However, the linker documentation pages states that symbols that are > defined within a section definition will create a relocatable > type with the value being a fixed offset from the base of a section. > > So let's start cleaning this up starting with the bss_start and bss_end > variables. Convert them into symbols within the .bss section definition. > > [0] https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_mono/ld.html#SEC13 > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> Tested-by: Caleb Connolly <caleb.connolly@linaro.org> # Qualcomm sdm845 This patch and a few others in this series conflict with my patch [1] which drops the dragonboard820c linker script. My patch doesn't have any external dependencies, so would it be easier for you to take it along with this series? Or how best should we avoid the merge conflicts here? [1]: https://lore.kernel.org/u-boot/20240226-b4-qcom-common-target-v5-19-10c8e078befb@linaro.org/ Thanks and regards, > --- > arch/arm/cpu/armv8/u-boot-spl.lds | 14 +++----------- > arch/arm/cpu/armv8/u-boot.lds | 15 +++------------ > arch/arm/cpu/u-boot.lds | 22 ++++------------------ > arch/arm/lib/sections.c | 2 -- > arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 14 +++----------- > arch/arm/mach-zynq/u-boot.lds | 21 +++------------------ > board/qualcomm/dragonboard820c/u-boot.lds | 15 +++------------ > 7 files changed, 19 insertions(+), 84 deletions(-) > > diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds > index 7cb9d731246d..16fddb46e9cb 100644 > --- a/arch/arm/cpu/armv8/u-boot-spl.lds > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds > @@ -63,18 +63,10 @@ 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)); > + __bss_end = .; > } >.sdram > > /DISCARD/ : { *(.rela*) } > diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds > index fb6a30c922f7..c4ee10ebc3ff 100644 > --- a/arch/arm/cpu/armv8/u-boot.lds > +++ b/arch/arm/cpu/armv8/u-boot.lds > @@ -149,19 +149,10 @@ SECTIONS > > _end = .; > > - . = ALIGN(8); > - > - .bss_start : { > - KEEP(*(.__bss_start)); > - } > - > - .bss : { > + .bss ALIGN(8): { > + __bss_start = .; > *(.bss*) > - . = ALIGN(8); > - } > - > - .bss_end : { > - KEEP(*(.__bss_end)); > + __bss_end = .; > } > > /DISCARD/ : { *(.dynsym) } > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > index 7724c9332c3b..90d329b1ebe0 100644 > --- a/arch/arm/cpu/u-boot.lds > +++ b/arch/arm/cpu/u-boot.lds > @@ -206,27 +206,13 @@ SECTIONS > *(.mmutable) > } > > -/* > - * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c > - * __bss_base and __bss_limit are for linker only (overlay ordering) > - */ > - > - .bss_start __rel_dyn_start (OVERLAY) : { > - KEEP(*(.__bss_start)); > - __bss_base = .; > - } > - > - .bss __bss_base (OVERLAY) : { > + .bss ALIGN(4): { > + __bss_start = .; > *(.bss*) > - . = ALIGN(4); > - __bss_limit = .; > - } > - > - .bss_end __bss_limit (OVERLAY) : { > - KEEP(*(.__bss_end)); > + __bss_end = .; > } > > - .dynsym _image_binary_end : { *(.dynsym) } > + .dynsym : { *(.dynsym) } > .dynbss : { *(.dynbss) } > .dynstr : { *(.dynstr*) } > .dynamic : { *(.dynamic*) } > diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c > index 857879711c6a..8e8bd5797e16 100644 > --- a/arch/arm/lib/sections.c > +++ b/arch/arm/lib/sections.c > @@ -19,8 +19,6 @@ > * aliasing warnings. > */ > > -char __bss_start[0] __section(".__bss_start"); > -char __bss_end[0] __section(".__bss_end"); > char __image_copy_start[0] __section(".__image_copy_start"); > char __image_copy_end[0] __section(".__image_copy_end"); > char __rel_dyn_start[0] __section(".__rel_dyn_start"); > diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds > index 74618eba591b..b7887194026e 100644 > --- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds > +++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds > @@ -56,18 +56,10 @@ SECTIONS > > _image_binary_end = .; > > - .bss_start (NOLOAD) : { > - . = ALIGN(8); > - KEEP(*(.__bss_start)); > - } > - > - .bss (NOLOAD) : { > + .bss ALIGN(8) : { > + __bss_start = .; > *(.bss*) > - . = ALIGN(8); > - } > - > - .bss_end (NOLOAD) : { > - KEEP(*(.__bss_end)); > + __bss_end = .; > } > > /DISCARD/ : { *(.dynsym) } > diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds > index 3b7c9d515f8b..8d3259821719 100644 > --- a/arch/arm/mach-zynq/u-boot.lds > +++ b/arch/arm/mach-zynq/u-boot.lds > @@ -102,26 +102,11 @@ SECTIONS > > _image_binary_end = .; > > -/* > - * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c > - * __bss_base and __bss_limit are for linker only (overlay ordering) > - */ > - > - .bss_start __rel_dyn_start (OVERLAY) : { > - KEEP(*(.__bss_start)); > - __bss_base = .; > - } > - > - .bss __bss_base (OVERLAY) : { > + .bss ALIGN(4): { > + __bss_start = .; > *(.bss*) > - . = ALIGN(8); > - __bss_limit = .; > - } > - > - .bss_end __bss_limit (OVERLAY) : { > - KEEP(*(.__bss_end)); > + __bss_end = .; > } > - > /* > * Zynq needs to discard these sections because the user > * is expected to pass this image on to tools for boot.bin > diff --git a/board/qualcomm/dragonboard820c/u-boot.lds b/board/qualcomm/dragonboard820c/u-boot.lds > index 5251b59fbe76..d3cc5278b610 100644 > --- a/board/qualcomm/dragonboard820c/u-boot.lds > +++ b/board/qualcomm/dragonboard820c/u-boot.lds > @@ -87,19 +87,10 @@ SECTIONS > > _end = .; > > - . = ALIGN(8); > - > - .bss_start : { > - KEEP(*(.__bss_start)); > - } > - > - .bss : { > + .bss ALIGN(8): { > + __bss_start = .; > *(.bss*) > - . = ALIGN(8); > - } > - > - .bss_end : { > - KEEP(*(.__bss_end)); > + __bss_end = .; > } > > /DISCARD/ : { *(.dynsym) } > -- > 2.37.2 >
Thanks for testing Caleb, On Thu, 29 Feb 2024 at 15:49, Caleb Connolly <caleb.connolly@linaro.org> wrote: > > > > On 28/02/2024 10:48, Ilias Apalodimas wrote: > > commit 3ebd1cbc49f0 ("arm: make __bss_start and __bss_end__ compiler-generated") > > and > > commit f84a7b8f54db ("ARM: Fix __bss_start and __bss_end in linker scripts") > > were moving the bss_start/end on c generated variables that were > > injected in their own sections. The reason was that we needed relative > > relocations for position independent code. > > > > However, the linker documentation pages states that symbols that are > > defined within a section definition will create a relocatable > > type with the value being a fixed offset from the base of a section. > > > > So let's start cleaning this up starting with the bss_start and bss_end > > variables. Convert them into symbols within the .bss section definition. > > > > [0] https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_mono/ld.html#SEC13 > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > Tested-by: Caleb Connolly <caleb.connolly@linaro.org> # Qualcomm sdm845 > > This patch and a few others in this series conflict with my patch [1] > which drops the dragonboard820c linker script. My patch doesn't have any > external dependencies, so would it be easier for you to take it along > with this series? Or how best should we avoid the merge conflicts here? Yes, I can carry the snapdragon lds removal. Rebasing on top is of it is pretty easy /Ilias > > [1]: > https://lore.kernel.org/u-boot/20240226-b4-qcom-common-target-v5-19-10c8e078befb@linaro.org/ > > Thanks and regards, > > --- > > arch/arm/cpu/armv8/u-boot-spl.lds | 14 +++----------- > > arch/arm/cpu/armv8/u-boot.lds | 15 +++------------ > > arch/arm/cpu/u-boot.lds | 22 ++++------------------ > > arch/arm/lib/sections.c | 2 -- > > arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 14 +++----------- > > arch/arm/mach-zynq/u-boot.lds | 21 +++------------------ > > board/qualcomm/dragonboard820c/u-boot.lds | 15 +++------------ > > 7 files changed, 19 insertions(+), 84 deletions(-) > > > > diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds > > index 7cb9d731246d..16fddb46e9cb 100644 > > --- a/arch/arm/cpu/armv8/u-boot-spl.lds > > +++ b/arch/arm/cpu/armv8/u-boot-spl.lds > > @@ -63,18 +63,10 @@ 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)); > > + __bss_end = .; > > } >.sdram > > > > /DISCARD/ : { *(.rela*) } > > diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds > > index fb6a30c922f7..c4ee10ebc3ff 100644 > > --- a/arch/arm/cpu/armv8/u-boot.lds > > +++ b/arch/arm/cpu/armv8/u-boot.lds > > @@ -149,19 +149,10 @@ SECTIONS > > > > _end = .; > > > > - . = ALIGN(8); > > - > > - .bss_start : { > > - KEEP(*(.__bss_start)); > > - } > > - > > - .bss : { > > + .bss ALIGN(8): { > > + __bss_start = .; > > *(.bss*) > > - . = ALIGN(8); > > - } > > - > > - .bss_end : { > > - KEEP(*(.__bss_end)); > > + __bss_end = .; > > } > > > > /DISCARD/ : { *(.dynsym) } > > diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds > > index 7724c9332c3b..90d329b1ebe0 100644 > > --- a/arch/arm/cpu/u-boot.lds > > +++ b/arch/arm/cpu/u-boot.lds > > @@ -206,27 +206,13 @@ SECTIONS > > *(.mmutable) > > } > > > > -/* > > - * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c > > - * __bss_base and __bss_limit are for linker only (overlay ordering) > > - */ > > - > > - .bss_start __rel_dyn_start (OVERLAY) : { > > - KEEP(*(.__bss_start)); > > - __bss_base = .; > > - } > > - > > - .bss __bss_base (OVERLAY) : { > > + .bss ALIGN(4): { > > + __bss_start = .; > > *(.bss*) > > - . = ALIGN(4); > > - __bss_limit = .; > > - } > > - > > - .bss_end __bss_limit (OVERLAY) : { > > - KEEP(*(.__bss_end)); > > + __bss_end = .; > > } > > > > - .dynsym _image_binary_end : { *(.dynsym) } > > + .dynsym : { *(.dynsym) } > > .dynbss : { *(.dynbss) } > > .dynstr : { *(.dynstr*) } > > .dynamic : { *(.dynamic*) } > > diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c > > index 857879711c6a..8e8bd5797e16 100644 > > --- a/arch/arm/lib/sections.c > > +++ b/arch/arm/lib/sections.c > > @@ -19,8 +19,6 @@ > > * aliasing warnings. > > */ > > > > -char __bss_start[0] __section(".__bss_start"); > > -char __bss_end[0] __section(".__bss_end"); > > char __image_copy_start[0] __section(".__image_copy_start"); > > char __image_copy_end[0] __section(".__image_copy_end"); > > char __rel_dyn_start[0] __section(".__rel_dyn_start"); > > diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds > > index 74618eba591b..b7887194026e 100644 > > --- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds > > +++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds > > @@ -56,18 +56,10 @@ SECTIONS > > > > _image_binary_end = .; > > > > - .bss_start (NOLOAD) : { > > - . = ALIGN(8); > > - KEEP(*(.__bss_start)); > > - } > > - > > - .bss (NOLOAD) : { > > + .bss ALIGN(8) : { > > + __bss_start = .; > > *(.bss*) > > - . = ALIGN(8); > > - } > > - > > - .bss_end (NOLOAD) : { > > - KEEP(*(.__bss_end)); > > + __bss_end = .; > > } > > > > /DISCARD/ : { *(.dynsym) } > > diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds > > index 3b7c9d515f8b..8d3259821719 100644 > > --- a/arch/arm/mach-zynq/u-boot.lds > > +++ b/arch/arm/mach-zynq/u-boot.lds > > @@ -102,26 +102,11 @@ SECTIONS > > > > _image_binary_end = .; > > > > -/* > > - * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c > > - * __bss_base and __bss_limit are for linker only (overlay ordering) > > - */ > > - > > - .bss_start __rel_dyn_start (OVERLAY) : { > > - KEEP(*(.__bss_start)); > > - __bss_base = .; > > - } > > - > > - .bss __bss_base (OVERLAY) : { > > + .bss ALIGN(4): { > > + __bss_start = .; > > *(.bss*) > > - . = ALIGN(8); > > - __bss_limit = .; > > - } > > - > > - .bss_end __bss_limit (OVERLAY) : { > > - KEEP(*(.__bss_end)); > > + __bss_end = .; > > } > > - > > /* > > * Zynq needs to discard these sections because the user > > * is expected to pass this image on to tools for boot.bin > > diff --git a/board/qualcomm/dragonboard820c/u-boot.lds b/board/qualcomm/dragonboard820c/u-boot.lds > > index 5251b59fbe76..d3cc5278b610 100644 > > --- a/board/qualcomm/dragonboard820c/u-boot.lds > > +++ b/board/qualcomm/dragonboard820c/u-boot.lds > > @@ -87,19 +87,10 @@ SECTIONS > > > > _end = .; > > > > - . = ALIGN(8); > > - > > - .bss_start : { > > - KEEP(*(.__bss_start)); > > - } > > - > > - .bss : { > > + .bss ALIGN(8): { > > + __bss_start = .; > > *(.bss*) > > - . = ALIGN(8); > > - } > > - > > - .bss_end : { > > - KEEP(*(.__bss_end)); > > + __bss_end = .; > > } > > > > /DISCARD/ : { *(.dynsym) } > > -- > > 2.37.2 > > > > -- > // Caleb (they/them)
diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds b/arch/arm/cpu/armv8/u-boot-spl.lds index 7cb9d731246d..16fddb46e9cb 100644 --- a/arch/arm/cpu/armv8/u-boot-spl.lds +++ b/arch/arm/cpu/armv8/u-boot-spl.lds @@ -63,18 +63,10 @@ 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)); + __bss_end = .; } >.sdram /DISCARD/ : { *(.rela*) } diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds index fb6a30c922f7..c4ee10ebc3ff 100644 --- a/arch/arm/cpu/armv8/u-boot.lds +++ b/arch/arm/cpu/armv8/u-boot.lds @@ -149,19 +149,10 @@ SECTIONS _end = .; - . = ALIGN(8); - - .bss_start : { - KEEP(*(.__bss_start)); - } - - .bss : { + .bss ALIGN(8): { + __bss_start = .; *(.bss*) - . = ALIGN(8); - } - - .bss_end : { - KEEP(*(.__bss_end)); + __bss_end = .; } /DISCARD/ : { *(.dynsym) } diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds index 7724c9332c3b..90d329b1ebe0 100644 --- a/arch/arm/cpu/u-boot.lds +++ b/arch/arm/cpu/u-boot.lds @@ -206,27 +206,13 @@ SECTIONS *(.mmutable) } -/* - * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c - * __bss_base and __bss_limit are for linker only (overlay ordering) - */ - - .bss_start __rel_dyn_start (OVERLAY) : { - KEEP(*(.__bss_start)); - __bss_base = .; - } - - .bss __bss_base (OVERLAY) : { + .bss ALIGN(4): { + __bss_start = .; *(.bss*) - . = ALIGN(4); - __bss_limit = .; - } - - .bss_end __bss_limit (OVERLAY) : { - KEEP(*(.__bss_end)); + __bss_end = .; } - .dynsym _image_binary_end : { *(.dynsym) } + .dynsym : { *(.dynsym) } .dynbss : { *(.dynbss) } .dynstr : { *(.dynstr*) } .dynamic : { *(.dynamic*) } diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c index 857879711c6a..8e8bd5797e16 100644 --- a/arch/arm/lib/sections.c +++ b/arch/arm/lib/sections.c @@ -19,8 +19,6 @@ * aliasing warnings. */ -char __bss_start[0] __section(".__bss_start"); -char __bss_end[0] __section(".__bss_end"); char __image_copy_start[0] __section(".__image_copy_start"); char __image_copy_end[0] __section(".__image_copy_end"); char __rel_dyn_start[0] __section(".__rel_dyn_start"); diff --git a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds index 74618eba591b..b7887194026e 100644 --- a/arch/arm/mach-rockchip/u-boot-tpl-v8.lds +++ b/arch/arm/mach-rockchip/u-boot-tpl-v8.lds @@ -56,18 +56,10 @@ SECTIONS _image_binary_end = .; - .bss_start (NOLOAD) : { - . = ALIGN(8); - KEEP(*(.__bss_start)); - } - - .bss (NOLOAD) : { + .bss ALIGN(8) : { + __bss_start = .; *(.bss*) - . = ALIGN(8); - } - - .bss_end (NOLOAD) : { - KEEP(*(.__bss_end)); + __bss_end = .; } /DISCARD/ : { *(.dynsym) } diff --git a/arch/arm/mach-zynq/u-boot.lds b/arch/arm/mach-zynq/u-boot.lds index 3b7c9d515f8b..8d3259821719 100644 --- a/arch/arm/mach-zynq/u-boot.lds +++ b/arch/arm/mach-zynq/u-boot.lds @@ -102,26 +102,11 @@ SECTIONS _image_binary_end = .; -/* - * Compiler-generated __bss_start and __bss_end, see arch/arm/lib/bss.c - * __bss_base and __bss_limit are for linker only (overlay ordering) - */ - - .bss_start __rel_dyn_start (OVERLAY) : { - KEEP(*(.__bss_start)); - __bss_base = .; - } - - .bss __bss_base (OVERLAY) : { + .bss ALIGN(4): { + __bss_start = .; *(.bss*) - . = ALIGN(8); - __bss_limit = .; - } - - .bss_end __bss_limit (OVERLAY) : { - KEEP(*(.__bss_end)); + __bss_end = .; } - /* * Zynq needs to discard these sections because the user * is expected to pass this image on to tools for boot.bin diff --git a/board/qualcomm/dragonboard820c/u-boot.lds b/board/qualcomm/dragonboard820c/u-boot.lds index 5251b59fbe76..d3cc5278b610 100644 --- a/board/qualcomm/dragonboard820c/u-boot.lds +++ b/board/qualcomm/dragonboard820c/u-boot.lds @@ -87,19 +87,10 @@ SECTIONS _end = .; - . = ALIGN(8); - - .bss_start : { - KEEP(*(.__bss_start)); - } - - .bss : { + .bss ALIGN(8): { + __bss_start = .; *(.bss*) - . = ALIGN(8); - } - - .bss_end : { - KEEP(*(.__bss_end)); + __bss_end = .; } /DISCARD/ : { *(.dynsym) }
commit 3ebd1cbc49f0 ("arm: make __bss_start and __bss_end__ compiler-generated") and commit f84a7b8f54db ("ARM: Fix __bss_start and __bss_end in linker scripts") were moving the bss_start/end on c generated variables that were injected in their own sections. The reason was that we needed relative relocations for position independent code. However, the linker documentation pages states that symbols that are defined within a section definition will create a relocatable type with the value being a fixed offset from the base of a section. So let's start cleaning this up starting with the bss_start and bss_end variables. Convert them into symbols within the .bss section definition. [0] https://ftp.gnu.org/old-gnu/Manuals/ld-2.9.1/html_mono/ld.html#SEC13 Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- arch/arm/cpu/armv8/u-boot-spl.lds | 14 +++----------- arch/arm/cpu/armv8/u-boot.lds | 15 +++------------ arch/arm/cpu/u-boot.lds | 22 ++++------------------ arch/arm/lib/sections.c | 2 -- arch/arm/mach-rockchip/u-boot-tpl-v8.lds | 14 +++----------- arch/arm/mach-zynq/u-boot.lds | 21 +++------------------ board/qualcomm/dragonboard820c/u-boot.lds | 15 +++------------ 7 files changed, 19 insertions(+), 84 deletions(-) -- 2.37.2