Historically, when we have an appended device tree and also our
resulting binary will contain the BSS section, we have ensured that
everything will be where it's expected to be by declaring that the BSS
is overlayed with a symbol matches the end of the port of the ELF binary
that is objcopy'd to the binary we concatenate with. This in turn means
that the logic to generate a "pad" file, which is the size found in the
__bss_size symbol, will be correct and then we can concatenate the
device tree and it will begin at __bss_size at run time.

With commit 5ffc1dcc26d3 ("arm: Remove rel.dyn from SPL linker scripts")
we removed this overlay as part of trying to ensure that we met both the
requirements of the device tree to be 8 byte aligned as well as that our
logic to generate the -pad file would match what ended up in the
resulting binary. While it was correct to remove an unused section it
did not solve ultimately solve the problem for all cases.

To really fix the problem, we need to do two things. First, our final
section prior to _image_binary_end must be 8 byte aligned (for the case
of having a separate BSS and so our appended DTB exists at this
location). This cannot be '.binman_sym_table' as it may be empty, and in
turn the ELF type would be NOBITS and so not copied with objcopy. The
__u_boot_list section will never be empty, so it is our final section,
and ends with a '. = ALIGN(8)' statement. Second, as this is the end of
our copied data it is safe to declare that the BSS starts here, so use
the OVERLAY keyword to place the BSS here.

Fixes: 5ffc1dcc26d3 ("arm: Remove rel.dyn from SPL linker scripts")
Reported-by: Brian Sune <[email protected]>
Reported-by: Phil Phil Sutter <[email protected]>
Signed-off-by: Tom Rini <[email protected]>
---
Cc: Richard Henderson <[email protected]>
Cc: Ilias Apalodimas <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Nobuhiro Iwamatsu <[email protected]>
Cc: Nathan Barrett-Morrison <[email protected]>
Cc: Greg Malysa <[email protected]>
Cc: Ian Roberts <[email protected]>
Cc: Vasileios Bimpikas <[email protected]>
Cc: Utsav Agarwal <[email protected]>
Cc: Arturs Artamonovs <[email protected]>
Cc: [email protected]

So, first, with the exception of:
axm brppt2 imx28_btt3 imx28_xea mk808 taurus work_92105
all platforms using arch/arm/cpu/u-boot-spl.lds do NOT enable
CONFIG_SPL_SEPARATE_BSS. And of those, only brppt2 enables SPL_OF_REAL.
As an aside, I'm thinking that there is room for a clean-up / merger of
arch/arm/cpu/u-boot-spl.lds and arch/arm/mach-omap2/u-boot-spl.lds
(*widely* used outside of mach-omap2). But for this patch, that one case
is handled correctly, with the first change I explained. All other 32bit
platforms are handled with the second.

Which means second, 64bit ARM platforms. With the exception of:
socfpga_agilex7m socfpga_n5x_atf socfpga_n5x_vab socfpga_stratix10_atf
all ARM64 SPL_FRAMEWORK using platforms use the
arch/arm/cpu/armv8/u-boot-spl.lds explicitly. These other ones set
CONFIG_SPL_LDSCRIPT to a non-existant file and then fall back to using
arch/arm/cpu/armv8/u-boot-spl.lds. There are only 4 platforms which
don't use a separate BSS section and so could trip up the above as well:
r8a779g0_whitehawk r8a779g3_sparrowhawk sc598-som-ezkit-spl sc598-som-ezlite-spl
I have inspected the Renesas platforms manually, and I'm a little unsure
how this case works on the two sc598 platforms.
---
 arch/arm/cpu/armv8/u-boot-spl.lds | 15 ++++++++-------
 arch/arm/cpu/u-boot-spl.lds       | 12 ++++++------
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/arch/arm/cpu/armv8/u-boot-spl.lds 
b/arch/arm/cpu/armv8/u-boot-spl.lds
index b732133ce76d..39020a1fddde 100644
--- a/arch/arm/cpu/armv8/u-boot-spl.lds
+++ b/arch/arm/cpu/armv8/u-boot-spl.lds
@@ -49,12 +49,6 @@ SECTIONS
        } >.sram
 #endif
 
-       __u_boot_list : {
-               . = ALIGN(8);
-               KEEP(*(SORT(__u_boot_list*)));
-               . = ALIGN(8);
-       } >.sram
-
        .binman_sym_table : {
                . = ALIGN(8);
                __binman_sym_start = .;
@@ -63,6 +57,12 @@ SECTIONS
                . = ALIGN(8);
        } > .sram
 
+       __u_boot_list : {
+               . = ALIGN(8);
+               KEEP(*(SORT(__u_boot_list*)));
+               . = ALIGN(8);
+       } >.sram
+
        __image_copy_end = .;
        _end = .;
        _image_binary_end = .;
@@ -75,7 +75,7 @@ SECTIONS
                __bss_end = .;
        } >.sdram
 #else
-       .bss (NOLOAD) : {
+       .bss _image_binary_end (OVERLAY) : {
                __bss_start = .;
                *(.bss*)
                 . = ALIGN(8);
@@ -99,5 +99,6 @@ SECTIONS
 
 ASSERT(_image_binary_end % 8 == 0, \
        "_image_binary_end must be 8-byte aligned for device tree");
+
 ASSERT(ADDR(.bss) % 8 == 0, \
        ".bss must be 8-byte aligned");
diff --git a/arch/arm/cpu/u-boot-spl.lds b/arch/arm/cpu/u-boot-spl.lds
index c578c3ebf821..97083319d39d 100644
--- a/arch/arm/cpu/u-boot-spl.lds
+++ b/arch/arm/cpu/u-boot-spl.lds
@@ -31,16 +31,16 @@ SECTIONS
                *(.data*)
        }
 
-       . = ALIGN(4);
-       __u_boot_list : {
-               KEEP(*(SORT(__u_boot_list*)));
-       }
-
        . = ALIGN(4);
        .binman_sym_table : {
                __binman_sym_start = .;
                KEEP(*(SORT(.binman_sym*)));
                __binman_sym_end = .;
+       }
+
+       . = ALIGN(4);
+       __u_boot_list : {
+               KEEP(*(SORT(__u_boot_list*)));
                . = ALIGN(8);
        }
 
@@ -48,7 +48,7 @@ SECTIONS
        _image_binary_end = .;
        _end = .;
 
-       .bss : {
+       .bss _image_binary_end (OVERLAY) : {
                __bss_start = .;
                *(.bss*)
                 . = ALIGN(8);
-- 
2.43.0

Reply via email to