Re: [PATCH v3 7/9] bloblist: Load the bloblist from the previous loader

2023-12-26 Thread Simon Glass
Hi Raymond,

On Fri, Dec 22, 2023 at 9:32 PM 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 
> ---
>  common/bloblist.c  | 47 --
>  include/bloblist.h | 16 
>  2 files changed, 45 insertions(+), 18 deletions(-)
>

We should not be using OF_BOARD for this...particularly as I
understood our intent was to make it go away.

For now I would like to have OF_BLOBLIST, with OF_BOARD being the
fallback for when we need board-specific things. See [1]. It is also
very important that U-Boot reports that the DT came from a bloblist.

Perhaps I should send just that patch so that Ilias and I can figure it out.

> diff --git a/common/bloblist.c b/common/bloblist.c
> index 5ad1db137a..e66afcde64 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -492,31 +492,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.

BTW there is now BLOBLIST_ALLOC

> +*/
> +   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);
> @@ -525,7 +532,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);
> @@ -538,6 +546,9 @@ int bloblist_init(void)
> return log_msg_ret("ini", ret);
> gd->flags |= GD_FLG_BLOBLIST_READY;
>
> +   bloblist_show_stats();
> +   bloblist_show_list();

Can you put those two lines behind a BLOBLIST_DEBUG option, perhaps?

> +
> return 0;
>  }
>
> diff --git a/include/bloblist.h b/include/bloblist.h
> index c1dab11f78..2f4246576e 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -445,6 +445,22 @@ int bloblist_reloc(void *to, uint to_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 

[PATCH v3 7/9] bloblist: Load the bloblist from the previous loader

2023-12-22 Thread 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 
---
 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 5ad1db137a..e66afcde64 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -492,31 +492,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);
@@ -525,7 +532,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);
@@ -538,6 +546,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 c1dab11f78..2f4246576e 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -445,6 +445,22 @@ int bloblist_reloc(void *to, uint to_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