[PATCH v2 3/3] bloblist: Rework bloblist_init and bloblist_maybe_init
Raymond Mao
raymondmaoca at gmail.com
Wed May 20 02:51:12 CEST 2026
Hi Tom,
On Tue, May 19, 2026 at 7:37 PM Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, May 19, 2026 at 07:30:53PM -0400, Raymond Mao wrote:
> > Hi Tom,
> >
> > On Tue, May 19, 2026 at 12:23 PM Tom Rini <trini at konsulko.com> wrote:
> > >
> > > With bloblist, we need to both see if one already exists as well as
> > > create one if it does not. However, the current implementation leads to
> > > odd cases where we attempt to create a bloblist before this is possible
> > > and have things be overly complicated when we are given one to work
> > > with.
> > >
> > > This reworks things to instead have a bloblist_exists function, which as
> > > the name implies checks for an existing bloblist. This is used in
> > > the case of booting, to see if we have one and in turn if we have a
> > > device tree there as well as in the bloblist_init function to see if we
> > > need to do anything.
> > >
> > > In practical details, we move the logic from bloblist_init that was
> > > checking for a bloblist to the new bloblist_exists function and then can
> > > clarify the logic as it is much easier to state when we know we do not
> > > have one rather than all the ways we might have one. Then we have the
> > > locations that set gd->bloblist now also set the GD_FLG_BLOBLIST_READY
> > > flag.
> > >
> > > Signed-off-by: Tom Rini <trini at konsulko.com>
> > > ---
> > > Changes in v2:
> > > - As noted by Raymond, we need to still check if SPL is being passed a
> > > bloblist by TPL.
> > > ---
> > > common/bloblist.c | 144 +++++++++++++++++++++++----------------------
> > > common/board_f.c | 4 +-
> > > include/bloblist.h | 32 +++++-----
> > > lib/fdtdec.c | 15 ++---
> > > 4 files changed, 96 insertions(+), 99 deletions(-)
> > >
> > > diff --git a/common/bloblist.c b/common/bloblist.c
> > > index 09afdd1f96b4..51ae9cc50a5f 100644
> > > --- a/common/bloblist.c
> > > +++ b/common/bloblist.c
> > > @@ -448,6 +448,7 @@ int bloblist_new(ulong addr, uint size, uint flags, uint align_log2)
> > > hdr->align_log2 = align_log2 ? align_log2 : BLOBLIST_BLOB_ALIGN_LOG2;
> > > hdr->chksum = 0;
> > > gd->bloblist = hdr;
> > > + gd->flags |= GD_FLG_BLOBLIST_READY;
> > >
> > > return 0;
> > > }
> > > @@ -475,6 +476,7 @@ int bloblist_check(ulong addr, uint size)
> > > return log_msg_ret("Bad checksum", -EIO);
> > > }
> > > gd->bloblist = hdr;
> > > + gd->flags |= GD_FLG_BLOBLIST_READY;
> > >
> > > return 0;
> > > }
> > > @@ -576,90 +578,88 @@ int __weak xferlist_from_boot_arg(ulong __always_unused *addr)
> > > return -ENOENT;
> > > }
> > >
> > > -int bloblist_init(void)
> > > +bool bloblist_exists(void)
> > > {
> > > - bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED);
> > > - int ret = 0;
> > > - ulong addr = 0, size;
> > > + int ret;
> > > + ulong addr = 0;
> > >
> > > /* Check if a valid transfer list passed in */
> > > - if (!xferlist_from_boot_arg(&addr)) {
> > > - size = bloblist_get_total_size();
> > > - } else {
> > > - /*
> > > - * 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 Firmware Handoff is mandatory but no transfer list is
> > > - * observed, report it as an error.
> > > - */
> > > - if (IS_ENABLED(CONFIG_BLOBLIST_PASSAGE_MANDATORY))
> > > - return -ENOENT;
> > > -
> > > - ret = -ENOENT;
> > > -
> > > - if (xpl_prev_phase() == PHASE_TPL &&
> > > - !IS_ENABLED(CONFIG_TPL_BLOBLIST))
> > > - from_addr = false;
> > > - if (fixed)
> > > - addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED,
> > > - CONFIG_BLOBLIST_ADDR);
> > > - size = CONFIG_BLOBLIST_SIZE;
> > > -
> > > - if (from_addr)
> > > - ret = bloblist_check(addr, size);
> > > + if (!xferlist_from_boot_arg(&addr))
> > > + goto found;
> > >
> > > - if (ret)
> > > - log_debug("Bloblist at %lx not found (err=%d)\n",
> > > - addr, ret);
> > > - else
> > > - /* Get the real size */
> > > - size = gd->bloblist->total_size;
> > > - }
> > > + /*
> > > + * If Firmware Handoff is mandatory but no transfer list is
> > > + * observed, report it as an error.
> > > + */
> > > + if (IS_ENABLED(CONFIG_BLOBLIST_PASSAGE_MANDATORY))
> > > + return false;
> > >
> > > - 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);
> > > -
> > > - if (!ptr)
> > > - return log_msg_ret("alloc", -ENOMEM);
> > > - addr = map_to_sysmem(ptr);
> > > - } else if (!fixed) {
> > > - return log_msg_ret("BLOBLIST_FIXED is not enabled",
> > > - ret);
> > > - }
> > > - log_debug("Creating new bloblist size %lx at %lx\n", size,
> > > - addr);
> > > - ret = bloblist_new(addr, size, 0, 0);
> > > - } else {
> > > - log_debug("Found existing bloblist size %lx at %lx\n", size,
> > > - addr);
> > > - }
> > > + /*
> > > + * We have checked for a valid transfer list being passed. At this
> > > + * point, if we do not have a fixed address for the bloblist, we cannot
> > > + * be provided with one.
> > > + */
> > > + if (xpl_is_first_phase() || !IS_ENABLED(CONFIG_BLOBLIST_FIXED))
> > > + return false;
> > >
> > > - if (ret)
> > > - return log_msg_ret("ini", ret);
> > > - gd->flags |= GD_FLG_BLOBLIST_READY;
> > > + /*
> > > + * Check for a valid list as the configured address.
> > > + */
> > > + addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED,
> > > + CONFIG_BLOBLIST_ADDR);
> > > + ret = bloblist_check(addr, CONFIG_BLOBLIST_SIZE);
> > > + if (!ret)
> > > + goto found;
> > > +
> > > + log_debug("Bloblist at %lx not found (err=%d)\n", addr, ret);
> > > + return false;
> > >
> > > +found:
> > > #ifdef DEBUG
> > > bloblist_show_stats();
> > > bloblist_show_list();
> > > #endif
> > > -
> > > - return 0;
> > > + return true;
> > > }
> > >
> > > -int bloblist_maybe_init(void)
> > > +int bloblist_init(void)
> > > {
> > > - if (CONFIG_IS_ENABLED(BLOBLIST) && !(gd->flags & GD_FLG_BLOBLIST_READY))
> > > - return bloblist_init();
> > > + int ret;
> > > + ulong addr = 0, size = CONFIG_BLOBLIST_SIZE;
> > > +
> > > + if (gd->flags & GD_FLG_BLOBLIST_READY) {
> > > + log_debug("Found existing bloblist size %x at %p\n",
> > > + gd->bloblist->total_size, gd->bloblist);
> > > + return 0;
> > > + }
> > > +
> >
> > This part is a bit difficult to read.
> > The patch seems to try to assume that '!(gd->flags &
> > GD_FLG_BLOBLIST_READY)' means 'no handoff from the previous stage',
>
> Correct.
>
> > but I doubt this is a part of GD_FLG_BLOBLIST_READY's semantics.
>
> It is part of the semantics now, however with this patch. That said, I
> don't love the name GD_FLG_BLOBLIST_READY but that name was a side
> effect of the old behavior too and needing to bail out from
> "maybe_init".
>
> > Under the 'handoff from the previous stage' scenario, if
> > GD_FLG_BLOBLIST_READY is not set in time, the logic here breaks.
>
> Now that "exists" is not part of "init", we should have already called
> bloblist_check and set the flag before we can get here.
>
Just one minor suggestion, can we introduce a GD_FLG_BLOBLIST_HANDOFF?
Since the _READY has an ambiguity that "it is ready from alloc"...
But with or without above suggested change, overall the patch cleans
up the logics, thus it is fine to me:
Reviewed-by: Raymond Mao <raymondmaoca at gmail.com>
Regards,
Raymond
> --
> Tom
More information about the U-Boot
mailing list