On Fri, May 15, 2026 at 08:10:48PM -0400, Raymond Mao wrote: > Hi Tom, > > On Fri, May 15, 2026 at 12:30 PM Tom Rini <[email protected]> wrote: > > > > On Fri, May 15, 2026 at 11:05:47AM -0400, Raymond Mao wrote: > > > Hi Tom, > > > > > > On Thu, May 14, 2026 at 1:55 PM Tom Rini <[email protected]> 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 <[email protected]> > > > > --- > > > > 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; > > > > + } > > > > + > > > > + /* > > > > + * If Firmware Handoff is mandatory but no transfer list has > > > > been > > > > + * observed by fdtdec_setup, report it as an error. > > > > + */ > > > > + if (IS_ENABLED(CONFIG_BLOBLIST_PASSAGE_MANDATORY)) > > > > + return -ENOENT; > > > > + > > > > + /* > > > > + * If we don't have an existing bloblist, we either need > > > > + * to allocate one now, or initialize the fixed address > > > > + * space as a bloblist. > > > > + */ > > > > + 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 > > > > + addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED, > > > > + CONFIG_BLOBLIST_ADDR); > > > > + > > > > + log_debug("Creating new bloblist size %lx at %lx\n", size, > > > > + addr); > > > > + ret = bloblist_new(addr, size, 0, 0); > > > > + if (ret) > > > > + return log_msg_ret("ini", ret); > > > > > > > > return 0; > > > > } > > > > @@ -687,7 +687,9 @@ int bloblist_check_reg_conv(ulong rfdt, ulong > > > > rzero, ulong rsig, ulong xlist) > > > > return ret; > > > > > > > > if (rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0)) { > > > > - gd->bloblist = NULL; /* Reset the gd bloblist pointer > > > > */ > > > > + /* Remove this bloblist from gd */ > > > > + gd->bloblist = NULL; > > > > + gd->flags &= ~GD_FLG_BLOBLIST_READY; > > > > return -EIO; > > > > } > > > > > > > > diff --git a/common/board_f.c b/common/board_f.c > > > > index ce87c619e680..c39e3b940d3b 100644 > > > > --- a/common/board_f.c > > > > +++ b/common/board_f.c > > > > @@ -894,7 +894,9 @@ static void initcall_run_f(void) > > > > INITCALL(log_init); > > > > INITCALL(initf_bootstage); /* uses its own timer, so does not > > > > need DM */ > > > > INITCALL(event_init); > > > > - INITCALL(bloblist_maybe_init); > > > > +#if CONFIG_IS_ENABLED(BLOBLIST) > > > > + INITCALL(bloblist_init); > > > > +#endif > > > > INITCALL(setup_spl_handoff); > > > > #if CONFIG_IS_ENABLED(CONSOLE_RECORD_INIT_F) > > > > INITCALL(console_record_init); > > > > diff --git a/include/bloblist.h b/include/bloblist.h > > > > index e67b2a76358d..4c5787729654 100644 > > > > --- a/include/bloblist.h > > > > +++ b/include/bloblist.h > > > > @@ -488,10 +488,24 @@ const char *bloblist_tag_name(enum bloblist_tag_t > > > > tag); > > > > */ > > > > int bloblist_reloc(void *to, uint to_size); > > > > > > > > +/** > > > > + * bloblist_exists() - Check for the prior existence of a bloblist > > > > + * > > > > + * This will check for a transfer list having been passed via standard > > > > + * convention. If CONFIG_BLOBLIST_PASSAGE_MANDATORY is selected and > > > > one is not > > > > + * found, we return false. > > > > + * > > > > + * If CONFIG_BLOBLIST_FIXED is selected, it uses CONFIG_BLOBLIST_ADDR > > > > and > > > > + * CONFIG_BLOBLIST_SIZE to check for a bloblist. > > > > + * > > > > + * Return: true if found, false if not > > > > + */ > > > > +bool bloblist_exists(void); > > > > + > > > > /** > > > > * bloblist_init() - Init the bloblist system with a single bloblist > > > > * > > > > - * This locates and sets up the blocklist for use. > > > > + * This creates a bloblist for use, if not already found. > > > > * > > > > * If CONFIG_BLOBLIST_FIXED is selected, it uses CONFIG_BLOBLIST_ADDR > > > > and > > > > * CONFIG_BLOBLIST_SIZE to set up a bloblist for use by U-Boot. > > > > @@ -508,22 +522,6 @@ int bloblist_reloc(void *to, uint to_size); > > > > */ > > > > int bloblist_init(void); > > > > > > > > -#if CONFIG_IS_ENABLED(BLOBLIST) > > > > -/** > > > > - * bloblist_maybe_init() - Init the bloblist system if not already done > > > > - * > > > > - * Calls bloblist_init() if the GD_FLG_BLOBLIST_READY flag is not set > > > > - * > > > > - * Return: 0 if OK, -ve on error > > > > - */ > > > > -int bloblist_maybe_init(void); > > > > -#else > > > > -static inline int bloblist_maybe_init(void) > > > > -{ > > > > - return 0; > > > > -} > > > > -#endif /* BLOBLIST */ > > > > - > > > > /** > > > > * bloblist_check_reg_conv() - Check whether the bloblist is compliant > > > > to > > > > * the register conventions according to the > > > > diff --git a/lib/fdtdec.c b/lib/fdtdec.c > > > > index c67b6e8c133e..7fd7f7ff01e5 100644 > > > > --- a/lib/fdtdec.c > > > > +++ b/lib/fdtdec.c > > > > @@ -1822,17 +1822,12 @@ int fdtdec_setup(void) > > > > int ret = -ENOENT; > > > > > > > > /* > > > > - * If allowing a bloblist, check that first. There was > > > > discussion about > > > > - * adding an OF_BLOBLIST Kconfig, but this was rejected. > > > > - * > > > > - * The necessary test is whether the previous phase passed a > > > > bloblist, > > > > - * not whether this phase creates one. > > > > + * If allowing a bloblist, check that first. The necessary test > > > > is > > > > + * whether the previous phase passed a bloblist, not whether > > > > this phase > > > > + * creates one. > > > > */ > > > > - if (CONFIG_IS_ENABLED(BLOBLIST) && > > > > - (xpl_prev_phase() != PHASE_TPL || > > > > - IS_ENABLED(CONFIG_TPL_BLOBLIST))) { > > > > - ret = bloblist_maybe_init(); > > > > - if (!ret) { > > > > + if (CONFIG_IS_ENABLED(BLOBLIST) && (xpl_prev_phase() != > > > > PHASE_TPL)) { > > > > > > I think this change drops the TPL-provided blobs on TPL->SPL boots. > > > When TPL_BLOBLIST is selected, the next phase starts with > > > xpl_prev_phase() == PHASE_TPL. > > > This new guard now skips bloblist_exists(), so gd->flags never gets > > > GD_FLG_BLOBLIST_READY; the later bloblist_init() in common/spl/spl.c > > > then creates a fresh bloblist and overwrites the fixed-address list > > > from TPL. > > > > Ah right, oops. I guess the only question is if we should continue to > > optimize the TPL case (it can't exist) or just always check (maybe > > there's some 4 stage boot..). > > > > That really depends on whether we shall consider the scenario of TPL. > For the case of the Arm Firmware Handoff, stages before BL2 are not > considered to hand information over to the next stage via Transfer > List (aka. bloblist in U-Boot).
OK, I think we can just go with SPL or higher looks for a bloblist, TPL can create one but not be passed one. It's simple enough to change if there's somehow a 4 stage boot we need to worry about later. -- Tom
signature.asc
Description: PGP signature
