[PATCH 28/29] Revert "bloblist: Load the bloblist from the previous loader"

[Raymond Mao]

+CC Ilias,

Hi Simon,

On Tue, 4 Feb 2025 at 20:57, Simon Glass <sjg at chromium.org> wrote:
>
> The logic of this has become too confusing.
>
> The primary issue with the patch is that U-Boot needs to set up a
> bloblist in the first phase where BLOBLIST is enabled. Subsequent
> phases can then use that bloblist.
>
> But the first phase of U-Boot cannot assume that one exists.
>
> Reverting this commit seems like a better starting point for getting
> things working for all use-cases.
>
> This reverts commit 66131310d8ff1ba228f989b41bd8812f43be41c3.
>
> https://lore.kernel.org/u-boot/CAPnjgZ3hMHtiH=f5ZKXNniOfV_-vFryq1Gn7QZ5hKU8Wjo8igw@mail.gmail.com/
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>

If my understanding is correct, you want to add some logic to control
when the U-Boot should or should not get the bloblist from the
existing register argument.
But xferlist_from_boot_arg() should be called when a valid register
argument is there, I didn't see this in your patch.
Maybe you plan to do this with other patch series, but simply
reverting this results in a breaking of handoff policy and the
firmware handoff won't work.

Regards,
Raymond

>  common/bloblist.c  | 64 ++++++++++++++--------------------------------
>  include/bloblist.h | 10 --------
>  2 files changed, 19 insertions(+), 55 deletions(-)
>
> diff --git a/common/bloblist.c b/common/bloblist.c
> index e8acfc74331..7eda94ecdf9 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -487,57 +487,37 @@ int bloblist_reloc(void *to, uint to_size)
>         return 0;
>  }
>
> -/*
> - * Weak default function for getting bloblist from boot args.
> - */
> -int __weak xferlist_from_boot_arg(ulong __always_unused addr,
> -                                 ulong __always_unused size)
> -{
> -       return -ENOENT;
> -}
> -
>  int bloblist_init(void)
>  {
>         bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED);
>         int ret = -ENOENT;
> -       ulong addr = 0, size;
> -       /*
> -        * If U-Boot is not in the first phase, an existing bloblist must be
> -        * at a fixed address.
> -        */
> -       bool from_addr = fixed && !xpl_is_first_phase();
> -       /*
> -        * If U-Boot is in the first phase that an arch custom routine should
> -        * install the bloblist passed from previous loader to this fixed
> +       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
>          * address.
>          */
> -       bool from_boot_arg = fixed && xpl_is_first_phase();
> -
> +       expected = fixed && !xpl_is_first_phase();
>         if (xpl_prev_phase() == PHASE_TPL && !IS_ENABLED(CONFIG_TPL_BLOBLIST))
> -               from_addr = false;
> +               expected = false;
>         if (fixed)
>                 addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED,
>                                       CONFIG_BLOBLIST_ADDR);
>         size = CONFIG_BLOBLIST_SIZE;
> -
> -       if (from_boot_arg)
> -               ret = xferlist_from_boot_arg(addr, size);
> -       else if (from_addr)
> +       if (expected) {
>                 ret = bloblist_check(addr, 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) {
> +                       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) {
> -               /*
> -                * If we don't have a bloblist from a fixed address, or the one
> -                * in the fixed address is not valid. we must allocate the
> -                * memory for it now.
> -                */
>                 if (CONFIG_IS_ENABLED(BLOBLIST_ALLOC)) {
>                         void *ptr = memalign(BLOBLIST_ALIGN, size);
>
> @@ -545,8 +525,7 @@ int bloblist_init(void)
>                                 return log_msg_ret("alloc", -ENOMEM);
>                         addr = map_to_sysmem(ptr);
>                 } else if (!fixed) {
> -                       return log_msg_ret("BLOBLIST_FIXED is not enabled",
> -                                          ret);
> +                       return log_msg_ret("!fixed", ret);
>                 }
>                 log_debug("Creating new bloblist size %lx at %lx\n", size,
>                           addr);
> @@ -559,11 +538,6 @@ int bloblist_init(void)
>                 return log_msg_ret("ini", ret);
>         gd->flags |= GD_FLG_BLOBLIST_READY;
>
> -#ifdef DEBUG
> -       bloblist_show_stats();
> -       bloblist_show_list();
> -#endif
> -
>         return 0;
>  }
>
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 1e1ca34aa92..5063ab66e4a 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -482,14 +482,4 @@ static inline int bloblist_maybe_init(void)
>   */
>  int bloblist_check_reg_conv(ulong rfdt, ulong rzero, ulong rsig);
>
> -/**
> - * xferlist_from_boot_arg() - Get bloblist from the boot args and relocate it
> - *                           to the specified address.
> - *
> - * @addr: Address for the bloblist
> - * @size: Size of space reserved for the bloblist
> - * Return: 0 if OK, else on error
> - */
> -int xferlist_from_boot_arg(ulong addr, ulong size);
> -
>  #endif /* __BLOBLIST_H */
> --
> 2.43.0
>