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

[Michal Simek]

Hi,

st 5. 2. 2025 v 3:00 odesílatel Simon Glass <sjg at chromium.org> napsal:
>
> 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>
> ---
>
>  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
>

First of all if this should be reverted you should also remove
arch/arm/lib/xferlist.c.

But we plan to use this feature and TL list will be passed via
register as is defined in firmware handoff spec.
In our configuration we won't be using any of TPL/VPL/SPL and A cores
will start with TF-A at bl31 which gets
TL list from the coprocessor. And TF-A should be passing TL list to
TEE (optional) and to U-Boot.
If only hard coded address is supported we would have to recompile
u-boot based on location where TL should
be which is an unwanted limitation.
If current implementation is not correct it should be fixed but revert
seems to me too early without saying
what should change.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs