[PATCH 0/5] Simplifiy retrieving FDT from SBI in S-Mode

[PATCH 0/5] Simplifiy retrieving FDT from SBI in S-Mode

[Yao Zi]

A typical bootflow of S-Mode RISC-V U-Boot looks like,

  -------    -------    ----------
  | SPL | -> | SBI | -> | U-Boot |
  -------    -------    ----------
                |
     M-Mode     |       S-Mode
                |

Both the most popular SBI implementation, OpenSBI, and U-Boot require a
FDT to function. The common solution is to pick an appropriate
devicetree at SPL stage and pass it to OpenSBI, which is also capable of
passing its (possibly modified) devicetree to the next stage (proper
U-Boot here) program.

The problem is although we retrieve the FDT passed by SBI in RISC-V's
start.S, a custom board_fdt_blob_setup is still required to make use of
it, resulting in duplicated similar functions in board-level code.

This series provides a weak version of board_fdt_blob_setup to setup the
SBI-passed FDT, serving as fallback on RISC-V platforms to eliminate the
duplication. We also select OF_HAS_PRIOR_STAGE by default on RISC-V
platforms if SBI is enabled, which is the usual configuration.

Tested on
 - Milk-V Duo (milkv_duo_defconfig)
 - Starfive VisionFive 2 (starfive_visionfive2_defconfig)
 - QEMU VM (qemu-riscv64_smode_defconfig).

Yao Zi (5):
  riscv: lib: Add a default implementation of board_fdt_blob_setup
  board: qemu: riscv: Remove duplicated board_fdt_blob_setup
  board: starfive: Remove duplicated board_fdt_blob_setup
  board: sifive: Remove duplicated board_fdt_blob_setup
  riscv: select OF_HAS_PRIOR_STAGE by default if SBI is enabled

 arch/riscv/Kconfig                            |  1 +
 arch/riscv/lib/Makefile                       |  1 +
 arch/riscv/lib/board.c                        | 19 +++++++++++++++++++
 board/emulation/qemu-riscv/qemu-riscv.c       |  8 --------
 board/sifive/unleashed/unleashed.c            | 11 -----------
 board/sifive/unmatched/unmatched.c            | 10 ----------
 .../visionfive2/starfive_visionfive2.c        | 10 ----------
 7 files changed, 21 insertions(+), 39 deletions(-)
 create mode 100644 arch/riscv/lib/board.c

-- 
2.48.1

[PATCH 1/5] riscv: lib: Add a default implementation of board_fdt_blob_setup

[Yao Zi]

It's common for S-Mode U-Boot to retrieve a FDT blob along with taking
control from SBI firmware. Add a weak version of board_fdt_blob_setup to
make use of it by default and avoid copy-pasting similar functions among
boards.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/riscv/lib/Makefile |  1 +
 arch/riscv/lib/board.c  | 19 +++++++++++++++++++
 2 files changed, 20 insertions(+)
 create mode 100644 arch/riscv/lib/board.c

diff --git a/arch/riscv/lib/Makefile b/arch/riscv/lib/Makefile
index 268116f3757..b33f9c646dd 100644
--- a/arch/riscv/lib/Makefile
+++ b/arch/riscv/lib/Makefile
@@ -27,6 +27,7 @@ obj-$(CONFIG_$(XPL_)SMP) += smp.o
 obj-$(CONFIG_XPL_BUILD)	+= spl.o
 obj-y   += fdt_fixup.o
 obj-$(CONFIG_$(SPL)CMD_BDI) += bdinfo.o
+obj-$(CONFIG_OF_BOARD) += board.o
 
 # For building EFI apps
 CFLAGS_NON_EFI := -fstack-protector-strong
diff --git a/arch/riscv/lib/board.c b/arch/riscv/lib/board.c
new file mode 100644
index 00000000000..77bc174b047
--- /dev/null
+++ b/arch/riscv/lib/board.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * RISC-V-specific handling of firmware FDT
+ */
+
+#include <asm/global_data.h>
+#include <linux/errno.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+__weak int board_fdt_blob_setup(void **fdtp)
+{
+	if (!gd->arch.firmware_fdt_addr)
+		return -EEXIST;
+
+	*fdtp = (void *)gd->arch.firmware_fdt_addr;
+
+	return 0;
+}
-- 
2.48.1

[PATCH 2/5] board: qemu: riscv: Remove duplicated board_fdt_blob_setup

[Yao Zi]

The default version should work for RISC-V QEMU.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 board/emulation/qemu-riscv/qemu-riscv.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/board/emulation/qemu-riscv/qemu-riscv.c b/board/emulation/qemu-riscv/qemu-riscv.c
index a90222ea6a4..70190ebe8fc 100644
--- a/board/emulation/qemu-riscv/qemu-riscv.c
+++ b/board/emulation/qemu-riscv/qemu-riscv.c
@@ -63,11 +63,3 @@ int board_fit_config_name_match(const char *name)
 	return 0;
 }
 #endif
-
-int board_fdt_blob_setup(void **fdtp)
-{
-	/* Stored the DTB address there during our init */
-	*fdtp = (void *)(ulong)gd->arch.firmware_fdt_addr;
-
-	return 0;
-}
-- 
2.48.1

[PATCH 3/5] board: starfive: Remove duplicated board_fdt_blob_setup

[Yao Zi]

The default version should work for Starfive VisionFive 2.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 board/starfive/visionfive2/starfive_visionfive2.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/board/starfive/visionfive2/starfive_visionfive2.c b/board/starfive/visionfive2/starfive_visionfive2.c
index 3940d45b13f..96fb8dfe934 100644
--- a/board/starfive/visionfive2/starfive_visionfive2.c
+++ b/board/starfive/visionfive2/starfive_visionfive2.c
@@ -115,16 +115,6 @@ int board_late_init(void)
 	return 0;
 }
 
-int board_fdt_blob_setup(void **fdtp)
-{
-	if (gd->arch.firmware_fdt_addr) {
-		*fdtp = (ulong *)(uintptr_t)gd->arch.firmware_fdt_addr;
-		return 0;
-	}
-
-	return -EEXIST;
-}
-
 int ft_board_setup(void *blob, struct bd_info *bd)
 {
 	return fdt_fixup_memory(blob, 0x40000000, gd->ram_size);
-- 
2.48.1

[PATCH 4/5] board: sifive: Remove duplicated board_fdt_blob_setup

[Yao Zi]

The default version should work for both SiFive Unmatched and Unleashed.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 board/sifive/unleashed/unleashed.c | 11 -----------
 board/sifive/unmatched/unmatched.c | 10 ----------
 2 files changed, 21 deletions(-)

diff --git a/board/sifive/unleashed/unleashed.c b/board/sifive/unleashed/unleashed.c
index c1c374610c3..f5da289b836 100644
--- a/board/sifive/unleashed/unleashed.c
+++ b/board/sifive/unleashed/unleashed.c
@@ -114,17 +114,6 @@ int misc_init_r(void)
 
 #endif
 
-int board_fdt_blob_setup(void **fdtp)
-{
-	if (gd->arch.firmware_fdt_addr) {
-		*fdtp = (ulong *)(uintptr_t)gd->arch.firmware_fdt_addr;
-
-		return 0;
-	}
-
-	return -EEXIST;
-}
-
 int board_init(void)
 {
 	/* enable all cache ways */
diff --git a/board/sifive/unmatched/unmatched.c b/board/sifive/unmatched/unmatched.c
index 23e03e145ee..a57ce1f10fe 100644
--- a/board/sifive/unmatched/unmatched.c
+++ b/board/sifive/unmatched/unmatched.c
@@ -10,16 +10,6 @@
 #include <dm.h>
 #include <asm/sections.h>
 
-int board_fdt_blob_setup(void **fdtp)
-{
-	if (gd->arch.firmware_fdt_addr) {
-		*fdtp = (ulong *)(uintptr_t)gd->arch.firmware_fdt_addr;
-		return 0;
-	}
-
-	return -EEXIST;
-}
-
 int board_init(void)
 {
 	/* enable all cache ways */
-- 
2.48.1

[PATCH 5/5] riscv: select OF_HAS_PRIOR_STAGE by default if SBI is enabled

[Yao Zi]

Availability of RISC-V SBI service implies a prior stage exists. As SBI
firmware usually passes a FDT to the loaded program, let's select
OF_HAS_PRIOR_STAGE if SBI is enabled.

With previously added fallback version of board_fdt_blob_setup, S-Mode
RISC-V ports use the SBI-provided FDT by default. This covers the most
common usecase, where a SPL (probably the U-Boot one) selects proper
devicetree, loads SBI and U-Boot then invokes SBI with the devicetree.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/riscv/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index b24623590f2..f7706788f92 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -410,6 +410,7 @@ config NR_CPUS
 config SBI
 	bool
 	default y if RISCV_SMODE || SPL_RISCV_SMODE
+	imply OF_HAS_PRIOR_STAGE
 
 choice
 	prompt "SBI support"
-- 
2.48.1

Re: [PATCH 1/5] riscv: lib: Add a default implementation of board_fdt_blob_setup

[Leo Liang]

On Thu, Feb 27, 2025 at 02:47:29PM +0000, Yao Zi wrote:
> It's common for S-Mode U-Boot to retrieve a FDT blob along with taking
> control from SBI firmware. Add a weak version of board_fdt_blob_setup to
> make use of it by default and avoid copy-pasting similar functions among
> boards.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/riscv/lib/Makefile |  1 +
>  arch/riscv/lib/board.c  | 19 +++++++++++++++++++
>  2 files changed, 20 insertions(+)
>  create mode 100644 arch/riscv/lib/board.c

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH 2/5] board: qemu: riscv: Remove duplicated board_fdt_blob_setup

[Leo Liang]

On Thu, Feb 27, 2025 at 02:47:30PM +0000, Yao Zi wrote:
> The default version should work for RISC-V QEMU.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  board/emulation/qemu-riscv/qemu-riscv.c | 8 --------
>  1 file changed, 8 deletions(-)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH 3/5] board: starfive: Remove duplicated board_fdt_blob_setup

[Leo Liang]

On Thu, Feb 27, 2025 at 02:47:31PM +0000, Yao Zi wrote:
> The default version should work for Starfive VisionFive 2.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  board/starfive/visionfive2/starfive_visionfive2.c | 10 ----------
>  1 file changed, 10 deletions(-)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH 4/5] board: sifive: Remove duplicated board_fdt_blob_setup

[Leo Liang]

On Thu, Feb 27, 2025 at 02:47:32PM +0000, Yao Zi wrote:
> The default version should work for both SiFive Unmatched and Unleashed.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  board/sifive/unleashed/unleashed.c | 11 -----------
>  board/sifive/unmatched/unmatched.c | 10 ----------
>  2 files changed, 21 deletions(-)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH 5/5] riscv: select OF_HAS_PRIOR_STAGE by default if SBI is enabled

[Leo Liang]

On Thu, Feb 27, 2025 at 02:47:33PM +0000, Yao Zi wrote:
> Availability of RISC-V SBI service implies a prior stage exists. As SBI
> firmware usually passes a FDT to the loaded program, let's select
> OF_HAS_PRIOR_STAGE if SBI is enabled.
> 
> With previously added fallback version of board_fdt_blob_setup, S-Mode
> RISC-V ports use the SBI-provided FDT by default. This covers the most
> common usecase, where a SPL (probably the U-Boot one) selects proper
> devicetree, loads SBI and U-Boot then invokes SBI with the devicetree.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  arch/riscv/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>

Re: [PATCH 5/5] riscv: select OF_HAS_PRIOR_STAGE by default if SBI is enabled

[Leo Liang]

On Thu, Feb 27, 2025 at 02:47:33PM +0000, Yao Zi wrote:
> Availability of RISC-V SBI service implies a prior stage exists. As SBI
> firmware usually passes a FDT to the loaded program, let's select
> OF_HAS_PRIOR_STAGE if SBI is enabled.
> 
> With previously added fallback version of board_fdt_blob_setup, S-Mode
> RISC-V ports use the SBI-provided FDT by default. This covers the most
> common usecase, where a SPL (probably the U-Boot one) selects proper
> devicetree, loads SBI and U-Boot then invokes SBI with the devicetree.
> 
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> ---
>  arch/riscv/Kconfig | 1 +
>  1 file changed, 1 insertion(+)

Hi Yao,

This patch would fail some of the CI tests.
I will take other patches from this patchset first,
and could you take a look at the CI tests issue ?
(https://source.denx.de/u-boot/custodians/u-boot-riscv/-/jobs/1049827)

Best regards,
Leo

Re: [PATCH 5/5] riscv: select OF_HAS_PRIOR_STAGE by default if SBI is enabled

[Yao Zi]

On Thu, Mar 06, 2025 at 07:09:06PM +0800, Leo Liang wrote:
> On Thu, Feb 27, 2025 at 02:47:33PM +0000, Yao Zi wrote:
> > Availability of RISC-V SBI service implies a prior stage exists. As SBI
> > firmware usually passes a FDT to the loaded program, let's select
> > OF_HAS_PRIOR_STAGE if SBI is enabled.
> > 
> > With previously added fallback version of board_fdt_blob_setup, S-Mode
> > RISC-V ports use the SBI-provided FDT by default. This covers the most
> > common usecase, where a SPL (probably the U-Boot one) selects proper
> > devicetree, loads SBI and U-Boot then invokes SBI with the devicetree.
> > 
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> > ---
> >  arch/riscv/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Hi Yao,
> 
> This patch would fail some of the CI tests.
> I will take other patches from this patchset first,
> and could you take a look at the CI tests issue ?
> (https://source.denx.de/u-boot/custodians/u-boot-riscv/-/jobs/1049827)

Sure, will have a check later.

> Best regards,
> Leo

Thanks,
Yao Zi

Re: [PATCH 5/5] riscv: select OF_HAS_PRIOR_STAGE by default if SBI is enabled

[Yao Zi]

On Thu, Mar 06, 2025 at 07:09:06PM +0800, Leo Liang wrote:
> On Thu, Feb 27, 2025 at 02:47:33PM +0000, Yao Zi wrote:
> > Availability of RISC-V SBI service implies a prior stage exists. As SBI
> > firmware usually passes a FDT to the loaded program, let's select
> > OF_HAS_PRIOR_STAGE if SBI is enabled.
> > 
> > With previously added fallback version of board_fdt_blob_setup, S-Mode
> > RISC-V ports use the SBI-provided FDT by default. This covers the most
> > common usecase, where a SPL (probably the U-Boot one) selects proper
> > devicetree, loads SBI and U-Boot then invokes SBI with the devicetree.
> > 
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > Reviewed-by: Leo Yu-Chi Liang <ycliang@andestech.com>
> > ---
> >  arch/riscv/Kconfig | 1 +
> >  1 file changed, 1 insertion(+)
> 
> Hi Yao,
> 
> This patch would fail some of the CI tests.
> I will take other patches from this patchset first,
> and could you take a look at the CI tests issue ?
> (https://source.denx.de/u-boot/custodians/u-boot-riscv/-/jobs/1049827)

After digging further with the failure, I found the series has two
problems,

- OF_HAS_PRIOR_STAGE is an option shared between XPL and proper U-Boot.
  The generic board_fdt_blob_setup() doesn't work in a XPL build,
  corresponding checks should be added.
- The default binman configuration for RISC-V platforms doesn't pack the
  FDTs into the fit image if OF_HAS_PRIOR_BOARD is enabled

Although it doesn't break anything to merge the first four patches for
now, I'd like to rework PATCH 1 and add a check against XPL build in the
new revision. Then we could merge a patch that is technically right and
avoid fixes tags.

Additionally, I found OF_BOARD is actually disabled on Unmatched and
Unleashed and the board_fdt_blob_setup() here is dead code in fact. Thus
the commit message of PATCH 4 should be fixed as well.

> Best regards,
> Leo

Best regards,
Yao Zi