[PATCH 0/5] Handoff bloblist from previous boot stage

[PATCH 0/5] Handoff bloblist from previous boot stage

[Raymond Mao]

This patch set depends on another series:
"[PATCH v3 00/14] Support Firmware Handoff spec via bloblist".

This patch set implements Qemu-Arm platform custom functions to retrieve
the bloblist (aka. Transfer List) from previous loader via boot arguments
when CONFIG_OF_BOARD option is enabled and all boot arguments are compliant to
the register conventions defined in the Firmware Handoff spec v0.9.

Qemu-Arm platform custom function will load the FDT from the bloblist if it
exists.
Otherwise it fallbacks to get the FDT from the specified memory address.

If a platform vendor wish to have different behaviors for loading bloblist
or FDT from the previous boot stage, it is required to implement the custom
functions board_bloblist_from_boot_arg() and board_fdt_blob_setup().

Raymond Mao (5):
  bloblist: add API to check the register conventions
  qemu-arm: Get bloblist from boot arguments
  bloblist: Load the bloblist from the previous loader
  fdt: update the document and Kconfig description
  qemu-arm: get FDT from bloblist

 board/emulation/qemu-arm/Makefile        |  1 +
 board/emulation/qemu-arm/lowlevel_init.S | 19 +++++++
 board/emulation/qemu-arm/qemu-arm.c      | 66 +++++++++++++++++++++++-
 common/bloblist.c                        | 62 +++++++++++++++-------
 configs/qemu_arm64_defconfig             |  3 ++
 doc/develop/devicetree/control.rst       |  6 +--
 dts/Kconfig                              |  7 ++-
 include/bloblist.h                       | 28 ++++++++++
 8 files changed, 167 insertions(+), 25 deletions(-)
 create mode 100644 board/emulation/qemu-arm/lowlevel_init.S

-- 
2.25.1

[PATCH 1/5] bloblist: add API to check the register conventions

[Raymond Mao]

Add bloblist_check_reg_conv() to check whether the bloblist is compliant
to the register conventions defined in Firmware Handoff specification.
This API can be used for all Arm platforms.

Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
---
 common/bloblist.c  | 15 +++++++++++++++
 include/bloblist.h | 12 ++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/common/bloblist.c b/common/bloblist.c
index 625e480f6b..232ca4c6ce 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -542,3 +542,18 @@ int bloblist_maybe_init(void)
 
 	return 0;
 }
+
+int bloblist_check_reg_conv(ulong rfdt, ulong rzero)
+{
+	ulong fdt;
+
+	if (!IS_ENABLED(CONFIG_OF_BOARD))
+		return 0;
+
+	fdt = (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
+
+	if (rfdt != fdt || rzero != 0)
+		return -EIO;
+
+	return 0;
+}
diff --git a/include/bloblist.h b/include/bloblist.h
index 84fc943819..b5d0f147f6 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -461,4 +461,16 @@ static inline int bloblist_maybe_init(void)
 }
 #endif /* BLOBLIST */
 
+/**
+ * bloblist_check_reg_conv() - Check whether the bloblist is compliant to
+ *			       the register conventions according to the
+ *			       Firmware Handoff spec.
+ *
+ * @rfdt:  Register that holds the FDT base address.
+ * @rzero: Register that must be zero.
+ * Return: 0 if OK, -EIO if the bloblist is not compliant to the register
+ *	   conventions.
+ */
+int bloblist_check_reg_conv(ulong rfdt, ulong rzero);
+
 #endif /* __BLOBLIST_H */
-- 
2.25.1

Re: [PATCH 1/5] bloblist: add API to check the register conventions

[Ilias Apalodimas]

Hi Raymond,

On Tue, 19 Dec 2023 at 23:11, Raymond Mao <raymond.mao@linaro.org> wrote:
>
> Add bloblist_check_reg_conv() to check whether the bloblist is compliant
> to the register conventions defined in Firmware Handoff specification.
> This API can be used for all Arm platforms.
>
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> ---
>  common/bloblist.c  | 15 +++++++++++++++
>  include/bloblist.h | 12 ++++++++++++
>  2 files changed, 27 insertions(+)
>
> diff --git a/common/bloblist.c b/common/bloblist.c
> index 625e480f6b..232ca4c6ce 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -542,3 +542,18 @@ int bloblist_maybe_init(void)
>
>         return 0;
>  }
> +
> +int bloblist_check_reg_conv(ulong rfdt, ulong rzero)
> +{
> +       ulong fdt;
> +
> +       if (!IS_ENABLED(CONFIG_OF_BOARD))
> +               return 0;
> +
> +       fdt = (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
> +
> +       if (rfdt != fdt || rzero != 0)
> +               return -EIO;

What is rzero supposed to be doing here? If its only valid value is !0
why arent we checking it earlier?

Thanks
/Ilias
> +
> +       return 0;
> +}
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 84fc943819..b5d0f147f6 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -461,4 +461,16 @@ static inline int bloblist_maybe_init(void)
>  }
>  #endif /* BLOBLIST */
>
> +/**
> + * bloblist_check_reg_conv() - Check whether the bloblist is compliant to
> + *                            the register conventions according to the
> + *                            Firmware Handoff spec.
> + *
> + * @rfdt:  Register that holds the FDT base address.
> + * @rzero: Register that must be zero.
> + * Return: 0 if OK, -EIO if the bloblist is not compliant to the register
> + *        conventions.
> + */
> +int bloblist_check_reg_conv(ulong rfdt, ulong rzero);
> +
>  #endif /* __BLOBLIST_H */
> --
> 2.25.1
>

Re: [PATCH 1/5] bloblist: add API to check the register conventions

[Raymond Mao]

Hi Ilias,

The rzero refers to the boot argument that must be 0 according to the
register conventions of FW Handoff spec.
Yes, you are right. I should check this at the beginning of the function.

Thanks and regards,
Raymond

On Wed, 20 Dec 2023 at 07:54, Ilias Apalodimas <ilias.apalodimas@linaro.org>
wrote:

> Hi Raymond,
>
> On Tue, 19 Dec 2023 at 23:11, Raymond Mao <raymond.mao@linaro.org> wrote:
> >
> > Add bloblist_check_reg_conv() to check whether the bloblist is compliant
> > to the register conventions defined in Firmware Handoff specification.
> > This API can be used for all Arm platforms.
> >
> > Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> > ---
> >  common/bloblist.c  | 15 +++++++++++++++
> >  include/bloblist.h | 12 ++++++++++++
> >  2 files changed, 27 insertions(+)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index 625e480f6b..232ca4c6ce 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -542,3 +542,18 @@ int bloblist_maybe_init(void)
> >
> >         return 0;
> >  }
> > +
> > +int bloblist_check_reg_conv(ulong rfdt, ulong rzero)
> > +{
> > +       ulong fdt;
> > +
> > +       if (!IS_ENABLED(CONFIG_OF_BOARD))
> > +               return 0;
> > +
> > +       fdt = (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
> > +
> > +       if (rfdt != fdt || rzero != 0)
> > +               return -EIO;
>
> What is rzero supposed to be doing here? If its only valid value is !0
> why arent we checking it earlier?
>
> Thanks
> /Ilias
> > +
> > +       return 0;
> > +}
> > diff --git a/include/bloblist.h b/include/bloblist.h
> > index 84fc943819..b5d0f147f6 100644
> > --- a/include/bloblist.h
> > +++ b/include/bloblist.h
> > @@ -461,4 +461,16 @@ static inline int bloblist_maybe_init(void)
> >  }
> >  #endif /* BLOBLIST */
> >
> > +/**
> > + * bloblist_check_reg_conv() - Check whether the bloblist is compliant
> to
> > + *                            the register conventions according to the
> > + *                            Firmware Handoff spec.
> > + *
> > + * @rfdt:  Register that holds the FDT base address.
> > + * @rzero: Register that must be zero.
> > + * Return: 0 if OK, -EIO if the bloblist is not compliant to the
> register
> > + *        conventions.
> > + */
> > +int bloblist_check_reg_conv(ulong rfdt, ulong rzero);
> > +
> >  #endif /* __BLOBLIST_H */
> > --
> > 2.25.1
> >
>

[PATCH 2/5] qemu-arm: Get bloblist from boot arguments

[Raymond Mao]

Add platform custom function to get bloblist from boot arguments.
Check whether boot arguments aligns with the register conventions
defined in FW Handoff spec v0.9.
Add bloblist related options into qemu default config.

Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
---
 board/emulation/qemu-arm/Makefile        |  1 +
 board/emulation/qemu-arm/lowlevel_init.S | 19 +++++++++
 board/emulation/qemu-arm/qemu-arm.c      | 51 ++++++++++++++++++++++++
 configs/qemu_arm64_defconfig             |  3 ++
 4 files changed, 74 insertions(+)
 create mode 100644 board/emulation/qemu-arm/lowlevel_init.S

diff --git a/board/emulation/qemu-arm/Makefile b/board/emulation/qemu-arm/Makefile
index a22d1237ff..0eb262e4e2 100644
--- a/board/emulation/qemu-arm/Makefile
+++ b/board/emulation/qemu-arm/Makefile
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0+
 
 obj-y	+= qemu-arm.o
+obj-y	+= lowlevel_init.o
diff --git a/board/emulation/qemu-arm/lowlevel_init.S b/board/emulation/qemu-arm/lowlevel_init.S
new file mode 100644
index 0000000000..d72d7c938a
--- /dev/null
+++ b/board/emulation/qemu-arm/lowlevel_init.S
@@ -0,0 +1,19 @@
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Copyright (c) 2023, Linaro Limited
+ */
+
+#include <config.h>
+
+.global save_boot_params
+save_boot_params:
+#ifdef CONFIG_ARM64
+	adr x9, qemu_saved_args
+	stp x0, x1, [x9]
+	/* Increment the address by 16 bytes for the next pair of values */
+	stp x2, x3, [x9, #16]
+#else
+	ldr r12, =qemu_saved_args
+	stm r12, {r0, r1, r2, r3}
+#endif
+	b	save_boot_params_ret
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
index 942f1fff57..72e0a97567 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -4,6 +4,7 @@
  */
 
 #include <common.h>
+#include <bloblist.h>
 #include <cpu_func.h>
 #include <dm.h>
 #include <efi.h>
@@ -102,6 +103,14 @@ static struct mm_region qemu_arm64_mem_map[] = {
 struct mm_region *mem_map = qemu_arm64_mem_map;
 #endif
 
+/* Boot parameters saved from lowlevel_init.S */
+struct {
+	unsigned long arg0;
+	unsigned long arg1;
+	unsigned long arg2;
+	unsigned long arg3;
+} qemu_saved_args __section(".data");
+
 int board_init(void)
 {
 	return 0;
@@ -144,6 +153,48 @@ void *board_fdt_blob_setup(int *err)
 	return (void *)CFG_SYS_SDRAM_BASE;
 }
 
+int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
+{
+	int ret = -ENOENT;
+	unsigned long reg_fdt;
+	unsigned long reg_zero;
+
+	if (!IS_ENABLED(CONFIG_OF_BOARD) || !IS_ENABLED(CONFIG_BLOBLIST))
+		return -ENOENT;
+
+	ret = bloblist_check(qemu_saved_args.arg3, 0);
+	if (ret)
+		return ret;
+
+	if (gd->bloblist->total_size > size) {
+		gd->bloblist = NULL; /* Reset the gd bloblist pointer */
+		log_err("Board bloblist total size:%d, reserved size:%ld\n",
+			gd->bloblist->total_size, size);
+		return -ENOSPC;
+	}
+
+	if (IS_ENABLED(CONFIG_ARM64)) {
+		reg_fdt = qemu_saved_args.arg0;
+		reg_zero = qemu_saved_args.arg2;
+	} else {
+		reg_fdt = qemu_saved_args.arg2;
+		reg_zero = qemu_saved_args.arg0;
+	}
+	/* Check the register conventions */
+	ret = bloblist_check_reg_conv(reg_fdt, reg_zero);
+
+	if (ret) {
+		gd->bloblist = NULL;  /* Reset the gd bloblist pointer */
+	} else {
+		/* Relocate the bloblist to the fixed address */
+		bloblist_reloc((void *)addr, gd->bloblist->total_size,
+			       gd->bloblist, gd->bloblist->total_size);
+		gd->bloblist = (struct bloblist_hdr *)addr;
+	}
+
+	return ret;
+}
+
 void enable_caches(void)
 {
 	 icache_enable();
diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
index c010c25a92..418f48001c 100644
--- a/configs/qemu_arm64_defconfig
+++ b/configs/qemu_arm64_defconfig
@@ -69,3 +69,6 @@ CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
 CONFIG_SEMIHOSTING=y
 CONFIG_TPM=y
+CONFIG_BLOBLIST=y
+CONFIG_BLOBLIST_ADDR=0x40004000
+CONFIG_BLOBLIST_SIZE=0x4000
-- 
2.25.1

Re: [PATCH 2/5] qemu-arm: Get bloblist from boot arguments

[Michal Simek]

On 12/19/23 22:11, Raymond Mao wrote:
> Add platform custom function to get bloblist from boot arguments.
> Check whether boot arguments aligns with the register conventions
> defined in FW Handoff spec v0.9.
> Add bloblist related options into qemu default config.
> 
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> ---
>   board/emulation/qemu-arm/Makefile        |  1 +
>   board/emulation/qemu-arm/lowlevel_init.S | 19 +++++++++
>   board/emulation/qemu-arm/qemu-arm.c      | 51 ++++++++++++++++++++++++
>   configs/qemu_arm64_defconfig             |  3 ++
>   4 files changed, 74 insertions(+)
>   create mode 100644 board/emulation/qemu-arm/lowlevel_init.S
> 
> diff --git a/board/emulation/qemu-arm/Makefile b/board/emulation/qemu-arm/Makefile
> index a22d1237ff..0eb262e4e2 100644
> --- a/board/emulation/qemu-arm/Makefile
> +++ b/board/emulation/qemu-arm/Makefile
> @@ -1,3 +1,4 @@
>   # SPDX-License-Identifier: GPL-2.0+
>   
>   obj-y	+= qemu-arm.o
> +obj-y	+= lowlevel_init.o
> diff --git a/board/emulation/qemu-arm/lowlevel_init.S b/board/emulation/qemu-arm/lowlevel_init.S
> new file mode 100644
> index 0000000000..d72d7c938a
> --- /dev/null
> +++ b/board/emulation/qemu-arm/lowlevel_init.S
> @@ -0,0 +1,19 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include <config.h>
> +
> +.global save_boot_params
> +save_boot_params:
> +#ifdef CONFIG_ARM64
> +	adr x9, qemu_saved_args
> +	stp x0, x1, [x9]
> +	/* Increment the address by 16 bytes for the next pair of values */
> +	stp x2, x3, [x9, #16]

There is specified how transfer list should be passed from one stage to another.
Which pretty much saying that when bloblist is enabled this is what you can 
expect on arm64.
Why not to add it directly to?
arch/arm/cpu/armv8/start.S

WEAK(save_boot_params)
#if bloblist
	(your code)
#endif
        b       save_boot_params_ret    /* back to my caller */
ENDPROC(save_boot_params)

because pretty much all platforms which want to switch to it will just c&p your 
code.

that being said qemu_saved_args can become just generic save_args and you can 
also allocate space in .data section in this file.

> +#else
> +	ldr r12, =qemu_saved_args
> +	stm r12, {r0, r1, r2, r3}

And this part should go to arch/arm/cpu/armv7/start.S

M

Re: [PATCH 2/5] qemu-arm: Get bloblist from boot arguments

[Raymond Mao]

Hi Michal,
Yes, I can put this part as armv8/v7 common functions.

Thanks and regards,
Raymond

On Wed, 20 Dec 2023 at 06:03, Michal Simek <michal.simek@amd.com> wrote:

>
>
> On 12/19/23 22:11, Raymond Mao wrote:
> > Add platform custom function to get bloblist from boot arguments.
> > Check whether boot arguments aligns with the register conventions
> > defined in FW Handoff spec v0.9.
> > Add bloblist related options into qemu default config.
> >
> > Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> > ---
> >   board/emulation/qemu-arm/Makefile        |  1 +
> >   board/emulation/qemu-arm/lowlevel_init.S | 19 +++++++++
> >   board/emulation/qemu-arm/qemu-arm.c      | 51 ++++++++++++++++++++++++
> >   configs/qemu_arm64_defconfig             |  3 ++
> >   4 files changed, 74 insertions(+)
> >   create mode 100644 board/emulation/qemu-arm/lowlevel_init.S
> >
> > diff --git a/board/emulation/qemu-arm/Makefile
> b/board/emulation/qemu-arm/Makefile
> > index a22d1237ff..0eb262e4e2 100644
> > --- a/board/emulation/qemu-arm/Makefile
> > +++ b/board/emulation/qemu-arm/Makefile
> > @@ -1,3 +1,4 @@
> >   # SPDX-License-Identifier: GPL-2.0+
> >
> >   obj-y       += qemu-arm.o
> > +obj-y        += lowlevel_init.o
> > diff --git a/board/emulation/qemu-arm/lowlevel_init.S
> b/board/emulation/qemu-arm/lowlevel_init.S
> > new file mode 100644
> > index 0000000000..d72d7c938a
> > --- /dev/null
> > +++ b/board/emulation/qemu-arm/lowlevel_init.S
> > @@ -0,0 +1,19 @@
> > +/* SPDX-License-Identifier: BSD-2-Clause */
> > +/*
> > + * Copyright (c) 2023, Linaro Limited
> > + */
> > +
> > +#include <config.h>
> > +
> > +.global save_boot_params
> > +save_boot_params:
> > +#ifdef CONFIG_ARM64
> > +     adr x9, qemu_saved_args
> > +     stp x0, x1, [x9]
> > +     /* Increment the address by 16 bytes for the next pair of values */
> > +     stp x2, x3, [x9, #16]
>
> There is specified how transfer list should be passed from one stage to
> another.
> Which pretty much saying that when bloblist is enabled this is what you
> can
> expect on arm64.
> Why not to add it directly to?
> arch/arm/cpu/armv8/start.S
>
> WEAK(save_boot_params)
> #if bloblist
>         (your code)
> #endif
>         b       save_boot_params_ret    /* back to my caller */
> ENDPROC(save_boot_params)
>
> because pretty much all platforms which want to switch to it will just c&p
> your
> code.
>
> that being said qemu_saved_args can become just generic save_args and you
> can
> also allocate space in .data section in this file.
>
> > +#else
> > +     ldr r12, =qemu_saved_args
> > +     stm r12, {r0, r1, r2, r3}
>
> And this part should go to arch/arm/cpu/armv7/start.S
>
> M
>

[PATCH 3/5] bloblist: Load the bloblist from the previous loader

[Raymond Mao]

During bloblist initialization, when CONFIG_OF_BOARD is defined,
invoke the platform custom function to load the bloblist via boot
arguments from the previous loader.
If the bloblist exists, copy it into the fixed bloblist memory region.

Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
---
 common/bloblist.c  | 47 ++++++++++++++++++++++++++++------------------
 include/bloblist.h | 16 ++++++++++++++++
 2 files changed, 45 insertions(+), 18 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 232ca4c6ce..c89f7a1554 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -486,31 +486,38 @@ int bloblist_init(void)
 	bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED);
 	int ret = -ENOENT;
 	ulong addr, size;
-	bool expected;
-
-	/**
-	 * We don't expect to find an existing bloblist in the first phase of
-	 * U-Boot that runs. Also we have no way to receive the address of an
-	 * allocated bloblist from a previous stage, so it must be at a fixed
+	/*
+	 * If U-Boot is not in the first phase, an existing bloblist must be
+	 * at a fixed address.
+	 */
+	bool from_addr = fixed && !u_boot_first_phase();
+	/*
+	 * If U-Boot is in the first phase that a board specific routine should
+	 * install the bloblist passed from previous loader to this fixed
 	 * address.
 	 */
-	expected = fixed && !u_boot_first_phase();
+	bool from_board = fixed && IS_ENABLED(CONFIG_OF_BOARD) &&
+			  u_boot_first_phase();
+
 	if (spl_prev_phase() == PHASE_TPL && !IS_ENABLED(CONFIG_TPL_BLOBLIST))
-		expected = false;
+		from_addr = false;
 	if (fixed)
 		addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED,
 				      CONFIG_BLOBLIST_ADDR);
 	size = CONFIG_BLOBLIST_SIZE;
-	if (expected) {
+
+	if (from_board)
+		ret = board_bloblist_from_boot_arg(addr, size);
+	else if (from_addr)
 		ret = bloblist_check(addr, size);
-		if (ret) {
-			log_warning("Expected bloblist at %lx not found (err=%d)\n",
-				    addr, ret);
-		} else {
-			/* Get the real size, if it is not what we expected */
-			size = gd->bloblist->total_size;
-		}
-	}
+
+	if (ret)
+		log_warning("Bloblist at %lx not found (err=%d)\n",
+			    addr, ret);
+	else
+		/* Get the real size */
+		size = gd->bloblist->total_size;
+
 	if (ret) {
 		if (CONFIG_IS_ENABLED(BLOBLIST_ALLOC)) {
 			void *ptr = memalign(BLOBLIST_ALIGN, size);
@@ -519,7 +526,8 @@ int bloblist_init(void)
 				return log_msg_ret("alloc", -ENOMEM);
 			addr = map_to_sysmem(ptr);
 		} else if (!fixed) {
-			return log_msg_ret("!fixed", ret);
+			return log_msg_ret("BLOBLIST_FIXED is not enabled",
+					   ret);
 		}
 		log_debug("Creating new bloblist size %lx at %lx\n", size,
 			  addr);
@@ -532,6 +540,9 @@ int bloblist_init(void)
 		return log_msg_ret("ini", ret);
 	gd->flags |= GD_FLG_BLOBLIST_READY;
 
+	bloblist_show_stats();
+	bloblist_show_list();
+
 	return 0;
 }
 
diff --git a/include/bloblist.h b/include/bloblist.h
index b5d0f147f6..f5c623133d 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -445,6 +445,22 @@ void bloblist_reloc(void *to, uint to_size, void *from, uint from_size);
  */
 int bloblist_init(void);
 
+#if (IS_ENABLED(CONFIG_ARCH_QEMU) && IS_ENABLED(CONFIG_ARM))
+/* Board custom function for qemu-arm */
+int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size);
+#else
+/*
+ * A board need to implement this custom function if it needs to retrieve
+ * bloblist from a previous loader
+ */
+static inline
+int board_bloblist_from_boot_arg(unsigned long __always_unused addr,
+				 unsigned long __always_unused size)
+{
+	return -1;
+}
+#endif
+
 #if CONFIG_IS_ENABLED(BLOBLIST)
 /**
  * bloblist_maybe_init() - Init the bloblist system if not already done
-- 
2.25.1

Re: [PATCH 3/5] bloblist: Load the bloblist from the previous loader

[Michal Simek]

On 12/19/23 22:11, Raymond Mao wrote:
> During bloblist initialization, when CONFIG_OF_BOARD is defined,
> invoke the platform custom function to load the bloblist via boot
> arguments from the previous loader.
> If the bloblist exists, copy it into the fixed bloblist memory region.
> 
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> ---
>   common/bloblist.c  | 47 ++++++++++++++++++++++++++++------------------
>   include/bloblist.h | 16 ++++++++++++++++
>   2 files changed, 45 insertions(+), 18 deletions(-)
> 
> diff --git a/common/bloblist.c b/common/bloblist.c
> index 232ca4c6ce..c89f7a1554 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -486,31 +486,38 @@ int bloblist_init(void)
>   	bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED);
>   	int ret = -ENOENT;
>   	ulong addr, size;
> -	bool expected;
> -
> -	/**
> -	 * We don't expect to find an existing bloblist in the first phase of
> -	 * U-Boot that runs. Also we have no way to receive the address of an
> -	 * allocated bloblist from a previous stage, so it must be at a fixed
> +	/*
> +	 * If U-Boot is not in the first phase, an existing bloblist must be
> +	 * at a fixed address.
> +	 */
> +	bool from_addr = fixed && !u_boot_first_phase();
> +	/*
> +	 * If U-Boot is in the first phase that a board specific routine should
> +	 * install the bloblist passed from previous loader to this fixed
>   	 * address.
>   	 */
> -	expected = fixed && !u_boot_first_phase();
> +	bool from_board = fixed && IS_ENABLED(CONFIG_OF_BOARD) &&
> +			  u_boot_first_phase();
> +
>   	if (spl_prev_phase() == PHASE_TPL && !IS_ENABLED(CONFIG_TPL_BLOBLIST))
> -		expected = false;
> +		from_addr = false;
>   	if (fixed)
>   		addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED,
>   				      CONFIG_BLOBLIST_ADDR);
>   	size = CONFIG_BLOBLIST_SIZE;
> -	if (expected) {
> +
> +	if (from_board)
> +		ret = board_bloblist_from_boot_arg(addr, size);
> +	else if (from_addr)
>   		ret = bloblist_check(addr, size);
> -		if (ret) {
> -			log_warning("Expected bloblist at %lx not found (err=%d)\n",
> -				    addr, ret);
> -		} else {
> -			/* Get the real size, if it is not what we expected */
> -			size = gd->bloblist->total_size;
> -		}
> -	}
> +
> +	if (ret)
> +		log_warning("Bloblist at %lx not found (err=%d)\n",
> +			    addr, ret);
> +	else
> +		/* Get the real size */
> +		size = gd->bloblist->total_size;
> +
>   	if (ret) {
>   		if (CONFIG_IS_ENABLED(BLOBLIST_ALLOC)) {
>   			void *ptr = memalign(BLOBLIST_ALIGN, size);
> @@ -519,7 +526,8 @@ int bloblist_init(void)
>   				return log_msg_ret("alloc", -ENOMEM);
>   			addr = map_to_sysmem(ptr);
>   		} else if (!fixed) {
> -			return log_msg_ret("!fixed", ret);
> +			return log_msg_ret("BLOBLIST_FIXED is not enabled",
> +					   ret);
>   		}
>   		log_debug("Creating new bloblist size %lx at %lx\n", size,
>   			  addr);
> @@ -532,6 +540,9 @@ int bloblist_init(void)
>   		return log_msg_ret("ini", ret);
>   	gd->flags |= GD_FLG_BLOBLIST_READY;
>   
> +	bloblist_show_stats();
> +	bloblist_show_list();
> +
>   	return 0;
>   }
>   
> diff --git a/include/bloblist.h b/include/bloblist.h
> index b5d0f147f6..f5c623133d 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -445,6 +445,22 @@ void bloblist_reloc(void *to, uint to_size, void *from, uint from_size);
>    */
>   int bloblist_init(void);
>   
> +#if (IS_ENABLED(CONFIG_ARCH_QEMU) && IS_ENABLED(CONFIG_ARM))

How do you think that others start to use it?
Adding that platforms here? I don't think it is scalable.

Thanks,
Michal

Re: [PATCH 3/5] bloblist: Load the bloblist from the previous loader

[Raymond Mao]

Hi Michal,

Other vendors should add their own custom function under the
"./board/<board-name>" directory.
The inline function here is just to avoid the building errors on other
platforms which do not yet implement the custom function.
Otherwise we have to create a function "board_bloblist_from_boot_arg()"
under all boards with this patch.

Thanks and regards,
Raymond


On Wed, 20 Dec 2023 at 05:57, Michal Simek <michal.simek@amd.com> wrote:

>
>
> On 12/19/23 22:11, Raymond Mao wrote:
> > During bloblist initialization, when CONFIG_OF_BOARD is defined,
> > invoke the platform custom function to load the bloblist via boot
> > arguments from the previous loader.
> > If the bloblist exists, copy it into the fixed bloblist memory region.
> >
> > Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> > ---
> >   common/bloblist.c  | 47 ++++++++++++++++++++++++++++------------------
> >   include/bloblist.h | 16 ++++++++++++++++
> >   2 files changed, 45 insertions(+), 18 deletions(-)
> >
> > diff --git a/common/bloblist.c b/common/bloblist.c
> > index 232ca4c6ce..c89f7a1554 100644
> > --- a/common/bloblist.c
> > +++ b/common/bloblist.c
> > @@ -486,31 +486,38 @@ int bloblist_init(void)
> >       bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED);
> >       int ret = -ENOENT;
> >       ulong addr, size;
> > -     bool expected;
> > -
> > -     /**
> > -      * We don't expect to find an existing bloblist in the first phase
> of
> > -      * U-Boot that runs. Also we have no way to receive the address of
> an
> > -      * allocated bloblist from a previous stage, so it must be at a
> fixed
> > +     /*
> > +      * If U-Boot is not in the first phase, an existing bloblist must
> be
> > +      * at a fixed address.
> > +      */
> > +     bool from_addr = fixed && !u_boot_first_phase();
> > +     /*
> > +      * If U-Boot is in the first phase that a board specific routine
> should
> > +      * install the bloblist passed from previous loader to this fixed
> >        * address.
> >        */
> > -     expected = fixed && !u_boot_first_phase();
> > +     bool from_board = fixed && IS_ENABLED(CONFIG_OF_BOARD) &&
> > +                       u_boot_first_phase();
> > +
> >       if (spl_prev_phase() == PHASE_TPL &&
> !IS_ENABLED(CONFIG_TPL_BLOBLIST))
> > -             expected = false;
> > +             from_addr = false;
> >       if (fixed)
> >               addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED,
> >                                     CONFIG_BLOBLIST_ADDR);
> >       size = CONFIG_BLOBLIST_SIZE;
> > -     if (expected) {
> > +
> > +     if (from_board)
> > +             ret = board_bloblist_from_boot_arg(addr, size);
> > +     else if (from_addr)
> >               ret = bloblist_check(addr, size);
> > -             if (ret) {
> > -                     log_warning("Expected bloblist at %lx not found
> (err=%d)\n",
> > -                                 addr, ret);
> > -             } else {
> > -                     /* Get the real size, if it is not what we
> expected */
> > -                     size = gd->bloblist->total_size;
> > -             }
> > -     }
> > +
> > +     if (ret)
> > +             log_warning("Bloblist at %lx not found (err=%d)\n",
> > +                         addr, ret);
> > +     else
> > +             /* Get the real size */
> > +             size = gd->bloblist->total_size;
> > +
> >       if (ret) {
> >               if (CONFIG_IS_ENABLED(BLOBLIST_ALLOC)) {
> >                       void *ptr = memalign(BLOBLIST_ALIGN, size);
> > @@ -519,7 +526,8 @@ int bloblist_init(void)
> >                               return log_msg_ret("alloc", -ENOMEM);
> >                       addr = map_to_sysmem(ptr);
> >               } else if (!fixed) {
> > -                     return log_msg_ret("!fixed", ret);
> > +                     return log_msg_ret("BLOBLIST_FIXED is not enabled",
> > +                                        ret);
> >               }
> >               log_debug("Creating new bloblist size %lx at %lx\n", size,
> >                         addr);
> > @@ -532,6 +540,9 @@ int bloblist_init(void)
> >               return log_msg_ret("ini", ret);
> >       gd->flags |= GD_FLG_BLOBLIST_READY;
> >
> > +     bloblist_show_stats();
> > +     bloblist_show_list();
> > +
> >       return 0;
> >   }
> >
> > diff --git a/include/bloblist.h b/include/bloblist.h
> > index b5d0f147f6..f5c623133d 100644
> > --- a/include/bloblist.h
> > +++ b/include/bloblist.h
> > @@ -445,6 +445,22 @@ void bloblist_reloc(void *to, uint to_size, void
> *from, uint from_size);
> >    */
> >   int bloblist_init(void);
> >
> > +#if (IS_ENABLED(CONFIG_ARCH_QEMU) && IS_ENABLED(CONFIG_ARM))
>
> How do you think that others start to use it?
> Adding that platforms here? I don't think it is scalable.
>
> Thanks,
> Michal
>

Re: [PATCH 3/5] bloblist: Load the bloblist from the previous loader

[Michal Simek]

Hi Raymond,

On 12/20/23 15:53, Raymond Mao wrote:
> Hi Michal,
> 

please avoid top posting.

> Other vendors should add their own custom function under the 
> "./board/<board-name>" directory.
> The inline function here is just to avoid the building errors on other platforms 
> which do not yet implement the custom function.
> Otherwise we have to create a function "board_bloblist_from_boot_arg()" under 
> all boards with this patch.

pretty much platforms which enables bloblist via Kconfig want to start to use 
this functionality and start to get information out of it.
It means I think dependency is on BLOBLIST. Simon is using it for sandbox that's 
why I can understand that you want to limit it to ARM or ARM64 or both.
But I don't think you should connect it to any specific ARM arch like ARCH_QEMU.

Thanks,
Michal

Re: [PATCH 3/5] bloblist: Load the bloblist from the previous loader

[Raymond Mao]

Hi Michal,

On Wed, 20 Dec 2023 at 11:41, Michal Simek <michal.simek@amd.com> wrote:

> Hi Raymond,
>
> On 12/20/23 15:53, Raymond Mao wrote:
> > Hi Michal,
> >
>
> please avoid top posting.
>
> > Other vendors should add their own custom function under the
> > "./board/<board-name>" directory.
> > The inline function here is just to avoid the building errors on other
> platforms
> > which do not yet implement the custom function.
> > Otherwise we have to create a function "board_bloblist_from_boot_arg()"
> under
> > all boards with this patch.
>
> pretty much platforms which enables bloblist via Kconfig want to start to
> use
> this functionality and start to get information out of it.
> It means I think dependency is on BLOBLIST. Simon is using it for sandbox
> that's
> why I can understand that you want to limit it to ARM or ARM64 or both.
> But I don't think you should connect it to any specific ARM arch like
> ARCH_QEMU.
>
The reason I have to limit it with ARCH_QEMU in this patch is due to the
fact that currently
only board "qemu-arm" has implemented the required platform custom function
"board_bloblist_from_boot_arg()".
We can remove this limit and make it depend on the BLOBLIST only when all
arm boards
implement the required function.
But at least now, with this patch, we need it under ARCH_QEMU, otherwise we
will have
linking errors when BLOBLIST is enabled on a board which is not "qemu-arm".
For the future, I agree, this only depends on ARM, BLOBLIST and OF_BOARD.

Thanks and regards,
Raymond

[PATCH 4/5] fdt: update the document and Kconfig description

[Raymond Mao]

Update the document and Kconfig to describe the behavior of board
specific custom functions when CONFIG_OF_BOARD is defined.

Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
---
 doc/develop/devicetree/control.rst | 6 +++---
 dts/Kconfig                        | 7 +++++--
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst
index cbb65c9b17..e061f4e812 100644
--- a/doc/develop/devicetree/control.rst
+++ b/doc/develop/devicetree/control.rst
@@ -104,9 +104,9 @@ in u-boot.bin so you can still just flash u-boot.bin onto your board. If you are
 using CONFIG_SPL_FRAMEWORK, then u-boot.img will be built to include the device
 tree binary.
 
-If CONFIG_OF_BOARD is defined, a board-specific routine will provide the
-devicetree at runtime, for example if an earlier bootloader stage creates
-it and passes it to U-Boot.
+If CONFIG_OF_BOARD is defined, board-specific routines will provide the
+bloblist and devicetree at runtime, for example if an earlier bootloader stage
+creates it and passes it to U-Boot.
 
 If CONFIG_SANDBOX is defined, then it will be read from a file on
 startup. Use the -d flag to U-Boot to specify the file to read, -D for the
diff --git a/dts/Kconfig b/dts/Kconfig
index 00c0aeff89..12d61dc748 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -110,8 +110,11 @@ config OF_BOARD
 	default y if SANDBOX || OF_HAS_PRIOR_STAGE
 	help
 	  If this option is enabled, the device tree is provided at runtime by
-	  a custom function called board_fdt_blob_setup(). The board must
-	  implement this function if it wishes to provide special behaviour.
+	  a custom function called board_fdt_blob_setup().
+	  If this option is enabled, bloblist is provided at runtime by a
+	  custom function called board_bloblist_from_boot_arg().
+	  The board must implement these functions if it wishes to provide
+	  special behaviour.
 
 	  With this option, the device tree build by U-Boot may be overridden or
 	  ignored. See OF_HAS_PRIOR_STAGE.
-- 
2.25.1

[PATCH 5/5] qemu-arm: get FDT from bloblist

[Raymond Mao]

Get devicetree from a bloblist if it exists.
If not, fallback to get FDT from the specified memory address.

Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
---
 board/emulation/qemu-arm/qemu-arm.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
index 72e0a97567..455c042a9a 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -148,9 +148,20 @@ int dram_init_banksize(void)
 
 void *board_fdt_blob_setup(int *err)
 {
+	void *fdt = NULL;
 	*err = 0;
-	/* QEMU loads a generated DTB for us at the start of RAM. */
-	return (void *)CFG_SYS_SDRAM_BASE;
+
+	/* Check if a DTB exists in bloblist */
+	if (IS_ENABLED(CONFIG_BLOBLIST)) {
+		if (bloblist_maybe_init())
+			return (void *)CFG_SYS_SDRAM_BASE;
+		fdt = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
+	}
+	if (!fdt)
+		/* QEMU loads a generated DTB for us at the start of RAM. */
+		return (void *)CFG_SYS_SDRAM_BASE;
+	else
+		return fdt;
 }
 
 int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
-- 
2.25.1

Re: [PATCH 5/5] qemu-arm: get FDT from bloblist

[Michal Simek]

On 12/19/23 22:11, Raymond Mao wrote:
> Get devicetree from a bloblist if it exists.
> If not, fallback to get FDT from the specified memory address.
> 
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> ---
>   board/emulation/qemu-arm/qemu-arm.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index 72e0a97567..455c042a9a 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -148,9 +148,20 @@ int dram_init_banksize(void)
>   
>   void *board_fdt_blob_setup(int *err)
>   {
> +	void *fdt = NULL;
>   	*err = 0;
> -	/* QEMU loads a generated DTB for us at the start of RAM. */
> -	return (void *)CFG_SYS_SDRAM_BASE;
> +
> +	/* Check if a DTB exists in bloblist */
> +	if (IS_ENABLED(CONFIG_BLOBLIST)) {
> +		if (bloblist_maybe_init())
> +			return (void *)CFG_SYS_SDRAM_BASE;
> +		fdt = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);

why not just.

return bloblist_find(...);

> +	}
> +	if (!fdt)

And you can ignore this if here.


> +		/* QEMU loads a generated DTB for us at the start of RAM. */
> +		return (void *)CFG_SYS_SDRAM_BASE;
> +	else
> +		return fdt;
>   }
>   
>   int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)

M

Re: [PATCH 5/5] qemu-arm: get FDT from bloblist

[Ilias Apalodimas]

On Tue, Dec 19, 2023 at 01:11:12PM -0800, Raymond Mao wrote:
> Get devicetree from a bloblist if it exists.
> If not, fallback to get FDT from the specified memory address.
>
> Signed-off-by: Raymond Mao <raymond.mao@linaro.org>
> ---
>  board/emulation/qemu-arm/qemu-arm.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c
> index 72e0a97567..455c042a9a 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -148,9 +148,20 @@ int dram_init_banksize(void)
>
>  void *board_fdt_blob_setup(int *err)
>  {
> +	void *fdt = NULL;
>  	*err = 0;
> -	/* QEMU loads a generated DTB for us at the start of RAM. */
> -	return (void *)CFG_SYS_SDRAM_BASE;
> +
> +	/* Check if a DTB exists in bloblist */
> +	if (IS_ENABLED(CONFIG_BLOBLIST)) {
> +		if (bloblist_maybe_init())
> +			return (void *)CFG_SYS_SDRAM_BASE;
> +		fdt = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);

if (!bloblist_maybe_init())
    fdt = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);

and you wont need the explicit return, since the code below will catch it

> +	}
> +	if (!fdt)
> +		/* QEMU loads a generated DTB for us at the start of RAM. */
> +		return (void *)CFG_SYS_SDRAM_BASE;
> +	else

and you dont need the else here either

Thanks
/Ilias
> +		return fdt;
>  }
>
>  int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
> --
> 2.25.1
>