[PATCH v2 0/3] Fix binman_sym functionality on RISC-V port

[PATCH v2 0/3] Fix binman_sym functionality on RISC-V port

[Yao Zi]

It's found that BINMAN_SYMBOLS_OK always evaluates to false on RISC-V,
because our linkerscripts don't define symbol __image_copy_start, on
which binman depends for determining the base address of an entry.
Binman simply bails out in case of missing the symbol.

This series contains three main changes,

1. Correct the entry type used for proper U-Boot, ensuring binman could
   find it, which is mandatory since SPL_BINMAN_UBOOT_SYMBOLS defaults
   to Y on RISC-V
2. Mark the SPL image of StarFive VisionFive2 as "no-write-symbols",
   since the SPL and proper U-Boot are split into two images.
3. Finally provide __image_copy{start,end} in RISC-V linkerscripts of
   both SPL and proper U-Boot, fixing functionality of binman_sym

I've tested building with
  - ae350_rv64_defconfig
  - ae350_rv64_spl_defconfig
  - bananapi-f3_defconfig
  - k230_canmv_defconfig
  - microchip_mpfs_icicle_defconfig
  - milkv_duo_defconfig

and booting successfully on
  - qemu-riscv64_smode_defconfig
  - qemu-riscv64_defconfig
  - sifive_unleashed_defconfig
  - starfive_visionfive2_defconfig
with either real hardware or QEMU.

Thank you Jonas Karlman for pointing out the root cause of binman
failures with SPL_BINMAN_UBOOT_SYMBOLS enabled. I'm looking forward to
further reviews.

Changed from v2
- Drop the patch which disables SPL_BINMAN_UBOOT_SYMBOLS by default on
  RISC-V
- Add patch to correct entry type of proper U-Boot in default binman
  configuration for RISC-V
- Add patch to prevent binman from relocating SPL of VisionFive 2
- Fix typo (one missing underscore) in linkerscript and collect Simon's
  r-b tag.
- Link to v1: https://lore.kernel.org/u-boot/20250407033744.4025-1-ziyao@disroot.org/

Yao Zi (3):
  riscv: dts: binman.dtsi: Switch to u-boot-nodtb entry for proper
    U-Boot
  riscv: dts: starfive: Prevent binman from relocating symbols in SPL
  riscv: Provide __image_copy_{start_end} symbols in linkerscript

 arch/riscv/cpu/u-boot-spl.lds                   | 2 ++
 arch/riscv/cpu/u-boot.lds                       | 3 +++
 arch/riscv/dts/binman.dtsi                      | 2 +-
 arch/riscv/dts/starfive-visionfive2-binman.dtsi | 1 +
 4 files changed, 7 insertions(+), 1 deletion(-)

-- 
2.49.0

[PATCH v2 1/3] riscv: dts: binman.dtsi: Switch to u-boot-nodtb entry for proper U-Boot

[Yao Zi]

Switch to u-boot-nodtb entry which precisely represents a proper U-Boot
and could be matched with u_boot_any. This allows RISC-V ports that make
use of binman to be built without disabling SPL_BINMAN_UBOOT_SYMBOLS
explicitly, which is set to y by default.

Fixes: 0784510f741 ("riscv: sifive: unleashed: Switch to use binman to generate u-boot.itb")
Suggested-by: Jonas Karlman <jonas@kwiboo.se>
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/riscv/dts/binman.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/dts/binman.dtsi b/arch/riscv/dts/binman.dtsi
index 0405faca574..b7656423b69 100644
--- a/arch/riscv/dts/binman.dtsi
+++ b/arch/riscv/dts/binman.dtsi
@@ -35,7 +35,7 @@
 					compression = "none";
 					load = /bits/ 64 <CONFIG_TEXT_BASE>;
 
-					uboot_blob: blob-ext {
+					uboot_blob: u-boot-nodtb {
 						filename = "u-boot-nodtb.bin";
 					};
 				};
-- 
2.49.0

Re: [PATCH v2 1/3] riscv: dts: binman.dtsi: Switch to u-boot-nodtb entry for proper U-Boot

[Simon Glass]

Hi Yao,

On Wed, 16 Apr 2025 at 10:25, Yao Zi <ziyao@disroot.org> wrote:
>
> Switch to u-boot-nodtb entry which precisely represents a proper U-Boot
> and could be matched with u_boot_any. This allows RISC-V ports that make
> use of binman to be built without disabling SPL_BINMAN_UBOOT_SYMBOLS
> explicitly, which is set to y by default.
>
> Fixes: 0784510f741 ("riscv: sifive: unleashed: Switch to use binman to generate u-boot.itb")
> Suggested-by: Jonas Karlman <jonas@kwiboo.se>
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/riscv/dts/binman.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Simon Glass <sjg@chromium.org>

> diff --git a/arch/riscv/dts/binman.dtsi b/arch/riscv/dts/binman.dtsi
> index 0405faca574..b7656423b69 100644
> --- a/arch/riscv/dts/binman.dtsi
> +++ b/arch/riscv/dts/binman.dtsi
> @@ -35,7 +35,7 @@
>                                         compression = "none";
>                                         load = /bits/ 64 <CONFIG_TEXT_BASE>;
>
> -                                       uboot_blob: blob-ext {
> +                                       uboot_blob: u-boot-nodtb {
>                                                 filename = "u-boot-nodtb.bin";

Also you can see the default filename for that entry type in
tools/binman/etype/u_boot_nodtb.py:

    def GetDefaultFilename(self):
        return 'u-boot-nodtb.bin'

so you can drop this filename.

>                                         };
>                                 };
> --
> 2.49.0
>

Regards,
Simon

Re: [PATCH v2 1/3] riscv: dts: binman.dtsi: Switch to u-boot-nodtb entry for proper U-Boot

[Yao Zi]

On Thu, Apr 17, 2025 at 03:35:50PM -0600, Simon Glass wrote:
> Hi Yao,
> 
> On Wed, 16 Apr 2025 at 10:25, Yao Zi <ziyao@disroot.org> wrote:
> >
> > Switch to u-boot-nodtb entry which precisely represents a proper U-Boot
> > and could be matched with u_boot_any. This allows RISC-V ports that make
> > use of binman to be built without disabling SPL_BINMAN_UBOOT_SYMBOLS
> > explicitly, which is set to y by default.
> >
> > Fixes: 0784510f741 ("riscv: sifive: unleashed: Switch to use binman to generate u-boot.itb")
> > Suggested-by: Jonas Karlman <jonas@kwiboo.se>
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  arch/riscv/dts/binman.dtsi | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> > diff --git a/arch/riscv/dts/binman.dtsi b/arch/riscv/dts/binman.dtsi
> > index 0405faca574..b7656423b69 100644
> > --- a/arch/riscv/dts/binman.dtsi
> > +++ b/arch/riscv/dts/binman.dtsi
> > @@ -35,7 +35,7 @@
> >                                         compression = "none";
> >                                         load = /bits/ 64 <CONFIG_TEXT_BASE>;
> >
> > -                                       uboot_blob: blob-ext {
> > +                                       uboot_blob: u-boot-nodtb {
> >                                                 filename = "u-boot-nodtb.bin";
> 
> Also you can see the default filename for that entry type in
> tools/binman/etype/u_boot_nodtb.py:
> 
>     def GetDefaultFilename(self):
>         return 'u-boot-nodtb.bin'
> 
> so you can drop this filename.

Okay, thanks for the hint.

> >                                         };
> >                                 };
> > --
> > 2.49.0
> >
> 
> Regards,
> Simon

Best regards,
Yao Zi

[PATCH v2 2/3] riscv: dts: starfive: Prevent binman from relocating symbols in SPL

[Yao Zi]

SPL and proper U-Boot are split into two images with default binman
configuration of StarFive VisionFive 2, thus proper U-Boot symbols
cannot be found in the SPL image. This fixes errors like

  Section '/binman/spl-img': Symbol '_binman_u_boot_any_prop_size'
    in entry '/binman/spl-img/mkimage/u-boot-spl/u-boot-spl-nodtb':
      Entry 'u-boot-any' not found in list (u-boot-spl-nodtb,
      u-boot-spl-dtb,u-boot-spl,mkimage,spl-img)

Fixes: 90602e779d3 ("riscv: dts: starfive: generate u-boot-spl.bin.normal.out")
Suggested-by: Jonas Karlman <jonas@kwiboo.se>
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/riscv/dts/starfive-visionfive2-binman.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/dts/starfive-visionfive2-binman.dtsi b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
index 4cce001e80d..10e3d964588 100644
--- a/arch/riscv/dts/starfive-visionfive2-binman.dtsi
+++ b/arch/riscv/dts/starfive-visionfive2-binman.dtsi
@@ -96,6 +96,7 @@
 			args = "-T sfspl";
 
 			u-boot-spl {
+				no-write-symbols;
 			};
 		};
 	};
-- 
2.49.0

[PATCH v2 3/3] riscv: Provide __image_copy_{start_end} symbols in linkerscript

[Yao Zi]

Binman looks for __image_copy_start to determine the base address of an
entry if elf-base-sym isn't specified, which is missing in RISC-V port.
This causes binman skips RISC-V SPL entries without filling addresses
into its .binman_sym_table section.

This patch defines __image_copy_start in linkerscript of both SPL and
proper U-Boot to ensure binman_sym functions correctly with the default
binman.dtsi. The paired symbol, __image_copy_end, is introduced as well
for completeness.

Signed-off-by: Yao Zi <ziyao@disroot.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 arch/riscv/cpu/u-boot-spl.lds | 2 ++
 arch/riscv/cpu/u-boot.lds     | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/arch/riscv/cpu/u-boot-spl.lds b/arch/riscv/cpu/u-boot-spl.lds
index 907094620bd..0717833df55 100644
--- a/arch/riscv/cpu/u-boot-spl.lds
+++ b/arch/riscv/cpu/u-boot-spl.lds
@@ -16,6 +16,7 @@ ENTRY(_start)
 SECTIONS
 {
 	. = ALIGN(4);
+	__image_copy_start = ADDR(.text);
 	.text : {
 		arch/riscv/cpu/start.o	(.text)
 		*(.text*)
@@ -46,6 +47,7 @@ SECTIONS
 
 	_end = .;
 	_image_binary_end = .;
+	__image_copy_end = .;
 
 	.bss : {
 		__bss_start = .;
diff --git a/arch/riscv/cpu/u-boot.lds b/arch/riscv/cpu/u-boot.lds
index 2ffe6ba3c8f..b11ea8b56d2 100644
--- a/arch/riscv/cpu/u-boot.lds
+++ b/arch/riscv/cpu/u-boot.lds
@@ -10,6 +10,7 @@ ENTRY(_start)
 SECTIONS
 {
 	. = ALIGN(4);
+	__image_copy_start = ADDR(.text);
 	.text : {
 		arch/riscv/cpu/start.o	(.text)
 	}
@@ -57,6 +58,8 @@ SECTIONS
 		__efi_runtime_rel_stop = .;
 	}
 
+	__image_copy_end = .;
+
 	/DISCARD/ : { *(.rela.plt*) }
 	.rela.dyn : {
 		__rel_dyn_start = .;
-- 
2.49.0