Hi Nikita, On 2026-08-07T08:06:41, Nikita Shubin <[email protected]> wrote: > spl: fit: Record loadables to spl_image->fdt_addr > > Record lodable to FDT passed to next boot stage.
Typo: 'lodable' should be 'loadable' (also 'lodables' below). > > This is required for spl_invoke_opensbi(), for example, which requires > u-boot/kernel load or entry address to fill opensbi_info.next_addr > entry. > > spl_load_simple_fit() does it, make spl_load_fit_image() also capable of > recording lodables. > > Signed-off-by: Nikita Shubin <[email protected]> > > common/spl/spl_fit.c | 44 +++++++++++++++++++++++++++++++++++--------- > 1 file changed, 35 insertions(+), 9 deletions(-) > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c > @@ -1009,17 +1009,43 @@ int spl_load_fit_image(struct spl_image_info > *spl_image, > + /* Record our loadables into the FDT */ > + if (spl_image->fdt_addr) { > + ulong entry_addr = -1; Please use FDT_ERROR (from image.h) rather than a bare -1 - it is the sentinel used elsewhere in this file for a missing entry point and makes the connection to the 'entry_point != -1' check in fdt_record_loadable() clearer. > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c > @@ -1009,17 +1009,43 @@ int spl_load_fit_image(struct spl_image_info > *spl_image, > + /* Try to make space, so we can inject details on the > loadable */ > + ret = fdt_increase_size(spl_image->fdt_addr, 256); > + if (ret < 0) { > + debug(PHASE_PROMPT "record_loadable: couldn't > increase space " \ > + "for fdt: %d\n", ret); > + continue; > + } This grows the blob by 256 bytes on every iteration, whether or not the space is needed, and nothing checks what lies beyond the FDT in memory, so a large loadables list could silently run over adjacent data. The spl_load_simple_fit() path handles this once, in spl_fit_append_fdt(), with fdt_shrink_to_minimum(spl_image->fdt_addr, 8192). Please can you do the size adjustment once, before the loop, ideally using the same approach so the two paths behave consistently? Also the trailing backslash is not needed (adjacent string literals concatenate) and 'record_loadable' names a function that does not exist - log_debug() would be better as if can show the function if CONFIG_LOGF_FUNC is enabled. > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c > @@ -1009,17 +1009,43 @@ int spl_load_fit_image(struct spl_image_info > *spl_image, > + ret = fdt_record_loadable(spl_image->fdt_addr, idx, > uname, > + img_data, img_len, entry_addr, > + fdt_getprop((const void > *)header, noffset, FIT_TYPE_PROP, NULL), > + fdt_getprop((const void > *)header, noffset, FIT_OS_PROP, NULL), > + fdt_getprop((const void > *)header, noffset, FIT_ARCH_PROP, NULL)); This open-codes what spl_fit_record_loadable() already does a few hundred lines above, just with a node offset instead of a ctx. Please can you pull this block out into a small static helper (e.g. taking the FIT, noffset, idx, uname and the load/size values)? That would reduce the duplication, keep these lines within 80 columns and cut the nesting depth in the loop. Regards, Simon
