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

[Simon Glass]

Hi Raymond,

On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao <raymond.mao at linaro.org> 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 at linaro.org>
> ---
>  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 __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
>

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/patch/20230924192536.1812799-37-sjg@chromium.org/