[PATCH 00/19] vbe: Series part E

This includes various patches towards implementing the VBE abrec bootmeth in U-Boot. It mostly focuses on SPL tweaks and adjusting what fatures are available in VPL.
Simon Glass (19): image: Add a prototype for fit_image_get_phase() serial: ns16550: Allow clocks to be missing boot: Allow FIT to fall back from best-match option bootstd: Avoid sprintf() in SPL when creating bootdevs boot: Respect the load_op in fit_image_load() malloc: Show amount of used space when memory runs out malloc: Provide a simple malloc for VPL Support setting a maximum size for the VPL image spl: Report a loader failure spl: Allow serial to be disabled in any XPL phase spl: Support a relocated stack in any XPL phase spl: Drop use of uintptr_t spl: Drop a duplicate variable in boot_from_devices() spl: Add some more debugging to load_simple_fit() spl: lib: Allow for decompression in any SPL build boot: Allow use of FIT in TPL and VPL lib: Allow crc8 in TPL and VPL boot: Imply CRC8 with VBE hash: Plumb crc8 into the hash functions
boot/Kconfig | 71 ++++++++++++++++++++++++++++++++++- boot/Makefile | 4 +- boot/bootdev-uclass.c | 10 ++++- boot/image-fit.c | 29 ++++++++------ common/hash.c | 8 ++++ common/malloc_simple.c | 3 +- common/spl/Kconfig.vpl | 17 +++++++++ common/spl/spl.c | 15 +++++--- common/spl/spl_atf.c | 36 +++++++++--------- common/spl/spl_fit.c | 12 +++++- common/spl/spl_legacy.c | 8 ++-- configs/sandbox_vpl_defconfig | 3 +- drivers/serial/ns16550.c | 2 +- include/image.h | 16 +++++++- include/spl.h | 28 +++++++------- include/u-boot/crc.h | 3 ++ lib/Kconfig | 53 ++++++++++++++++++++++++++ lib/Makefile | 8 ++-- lib/crc8.c | 6 +++ 19 files changed, 263 insertions(+), 69 deletions(-)

This function exists but is not exported. Add a prototype so it can be used elsewhere.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/image.h | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/include/image.h b/include/image.h index dd4042d1bd9..dbf8d0e7ba9 100644 --- a/include/image.h +++ b/include/image.h @@ -1183,6 +1183,18 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset, int fit_image_get_data_and_size(const void *fit, int noffset, const void **data, size_t *size);
+/** + * fit_image_get_phase() - Get the phase from a FIT image + * + * @fit: FIT to read from + * @offset: offset node to read + * @phasep: Returns phase, if any + * Return: 0 if read OK and *phasep is value, -ENOENT if there was no phase + * property in the node, other -ve value on other error + */ +int fit_image_get_phase(const void *fit, int offset, + enum image_phase_t *phasep); + /** * fit_get_data_node() - Get verified image data for an image * @fit: Pointer to the FIT format image header

Allow serial init when clock support is not enabled in a particular phase.
Signed-off-by: Simon Glass sjg@chromium.org ---
drivers/serial/ns16550.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 6fcb5b523ac..5cda273623d 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -570,7 +570,7 @@ int ns16550_serial_of_to_plat(struct udevice *dev) err = clk_get_rate(&clk); if (!IS_ERR_VALUE(err)) plat->clock = err; - } else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS) { + } else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS && err != -EALREADY) { debug("ns16550 failed to get clock\n"); return err; }

On Thu, Aug 29, 2024 at 08:57:45AM -0600, Simon Glass wrote:
Allow serial init when clock support is not enabled in a particular phase.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/serial/ns16550.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 6fcb5b523ac..5cda273623d 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -570,7 +570,7 @@ int ns16550_serial_of_to_plat(struct udevice *dev) err = clk_get_rate(&clk); if (!IS_ERR_VALUE(err)) plat->clock = err;
- } else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS) {
- } else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS && err != -EALREADY) { debug("ns16550 failed to get clock\n"); return err; }
Your patch and https://patchwork.ozlabs.org/project/uboot/patch/20240804150955.3521296-1-jo... would seem to be in conflict and I'm not sure what's the best thing moving forward.

Hi Tom,
On Thu, 29 Aug 2024 at 19:24, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 29, 2024 at 08:57:45AM -0600, Simon Glass wrote:
Allow serial init when clock support is not enabled in a particular phase.
Signed-off-by: Simon Glass sjg@chromium.org
drivers/serial/ns16550.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 6fcb5b523ac..5cda273623d 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -570,7 +570,7 @@ int ns16550_serial_of_to_plat(struct udevice *dev) err = clk_get_rate(&clk); if (!IS_ERR_VALUE(err)) plat->clock = err;
} else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS) {
} else if (err != -ENOENT && err != -ENODEV && err != -ENOSYS && err != -EALREADY) { debug("ns16550 failed to get clock\n"); return err; }
Your patch and https://patchwork.ozlabs.org/project/uboot/patch/20240804150955.3521296-1-jo... would seem to be in conflict and I'm not sure what's the best thing moving forward.
That patch seems good to me. For this one, I will drop it from the next version. The existing conditions should be enough for the case I have (clocks in the previous phase but not in this one) so long as -ENOSYS is returned.
Regards, Simon

When the best-match feature fails to find something, use the provided config name as a fallback. The allows SPL to select a suitable config when best-match is enabled.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image-fit.c | 19 ++++++++++--------- include/image.h | 4 +++- 2 files changed, 13 insertions(+), 10 deletions(-)
diff --git a/boot/image-fit.c b/boot/image-fit.c index 7d56f0b5e6e..439ff51edf6 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -1729,13 +1729,13 @@ int fit_conf_find_compat(const void *fit, const void *fdt) images_noffset = fdt_path_offset(fit, FIT_IMAGES_PATH); if (confs_noffset < 0 || images_noffset < 0) { debug("Can't find configurations or images nodes.\n"); - return -1; + return -EINVAL; }
fdt_compat = fdt_getprop(fdt, 0, "compatible", &fdt_compat_len); if (!fdt_compat) { debug("Fdt for comparison has no "compatible" property.\n"); - return -1; + return -ENXIO; }
/* @@ -1812,7 +1812,7 @@ int fit_conf_find_compat(const void *fit, const void *fdt) } if (!best_match_offset) { debug("No match found.\n"); - return -1; + return -ENOENT; }
return best_match_offset; @@ -2095,17 +2095,18 @@ int fit_image_load(struct bootm_headers *images, ulong addr, * fit_conf_get_node() will try to find default config node */ bootstage_mark(bootstage_id + BOOTSTAGE_SUB_NO_UNIT_NAME); - if (IS_ENABLED(CONFIG_FIT_BEST_MATCH) && !fit_uname_config) { - cfg_noffset = fit_conf_find_compat(fit, gd_fdt_blob()); - } else { - cfg_noffset = fit_conf_get_node(fit, fit_uname_config); - } - if (cfg_noffset < 0) { + ret = -ENXIO; + if (IS_ENABLED(CONFIG_FIT_BEST_MATCH) && !fit_uname_config) + ret = fit_conf_find_compat(fit, gd_fdt_blob()); + if (ret < 0 && ret != -EINVAL) + ret = fit_conf_get_node(fit, fit_uname_config); + if (ret < 0) { puts("Could not find configuration node\n"); bootstage_error(bootstage_id + BOOTSTAGE_SUB_NO_UNIT_NAME); return -ENOENT; } + cfg_noffset = ret;
fit_base_uname_config = fdt_get_name(fit, cfg_noffset, NULL); printf(" Using '%s' configuration\n", fit_base_uname_config); diff --git a/include/image.h b/include/image.h index dbf8d0e7ba9..92128180e0f 100644 --- a/include/image.h +++ b/include/image.h @@ -1423,7 +1423,9 @@ int fit_check_format(const void *fit, ulong size); * copied into the configuration node in the FIT image. This is required to * match configurations with compressed FDTs. * - * Returns: offset to the configuration to use if one was found, -1 otherwise + * Returns: offset to the configuration to use if one was found, -EINVAL if + * there a /configurations or /images node is missing, -ENOENT if no match was + * found, -ENXIO if the FDT node has no compatible string */ int fit_conf_find_compat(const void *fit, const void *fdt);

On Thu, Aug 29, 2024 at 08:57:46AM -0600, Simon Glass wrote:
When the best-match feature fails to find something, use the provided config name as a fallback. The allows SPL to select a suitable config when best-match is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
boot/image-fit.c | 19 ++++++++++--------- include/image.h | 4 +++- 2 files changed, 13 insertions(+), 10 deletions(-)
Don't we need a documentation update? If no, because this mechanism isn't well documented, can you please follow up with that? Thanks.

Hi Tom,
On Thu, 5 Sept 2024 at 00:25, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 29, 2024 at 08:57:46AM -0600, Simon Glass wrote:
When the best-match feature fails to find something, use the provided config name as a fallback. The allows SPL to select a suitable config when best-match is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
boot/image-fit.c | 19 ++++++++++--------- include/image.h | 4 +++- 2 files changed, 13 insertions(+), 10 deletions(-)
Don't we need a documentation update? If no, because this mechanism isn't well documented, can you please follow up with that? Thanks.
CONFIG_FIT_BEST_MATCH is mentioned in the spec [1]
But re the fallback, I'm not sure where that should be documented. Perhaps add a note at [2]?
Regards, Simon
[1] https://fitspec.osfw.foundation/ [2] https://docs.u-boot.org/en/latest/usage/fit/howto.html

On Fri, Sep 20, 2024 at 05:59:00PM +0200, Simon Glass wrote:
Hi Tom,
On Thu, 5 Sept 2024 at 00:25, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 29, 2024 at 08:57:46AM -0600, Simon Glass wrote:
When the best-match feature fails to find something, use the provided config name as a fallback. The allows SPL to select a suitable config when best-match is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
boot/image-fit.c | 19 ++++++++++--------- include/image.h | 4 +++- 2 files changed, 13 insertions(+), 10 deletions(-)
Don't we need a documentation update? If no, because this mechanism isn't well documented, can you please follow up with that? Thanks.
CONFIG_FIT_BEST_MATCH is mentioned in the spec [1]
But re the fallback, I'm not sure where that should be documented. Perhaps add a note at [2]?
Should we be using the mechanism that I'd swear the documentation build process has, to fetch an external document? I think we would have a better user/developer experience if our documentation explained how to use a feature, as that's where people will tend to look first.

The name of the bootdev device is not that important, particular in SPL. Save a little code space by using a simpler name.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/bootdev-uclass.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/boot/bootdev-uclass.c b/boot/bootdev-uclass.c index 7c7bba088c9..6a0d8466e95 100644 --- a/boot/bootdev-uclass.c +++ b/boot/bootdev-uclass.c @@ -16,6 +16,7 @@ #include <malloc.h> #include <part.h> #include <sort.h> +#include <spl.h> #include <dm/device-internal.h> #include <dm/lists.h> #include <dm/uclass-internal.h> @@ -278,8 +279,13 @@ int bootdev_setup_for_sibling_blk(struct udevice *blk, const char *drv_name) int ret, len;
len = bootdev_get_suffix_start(blk, ".blk"); - snprintf(dev_name, sizeof(dev_name), "%.*s.%s", len, blk->name, - "bootdev"); + if (spl_phase() < PHASE_BOARD_R) { + strlcpy(dev_name, blk->name, sizeof(dev_name) - 4); + strcat(dev_name, ".sib"); + } else { + snprintf(dev_name, sizeof(dev_name), "%.*s.%s", len, blk->name, + "bootdev"); + }
parent = dev_get_parent(blk); ret = device_find_child_by_name(parent, dev_name, &dev);

Some code has crept in which ignores this parameter. Fix this and add a little debugging.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/image-fit.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/boot/image-fit.c b/boot/image-fit.c index 439ff51edf6..db7fb61bca9 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -2226,6 +2226,7 @@ int fit_image_load(struct bootm_headers *images, ulong addr, data = map_to_sysmem(buf); load = data; if (load_op == FIT_LOAD_IGNORED) { + log_debug("load_op: not loading\n"); /* Don't load */ } else if (fit_image_get_load(fit, noffset, &load)) { if (load_op == FIT_LOAD_REQUIRED) { @@ -2262,10 +2263,13 @@ int fit_image_load(struct bootm_headers *images, ulong addr, /* Kernel images get decompressed later in bootm_load_os(). */ if (!fit_image_get_comp(fit, noffset, &comp) && comp != IH_COMP_NONE && + load_op != FIT_LOAD_IGNORED && !(image_type == IH_TYPE_KERNEL || image_type == IH_TYPE_KERNEL_NOLOAD || image_type == IH_TYPE_RAMDISK)) { ulong max_decomp_len = len * 20; + + log_debug("decompressing image\n"); if (load == data) { loadbuf = malloc(max_decomp_len); load = map_to_sysmem(loadbuf); @@ -2280,6 +2284,7 @@ int fit_image_load(struct bootm_headers *images, ulong addr, } len = load_end - load; } else if (load != data) { + log_debug("copying\n"); loadbuf = map_sysmem(load, len); memcpy(loadbuf, buf, len); } @@ -2289,8 +2294,9 @@ int fit_image_load(struct bootm_headers *images, ulong addr, " please fix your .its file!\n");
/* verify that image data is a proper FDT blob */ - if (image_type == IH_TYPE_FLATDT && fdt_check_header(loadbuf)) { - puts("Subimage data is not a FDT"); + if (load_op != FIT_LOAD_IGNORED && image_type == IH_TYPE_FLATDT && + fdt_check_header(loadbuf)) { + puts("Subimage data is not a FDT\n"); return -ENOEXEC; }

On Thu, Aug 29, 2024 at 08:57:48AM -0600, Simon Glass wrote:
Some code has crept in which ignores this parameter. Fix this and add a little debugging.
Signed-off-by: Simon Glass sjg@chromium.org
Can you please add a Fixes tag, as that will help in reviewing this too, to ensure we aren't breaking the case that initially changed things here, thanks.

Show a bit more information when malloc() space is exhausted and debugging is enabled.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/malloc_simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 5a8ec538f8f..f2b3dc53689 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align) log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr, gd->malloc_limit); if (new_ptr > gd->malloc_limit) { - log_err("alloc space exhausted\n"); + log_err("alloc space exhausted %lx %x\n", new_ptr, + gd->malloc_limit); return NULL; }

On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
Show a bit more information when malloc() space is exhausted and debugging is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
common/malloc_simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 5a8ec538f8f..f2b3dc53689 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align) log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr, gd->malloc_limit); if (new_ptr > gd->malloc_limit) {
log_err("alloc space exhausted\n");
log_err("alloc space exhausted %lx %x\n", new_ptr,
return NULL; }gd->malloc_limit);
Since debugging is enabled you should update the error message to say what the values mean...

Hi Tom,
On Thu, 29 Aug 2024 at 19:26, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
Show a bit more information when malloc() space is exhausted and debugging is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
common/malloc_simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 5a8ec538f8f..f2b3dc53689 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align) log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr, gd->malloc_limit); if (new_ptr > gd->malloc_limit) {
log_err("alloc space exhausted\n");
log_err("alloc space exhausted %lx %x\n", new_ptr,
gd->malloc_limit); return NULL; }
Since debugging is enabled you should update the error message to say what the values mean...
OK, will do. This message is shown without debugging enabled, so I do want to keep it short.
Regards, Simon

On Fri, Sep 20, 2024 at 09:25:44AM +0200, Simon Glass wrote:
Hi Tom,
On Thu, 29 Aug 2024 at 19:26, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
Show a bit more information when malloc() space is exhausted and debugging is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
common/malloc_simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 5a8ec538f8f..f2b3dc53689 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align) log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr, gd->malloc_limit); if (new_ptr > gd->malloc_limit) {
log_err("alloc space exhausted\n");
log_err("alloc space exhausted %lx %x\n", new_ptr,
gd->malloc_limit); return NULL; }
Since debugging is enabled you should update the error message to say what the values mean...
OK, will do. This message is shown without debugging enabled, so I do want to keep it short.
The commit message says "and debugging is enabled", so is that the case or not?

Hi Tom,
On Fri, 20 Sept 2024 at 16:59, Tom Rini trini@konsulko.com wrote:
On Fri, Sep 20, 2024 at 09:25:44AM +0200, Simon Glass wrote:
Hi Tom,
On Thu, 29 Aug 2024 at 19:26, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
Show a bit more information when malloc() space is exhausted and debugging is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
common/malloc_simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 5a8ec538f8f..f2b3dc53689 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align) log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr, gd->malloc_limit); if (new_ptr > gd->malloc_limit) {
log_err("alloc space exhausted\n");
log_err("alloc space exhausted %lx %x\n", new_ptr,
gd->malloc_limit); return NULL; }
Since debugging is enabled you should update the error message to say what the values mean...
OK, will do. This message is shown without debugging enabled, so I do want to keep it short.
The commit message says "and debugging is enabled", so is that the case or not?
Oh, I see. It should say 'not' :-) I will send a new patch.
- Simon

On Fri, Sep 20, 2024 at 06:04:01PM +0200, Simon Glass wrote:
Hi Tom,
On Fri, 20 Sept 2024 at 16:59, Tom Rini trini@konsulko.com wrote:
On Fri, Sep 20, 2024 at 09:25:44AM +0200, Simon Glass wrote:
Hi Tom,
On Thu, 29 Aug 2024 at 19:26, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
Show a bit more information when malloc() space is exhausted and debugging is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
common/malloc_simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 5a8ec538f8f..f2b3dc53689 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align) log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr, gd->malloc_limit); if (new_ptr > gd->malloc_limit) {
log_err("alloc space exhausted\n");
log_err("alloc space exhausted %lx %x\n", new_ptr,
gd->malloc_limit); return NULL; }
Since debugging is enabled you should update the error message to say what the values mean...
OK, will do. This message is shown without debugging enabled, so I do want to keep it short.
The commit message says "and debugging is enabled", so is that the case or not?
Oh, I see. It should say 'not' :-) I will send a new patch.
Thanks!

Hi Tom,
On Fri, 20 Sept 2024 at 18:35, Tom Rini trini@konsulko.com wrote:
On Fri, Sep 20, 2024 at 06:04:01PM +0200, Simon Glass wrote:
Hi Tom,
On Fri, 20 Sept 2024 at 16:59, Tom Rini trini@konsulko.com wrote:
On Fri, Sep 20, 2024 at 09:25:44AM +0200, Simon Glass wrote:
Hi Tom,
On Thu, 29 Aug 2024 at 19:26, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 29, 2024 at 08:57:49AM -0600, Simon Glass wrote:
Show a bit more information when malloc() space is exhausted and debugging is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
common/malloc_simple.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/common/malloc_simple.c b/common/malloc_simple.c index 5a8ec538f8f..f2b3dc53689 100644 --- a/common/malloc_simple.c +++ b/common/malloc_simple.c @@ -26,7 +26,8 @@ static void *alloc_simple(size_t bytes, int align) log_debug("size=%lx, ptr=%lx, limit=%x: ", (ulong)bytes, new_ptr, gd->malloc_limit); if (new_ptr > gd->malloc_limit) {
log_err("alloc space exhausted\n");
log_err("alloc space exhausted %lx %x\n", new_ptr,
gd->malloc_limit); return NULL; }
Since debugging is enabled you should update the error message to say what the values mean...
OK, will do. This message is shown without debugging enabled, so I do want to keep it short.
The commit message says "and debugging is enabled", so is that the case or not?
Oh, I see. It should say 'not' :-) I will send a new patch.
Thanks!
OK done, it is here:
https://patchwork.ozlabs.org/project/uboot/patch/20240921113450.319072-6-sjg...
Regards, Simon

The VPL phase may want to use the smaller malloc() implementation, so add an option for this.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/spl/Kconfig.vpl | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/common/spl/Kconfig.vpl b/common/spl/Kconfig.vpl index d06f36d4ee4..3dc0e95e09f 100644 --- a/common/spl/Kconfig.vpl +++ b/common/spl/Kconfig.vpl @@ -222,6 +222,15 @@ config VPL_SPI_FLASH_SUPPORT lines). This enables the drivers in drivers/mtd/spi as part of a VPL build. This normally requires VPL_SPI_SUPPORT.
+config VPL_SYS_MALLOC_SIMPLE + bool "Only use malloc_simple functions in the VPL" + default y + help + Say Y here to only use the *_simple malloc functions from + malloc_simple.c, rather then using the versions from dlmalloc.c; + this will make the VPL binary smaller at the cost of more heap + usage as the *_simple malloc functions do not re-use free-ed mem. + config VPL_TEXT_BASE hex "VPL Text Base" default 0x0

Add a size limit for VPL, to match those for SPL and TPL
Signed-off-by: Simon Glass sjg@chromium.org ---
common/spl/Kconfig.vpl | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/common/spl/Kconfig.vpl b/common/spl/Kconfig.vpl index 3dc0e95e09f..eb57dfabea5 100644 --- a/common/spl/Kconfig.vpl +++ b/common/spl/Kconfig.vpl @@ -237,6 +237,14 @@ config VPL_TEXT_BASE help The address in memory that VPL will be running from.
+config VPL_MAX_SIZE + hex "Maximum size (in bytes) for the VPL stage" + default 0x2e000 if ROCKCHIP_RK3399 + default 0x0 + help + The maximum size (in bytes) of the TPL stage. This size is determined + by the amount of internal SRAM memory. + config VPL_BINMAN_SYMBOLS bool "Declare binman symbols in VPL" depends on VPL_FRAMEWORK && BINMAN

If a loader returns an error code it is silently ignored. Show a message to at least provide some feedback to the user.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/spl/spl.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index da110ee0783..143ae96baf0 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -613,6 +613,7 @@ static int boot_from_devices(struct spl_image_info *spl_image, for (i = 0; i < count && spl_boot_list[i] != BOOT_DEVICE_NONE; i++) { struct spl_image_loader *loader; int bootdev = spl_boot_list[i]; + int ret;
if (CONFIG_IS_ENABLED(SHOW_ERRORS)) ret = -ENXIO; @@ -632,10 +633,13 @@ static int boot_from_devices(struct spl_image_info *spl_image, "Unsupported Boot Device!\n"); } } - if (loader && - !spl_load_image(spl_image, loader)) { - spl_image->boot_device = bootdev; - return 0; + if (loader) { + ret = spl_load_image(spl_image, loader); + if (!ret) { + spl_image->boot_device = bootdev; + return 0; + } + printf("Error: %d\n", ret); } } }

The current check looks only at SPL, but TPL or VPL might have a different setting. Update the condition.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/spl/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 143ae96baf0..8b868a298f2 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -837,7 +837,7 @@ void board_init_r(gd_t *dummy1, ulong dummy2) */ void preloader_console_init(void) { -#ifdef CONFIG_SPL_SERIAL +#if CONFIG_IS_ENABLED(SERIAL) gd->baudrate = CONFIG_BAUDRATE;
serial_init(); /* serial communications setup */

The current check looks only at SPL, but TPL or VPL might have a different setting. Update the condition.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/spl/spl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 8b868a298f2..5c687cfc73b 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -896,7 +896,7 @@ __weak void spl_relocate_stack_check(void) */ ulong spl_relocate_stack_gd(void) { -#ifdef CONFIG_SPL_STACK_R +#if CONFIG_IS_ENABLED(STACK_R) gd_t *new_gd; ulong ptr = CONFIG_SPL_STACK_R_ADDR;

U-Boot uses ulong for addresses. It is confusing to use uintptr_t in a few places, since it makes people wonder if the types are compatible. Change the few occurences in SPL to use ulong
Signed-off-by: Simon Glass sjg@chromium.org ---
common/spl/spl_atf.c | 36 ++++++++++++++++++------------------ common/spl/spl_fit.c | 2 +- common/spl/spl_legacy.c | 8 ++++---- include/spl.h | 28 ++++++++++++++-------------- 4 files changed, 37 insertions(+), 37 deletions(-)
diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c index 0397b86a33b..8bc5db77395 100644 --- a/common/spl/spl_atf.c +++ b/common/spl/spl_atf.c @@ -41,9 +41,9 @@ struct bl2_to_bl31_params_mem_v2 { struct entry_point_info bl31_ep_info; };
-struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry, - uintptr_t bl33_entry, - uintptr_t fdt_addr) +struct bl31_params *bl2_plat_get_bl31_params_default(ulong bl32_entry, + ulong bl33_entry, + ulong fdt_addr) { static struct bl2_to_bl31_params_mem bl31_params_mem; struct bl31_params *bl2_to_bl31_params; @@ -100,17 +100,17 @@ struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry, return bl2_to_bl31_params; }
-__weak struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry, - uintptr_t bl33_entry, - uintptr_t fdt_addr) +__weak struct bl31_params *bl2_plat_get_bl31_params(ulong bl32_entry, + ulong bl33_entry, + ulong fdt_addr) { return bl2_plat_get_bl31_params_default(bl32_entry, bl33_entry, fdt_addr); }
-struct bl_params *bl2_plat_get_bl31_params_v2_default(uintptr_t bl32_entry, - uintptr_t bl33_entry, - uintptr_t fdt_addr) +struct bl_params *bl2_plat_get_bl31_params_v2_default(ulong bl32_entry, + ulong bl33_entry, + ulong fdt_addr) { static struct bl2_to_bl31_params_mem_v2 bl31_params_mem; struct bl_params *bl_params; @@ -173,9 +173,9 @@ struct bl_params *bl2_plat_get_bl31_params_v2_default(uintptr_t bl32_entry, return bl_params; }
-__weak struct bl_params *bl2_plat_get_bl31_params_v2(uintptr_t bl32_entry, - uintptr_t bl33_entry, - uintptr_t fdt_addr) +__weak struct bl_params *bl2_plat_get_bl31_params_v2(ulong bl32_entry, + ulong bl33_entry, + ulong fdt_addr) { return bl2_plat_get_bl31_params_v2_default(bl32_entry, bl33_entry, fdt_addr); @@ -188,8 +188,8 @@ static inline void raw_write_daif(unsigned int daif)
typedef void __noreturn (*atf_entry_t)(struct bl31_params *params, void *plat_params);
-static void __noreturn bl31_entry(uintptr_t bl31_entry, uintptr_t bl32_entry, - uintptr_t bl33_entry, uintptr_t fdt_addr) +static void __noreturn bl31_entry(ulong bl31_entry, ulong bl32_entry, + ulong bl33_entry, ulong fdt_addr) { atf_entry_t atf_entry = (atf_entry_t)bl31_entry; void *bl31_params; @@ -238,7 +238,7 @@ static int spl_fit_images_find(void *blob, int os) return -FDT_ERR_NOTFOUND; }
-uintptr_t spl_fit_images_get_entry(void *blob, int node) +ulong spl_fit_images_get_entry(void *blob, int node) { ulong val; int ret; @@ -253,10 +253,10 @@ uintptr_t spl_fit_images_get_entry(void *blob, int node)
void __noreturn spl_invoke_atf(struct spl_image_info *spl_image) { - uintptr_t bl32_entry = 0; - uintptr_t bl33_entry = CONFIG_TEXT_BASE; + ulong bl32_entry = 0; + ulong bl33_entry = CONFIG_TEXT_BASE; void *blob = spl_image->fdt_addr; - uintptr_t platform_param = (uintptr_t)blob; + ulong platform_param = (ulong)blob; int node;
/* diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 1ad5a69d807..614dea9ae18 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -862,7 +862,7 @@ int spl_load_fit_image(struct spl_image_info *spl_image, { struct bootm_headers images; const char *fit_uname_config = NULL; - uintptr_t fdt_hack; + ulong fdt_hack; const char *uname; ulong fw_data = 0, dt_data = 0, img_data = 0; ulong fw_len = 0, dt_len = 0, img_len = 0; diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c index a77893455f2..3af7ea33ae9 100644 --- a/common/spl/spl_legacy.c +++ b/common/spl/spl_legacy.c @@ -16,11 +16,11 @@
#define LZMA_LEN (1 << 20)
-static void spl_parse_legacy_validate(uintptr_t start, uintptr_t size) +static void spl_parse_legacy_validate(ulong start, ulong size) { - uintptr_t spl_start = (uintptr_t)_start; - uintptr_t spl_end = (uintptr_t)&_image_binary_end; - uintptr_t end = start + size; + ulong spl_start = (ulong)_start; + ulong spl_end = (ulong)&_image_binary_end; + ulong end = start + size;
if ((start >= spl_start && start < spl_end) || (end > spl_start && end <= spl_end) || diff --git a/include/spl.h b/include/spl.h index de808ccd413..d90eed956af 100644 --- a/include/spl.h +++ b/include/spl.h @@ -256,8 +256,8 @@ enum spl_sandbox_flags { struct spl_image_info { const char *name; u8 os; - uintptr_t load_addr; - uintptr_t entry_point; + ulong load_addr; + ulong entry_point; #if CONFIG_IS_ENABLED(LOAD_FIT) || CONFIG_IS_ENABLED(LOAD_FIT_FULL) void *fdt_addr; #endif @@ -939,9 +939,9 @@ void __noreturn spl_invoke_atf(struct spl_image_info *spl_image); * * Return: bl31 params structure pointer */ -struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry, - uintptr_t bl33_entry, - uintptr_t fdt_addr); +struct bl31_params *bl2_plat_get_bl31_params(ulong bl32_entry, + ulong bl33_entry, + ulong fdt_addr);
/** * bl2_plat_get_bl31_params_default() - prepare params for bl31. @@ -960,9 +960,9 @@ struct bl31_params *bl2_plat_get_bl31_params(uintptr_t bl32_entry, * * Return: bl31 params structure pointer */ -struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry, - uintptr_t bl33_entry, - uintptr_t fdt_addr); +struct bl31_params *bl2_plat_get_bl31_params_default(ulong bl32_entry, + ulong bl33_entry, + ulong fdt_addr);
/** * bl2_plat_get_bl31_params_v2() - return params for bl31 @@ -976,9 +976,9 @@ struct bl31_params *bl2_plat_get_bl31_params_default(uintptr_t bl32_entry, * * Return: bl31 params structure pointer */ -struct bl_params *bl2_plat_get_bl31_params_v2(uintptr_t bl32_entry, - uintptr_t bl33_entry, - uintptr_t fdt_addr); +struct bl_params *bl2_plat_get_bl31_params_v2(ulong bl32_entry, + ulong bl33_entry, + ulong fdt_addr);
/** * bl2_plat_get_bl31_params_v2_default() - prepare params for bl31. @@ -995,9 +995,9 @@ struct bl_params *bl2_plat_get_bl31_params_v2(uintptr_t bl32_entry, * * Return: bl31 params structure pointer */ -struct bl_params *bl2_plat_get_bl31_params_v2_default(uintptr_t bl32_entry, - uintptr_t bl33_entry, - uintptr_t fdt_addr); +struct bl_params *bl2_plat_get_bl31_params_v2_default(ulong bl32_entry, + ulong bl33_entry, + ulong fdt_addr); /** * spl_optee_entry - entry function for optee *

The variable 'ret' is defined twice, which is not intended. This may have been a local merge error.
Signed-off-by: Simon Glass sjg@chromium.org Fixes: 2eefeb6d893 ("spl: Report a loader failure") ---
common/spl/spl.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 5c687cfc73b..d60e60e708e 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -613,7 +613,6 @@ static int boot_from_devices(struct spl_image_info *spl_image, for (i = 0; i < count && spl_boot_list[i] != BOOT_DEVICE_NONE; i++) { struct spl_image_loader *loader; int bootdev = spl_boot_list[i]; - int ret;
if (CONFIG_IS_ENABLED(SHOW_ERRORS)) ret = -ENXIO;

Add debugging of image-loading progress. Fix a stale comment in the function comment while we are here.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/spl/spl_fit.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 614dea9ae18..303d5416b59 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -190,7 +190,7 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size, /** * load_simple_fit(): load the image described in a certain FIT node * @info: points to information about the device to load data from - * @sector: the start sector of the FIT image on the device + * @fit_offset: the offset of the FIT image on the device * @ctx: points to the FIT context structure * @node: offset of the DT node describing the image to load (relative * to @fit) @@ -243,11 +243,14 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset, if (!fit_image_get_data_position(fit, node, &offset)) { external_data = true; } else if (!fit_image_get_data_offset(fit, node, &offset)) { + log_debug("read offset %x = offset from fit %lx\n", + offset, (ulong)offset + ctx->ext_data_offset); offset += ctx->ext_data_offset; external_data = true; }
if (external_data) { + ulong read_offset; void *src_ptr;
/* External data */ @@ -270,6 +273,10 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset,
overhead = get_aligned_image_overhead(info, offset); size = get_aligned_image_size(info, length, offset); + read_offset = fit_offset + get_aligned_image_offset(info, + offset); + log_debug("reading from offset %x / %lx size %lx to %p: ", + offset, read_offset, size, src_ptr);
if (info->read(info, fit_offset + @@ -336,6 +343,7 @@ static int load_simple_fit(struct spl_load_info *info, ulong fit_offset, else image_info->entry_point = FDT_ERROR; } + log_debug("- done loading\n");
upl_add_image(fit, node, load_addr, length);

Add Kconfig symbols and update the Makefile rules so that decompression can be used in TPL and VPL
Signed-off-by: Simon Glass sjg@chromium.org ---
lib/Kconfig | 35 +++++++++++++++++++++++++++++++++++ lib/Makefile | 8 ++++---- 2 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/lib/Kconfig b/lib/Kconfig index 2059219a120..72a5f3cb04f 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -811,12 +811,36 @@ config SPL_LZ4 fast compression and decompression speed. It belongs to the LZ77 family of byte-oriented compression schemes.
+config TPL_LZ4 + bool "Enable LZ4 decompression support in TPL" + depends on TPL + help + This enables support for the LZ4 decompression algorithm in TPL. LZ4 + is a lossless data compression algorithm that is focused on + fast compression and decompression speed. It belongs to the LZ77 + family of byte-oriented compression schemes. + +config VPL_LZ4 + bool "Enable LZ4 decompression support in VPL" + depends on VPL + help + This enables support for the LZ4 decompression algorithm in VPL. LZ4 + is a lossless data compression algorithm that is focused on + fast compression and decompression speed. It belongs to the LZ77 + family of byte-oriented compression schemes. + config SPL_LZMA bool "Enable LZMA decompression support for SPL build" depends on SPL help This enables support for LZMA compression algorithm for SPL boot.
+config TPL_LZMA + bool "Enable LZMA decompression support for TPL build" + depends on TPL + help + This enables support for LZMA compression algorithm for TPL boot. + config VPL_LZMA bool "Enable LZMA decompression support for VPL build" default y if LZMA @@ -835,11 +859,22 @@ config SPL_GZIP help This enables support for the GZIP compression algorithm for SPL boot.
+config TPL_GZIP + bool "Enable gzip decompression support for SPL build" + select TPL_ZLIB + help + This enables support for the GZIP compression algorithm for TPL + config SPL_ZLIB bool help This enables compression lib for SPL boot.
+config TPL_ZLIB + bool + help + This enables compression lib for TPL + config SPL_ZSTD bool "Enable Zstandard decompression support in SPL" depends on SPL diff --git a/lib/Makefile b/lib/Makefile index 81b503ab526..d122f764f9f 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -82,12 +82,12 @@ obj-$(CONFIG_$(SPL_)SHA512) += sha512.o obj-$(CONFIG_CRYPT_PW) += crypt/ obj-$(CONFIG_$(SPL_)ASN1_DECODER) += asn1_decoder.o
-obj-$(CONFIG_$(SPL_)ZLIB) += zlib/ +obj-$(CONFIG_$(SPL_TPL_)ZLIB) += zlib/ obj-$(CONFIG_$(SPL_)ZSTD) += zstd/ -obj-$(CONFIG_$(SPL_)GZIP) += gunzip.o +obj-$(CONFIG_$(SPL_TPL_)GZIP) += gunzip.o obj-$(CONFIG_$(SPL_)LZO) += lzo/ -obj-$(CONFIG_$(SPL_)LZMA) += lzma/ -obj-$(CONFIG_$(SPL_)LZ4) += lz4_wrapper.o +obj-$(CONFIG_$(SPL_TPL_)LZMA) += lzma/ +obj-$(CONFIG_$(SPL_TPL_)LZ4) += lz4_wrapper.o
obj-$(CONFIG_$(SPL_)LIB_RATIONAL) += rational.o

With VBE we want to use FIT in all phases of the boot. Add Kconfig options to support this.
Disable the options for sandbox_vpl for now.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/Kconfig | 69 ++++++++++++++++++++++++++++++++++- boot/Makefile | 4 +- configs/sandbox_vpl_defconfig | 3 +- 3 files changed, 71 insertions(+), 5 deletions(-)
diff --git a/boot/Kconfig b/boot/Kconfig index 7ac34574079..afe04c7327a 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -161,6 +161,18 @@ config SPL_FIT select SPL_HASH select SPL_OF_LIBFDT
+config VPL_FIT + bool "Support Flattened Image Tree within VPL" + depends on VPL + select VPL_HASH + select VPL_OF_LIBFDT + +config TPL_FIT + bool "Support Flattened Image Tree within TPL" + depends on TPL + select TPL_HASH + select TPL_OF_LIBFDT + config SPL_FIT_PRINT bool "Support FIT printing within SPL" depends on SPL_FIT @@ -265,6 +277,28 @@ config SPL_LOAD_FIT_FULL particular it can handle selecting from multiple device tree and passing the correct one to U-Boot.
+config TPL_LOAD_FIT + bool "Enable TPL loading U-Boot as a FIT (basic fitImage features)" + depends on TPL + select TPL_FIT + help + Normally with the SPL framework a legacy image is generated as part + of the build. This contains U-Boot along with information as to + where it should be loaded. This option instead enables generation + of a FIT (Flat Image Tree) which provides more flexibility. In + particular it can handle selecting from multiple device tree + and passing the correct one to U-Boot. + + This path has the following limitations: + + 1. "loadables" images, other than FDTs, which do not have a "load" + property will not be loaded. This limitation also applies to FPGA + images with the correct "compatible" string. + 2. For FPGA images, the supported "compatible" list is in the + doc/uImage.FIT/source_file_format.txt. + 3. FDTs are only loaded for images with an "os" property of "u-boot". + "linux" images are also supported with Falcon boot mode. + config SPL_FIT_IMAGE_POST_PROCESS bool "Enable post-processing of FIT artifacts after loading by the SPL" depends on SPL_LOAD_FIT @@ -312,6 +346,22 @@ config VPL_FIT select VPL_HASH select VPL_OF_LIBFDT
+config VPL_LOAD_FIT + bool "Enable VPL loading U-Boot as a FIT (basic fitImage features)" + select VPL_FIT + default y + +config VPL_LOAD_FIT_FULL + bool "Enable SPL loading U-Boot as a FIT (full fitImage features)" + select VPL_FIT + help + Normally with the SPL framework a legacy image is generated as part + of the build. This contains U-Boot along with information as to + where it should be loaded. This option instead enables generation + of a FIT (Flat Image Tree) which provides more flexibility. In + particular it can handle selecting from multiple device tree + and passing the correct one to U-Boot. + config VPL_FIT_PRINT bool "Support FIT printing within VPL" depends on VPL_FIT @@ -632,6 +682,15 @@ config VPL_BOOTMETH_VBE supports selection of various firmware components, selection of an OS to boot as well as updating these using fwupd.
+config TPL_BOOTMETH_VBE + bool "Bootdev support for Verified Boot for Embedded (TPL)" + depends on TPL + default y + help + Enables support for VBE boot. This is a standard boot method which + supports selection of various firmware components, seleciton of an OS to + boot as well as updating these using fwupd. + if BOOTMETH_VBE
config BOOTMETH_VBE_REQUEST @@ -710,7 +769,15 @@ config VPL_BOOTMETH_VBE_SIMPLE_FW This option enabled for VPL, since it is the phase where the SPL decision is made.
-endif # BOOTMETH_VBE +config TPL_BOOTMETH_VBE_SIMPLE_FW + bool "Bootdev support for VBE 'simple' method firmware phase (TPL)" + depends on VPL + default y + help + Enables support for the firmware parts of VBE 'simple' boot, in TPL. + TPL loads a FIT containing the VPL binary and a suitable devicetree. + +endif # BOOTMETH_VBE_SIMPLE
config EXPO bool "Support for expos - groups of scenes displaying a UI" diff --git a/boot/Makefile b/boot/Makefile index f4675d6ffd5..40e2337de0f 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -58,9 +58,7 @@ obj-$(CONFIG_$(SPL_TPL_)FIT_CIPHER) += image-cipher.o
obj-$(CONFIG_CMD_ADTIMG) += image-android-dt.o
-ifdef CONFIG_SPL_BUILD -obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o -endif +obj-$(CONFIG_$(SPL_TPL_)LOAD_FIT) += common_fit.o
obj-$(CONFIG_$(SPL_TPL_)EXPO) += expo.o scene.o expo_build.o obj-$(CONFIG_$(SPL_TPL_)EXPO) += scene_menu.o scene_textline.o diff --git a/configs/sandbox_vpl_defconfig b/configs/sandbox_vpl_defconfig index 96e9211bd19..d18bb7a0353 100644 --- a/configs/sandbox_vpl_defconfig +++ b/configs/sandbox_vpl_defconfig @@ -27,9 +27,10 @@ CONFIG_FIT=y CONFIG_FIT_VERBOSE=y CONFIG_FIT_BEST_MATCH=y CONFIG_SPL_LOAD_FIT=y +# CONFIG_TPL_BOOTMETH_VBE is not set +# CONFIG_TPL_BOOTMETH_VBE_SIMPLE_FW is not set CONFIG_UPL=y CONFIG_UPL_IN=y -CONFIG_SPL_UPL_OUT=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y CONFIG_BOOTSTAGE_FDT=y

Provide options to enable the CRC8 feature in TPL and VPL builds.
Signed-off-by: Simon Glass sjg@chromium.org ---
lib/Kconfig | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/lib/Kconfig b/lib/Kconfig index 72a5f3cb04f..b2d5e8d04a7 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -698,6 +698,24 @@ config SPL_CRC8 checksum with feedback to produce an 8-bit result. The code is small and it does not require a lookup table (unlike CRC32).
+config TPL_CRC8 + bool "Support CRC8 in TPL" + depends on SPL + help + Enables CRC8 support in TPL. This is not normally required. CRC8 is + a simple and fast checksumming algorithm which does a bytewise + checksum with feedback to produce an 8-bit result. The code is small + and it does not require a lookup table (unlike CRC32). + +config VPL_CRC8 + bool "Support CRC8 in VPL" + depends on VPL + help + Enables CRC8 support in VPL. This is not normally required. CRC8 is + a simple and fast checksumming algorithm which does a bytewise + checksum with feedback to produce an 8-bit result. The code is small + and it does not require a lookup table (unlike CRC32). + config SPL_CRC16 bool "Support CRC16 in SPL" depends on SPL

On Thu, Aug 29, 2024 at 08:58:00AM -0600, Simon Glass wrote:
Provide options to enable the CRC8 feature in TPL and VPL builds.
Signed-off-by: Simon Glass sjg@chromium.org
lib/Kconfig | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/lib/Kconfig b/lib/Kconfig index 72a5f3cb04f..b2d5e8d04a7 100644 --- a/lib/Kconfig +++ b/lib/Kconfig @@ -698,6 +698,24 @@ config SPL_CRC8 checksum with feedback to produce an 8-bit result. The code is small and it does not require a lookup table (unlike CRC32).
+config TPL_CRC8
- bool "Support CRC8 in TPL"
- depends on SPL
Should be TPL not SPL here I assume.

VBE uses a crc8 checksum to verify that the nvdata is valid, so make sure it is available if VBE is enabled.
Signed-off-by: Simon Glass sjg@chromium.org ---
boot/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/boot/Kconfig b/boot/Kconfig index afe04c7327a..49e99e5e155 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -716,6 +716,8 @@ config SPL_BOOTMETH_VBE_REQUEST config BOOTMETH_VBE_SIMPLE bool "Bootdev support for VBE 'simple' method" default y + imply SPL_CRC8 + imply VPL_CRC8 help Enables support for VBE 'simple' boot. This allows updating a single firmware image in boot media such as MMC. It does not support any sort

On Thu, Aug 29, 2024 at 08:58:01AM -0600, Simon Glass wrote:
VBE uses a crc8 checksum to verify that the nvdata is valid, so make sure it is available if VBE is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
boot/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/boot/Kconfig b/boot/Kconfig index afe04c7327a..49e99e5e155 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -716,6 +716,8 @@ config SPL_BOOTMETH_VBE_REQUEST config BOOTMETH_VBE_SIMPLE bool "Bootdev support for VBE 'simple' method" default y
- imply SPL_CRC8
- imply VPL_CRC8 help Enables support for VBE 'simple' boot. This allows updating a single firmware image in boot media such as MMC. It does not support any sort
I really worry the lack of "if SPL" / "if VPL" here leads to Kconfig warnings that CI doesn't pick up.

Hi Tom,
On Thu, 29 Aug 2024 at 19:31, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 29, 2024 at 08:58:01AM -0600, Simon Glass wrote:
VBE uses a crc8 checksum to verify that the nvdata is valid, so make sure it is available if VBE is enabled.
Signed-off-by: Simon Glass sjg@chromium.org
boot/Kconfig | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/boot/Kconfig b/boot/Kconfig index afe04c7327a..49e99e5e155 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -716,6 +716,8 @@ config SPL_BOOTMETH_VBE_REQUEST config BOOTMETH_VBE_SIMPLE bool "Bootdev support for VBE 'simple' method" default y
imply SPL_CRC8
imply VPL_CRC8 help Enables support for VBE 'simple' boot. This allows updating a single firmware image in boot media such as MMC. It does not support any sort
I really worry the lack of "if SPL" / "if VPL" here leads to Kconfig warnings that CI doesn't pick up.
So far, BOOTMETH_VBE requires SPL and VPL, but I've added the condition just to be safe.
Regards, Simon

Add an entry for crc8, with watchdog handling.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/hash.c | 8 ++++++++ include/u-boot/crc.h | 3 +++ lib/crc8.c | 6 ++++++ 3 files changed, 17 insertions(+)
diff --git a/common/hash.c b/common/hash.c index ac63803fed9..43c4e185750 100644 --- a/common/hash.c +++ b/common/hash.c @@ -304,6 +304,14 @@ static struct hash_algo hash_algo[] = { .hash_update = hash_update_crc16_ccitt, .hash_finish = hash_finish_crc16_ccitt, }, +#if CONFIG_IS_ENABLED(CRC8) + { + .name = "crc8", + .digest_size = 1, + .chunk_size = CHUNKSZ_CRC32, + .hash_func_ws = crc8_wd_buf, + }, +#endif #if CONFIG_IS_ENABLED(CRC32) { .name = "crc32", diff --git a/include/u-boot/crc.h b/include/u-boot/crc.h index 5174bd7ac41..b2badaf6a97 100644 --- a/include/u-boot/crc.h +++ b/include/u-boot/crc.h @@ -25,6 +25,9 @@ */ unsigned int crc8(unsigned int crc_start, const unsigned char *vptr, int len);
+void crc8_wd_buf(const unsigned char *input, unsigned int len, + unsigned char output[1], unsigned int chunk_sz); + /* lib/crc16.c - 16 bit CRC with polynomial x^16 + x^15 + x^2 + 1 */ uint16_t crc16(uint16_t crc, const unsigned char *buffer, size_t len);
diff --git a/lib/crc8.c b/lib/crc8.c index 20d46d16147..811e19917b4 100644 --- a/lib/crc8.c +++ b/lib/crc8.c @@ -32,3 +32,9 @@ unsigned int crc8(unsigned int crc, const unsigned char *vptr, int len)
return crc; } + +void crc8_wd_buf(const unsigned char *input, unsigned int len, + unsigned char output[1], unsigned int chunk_sz) +{ + *output = crc8(0, input, len); +}

On Thu, 29 Aug 2024 at 16:02, Simon Glass sjg@chromium.org wrote:
Add an entry for crc8, with watchdog handling.
What's the watchdog handling do? What's this used for?
Looking at the use of crc8 it's minimally used.
Signed-off-by: Simon Glass sjg@chromium.org
common/hash.c | 8 ++++++++ include/u-boot/crc.h | 3 +++ lib/crc8.c | 6 ++++++ 3 files changed, 17 insertions(+)
diff --git a/common/hash.c b/common/hash.c index ac63803fed9..43c4e185750 100644 --- a/common/hash.c +++ b/common/hash.c @@ -304,6 +304,14 @@ static struct hash_algo hash_algo[] = { .hash_update = hash_update_crc16_ccitt, .hash_finish = hash_finish_crc16_ccitt, }, +#if CONFIG_IS_ENABLED(CRC8)
{
.name = "crc8",
.digest_size = 1,
.chunk_size = CHUNKSZ_CRC32,
.hash_func_ws = crc8_wd_buf,
},
+#endif #if CONFIG_IS_ENABLED(CRC32) { .name = "crc32", diff --git a/include/u-boot/crc.h b/include/u-boot/crc.h index 5174bd7ac41..b2badaf6a97 100644 --- a/include/u-boot/crc.h +++ b/include/u-boot/crc.h @@ -25,6 +25,9 @@ */ unsigned int crc8(unsigned int crc_start, const unsigned char *vptr, int len);
+void crc8_wd_buf(const unsigned char *input, unsigned int len,
unsigned char output[1], unsigned int chunk_sz);
/* lib/crc16.c - 16 bit CRC with polynomial x^16 + x^15 + x^2 + 1 */ uint16_t crc16(uint16_t crc, const unsigned char *buffer, size_t len);
diff --git a/lib/crc8.c b/lib/crc8.c index 20d46d16147..811e19917b4 100644 --- a/lib/crc8.c +++ b/lib/crc8.c @@ -32,3 +32,9 @@ unsigned int crc8(unsigned int crc, const unsigned char *vptr, int len)
return crc;
}
+void crc8_wd_buf(const unsigned char *input, unsigned int len,
unsigned char output[1], unsigned int chunk_sz)
+{
*output = crc8(0, input, len);
+}
2.34.1

Hi Peter,
On Fri, 30 Aug 2024 at 06:17, Peter Robinson pbrobinson@gmail.com wrote:
On Thu, 29 Aug 2024 at 16:02, Simon Glass sjg@chromium.org wrote:
Add an entry for crc8, with watchdog handling.
What's the watchdog handling do? What's this used for?
It does the hashing in chunks and touches the watchdog after every chunk, to avoid the board resetting.
Looking at the use of crc8 it's minimally used.
Yes, it's a cheap algorithm which provides some protection, so I am using it in the VBE relocating loader (to come).
Signed-off-by: Simon Glass sjg@chromium.org
common/hash.c | 8 ++++++++ include/u-boot/crc.h | 3 +++ lib/crc8.c | 6 ++++++ 3 files changed, 17 insertions(+)
diff --git a/common/hash.c b/common/hash.c index ac63803fed9..43c4e185750 100644 --- a/common/hash.c +++ b/common/hash.c @@ -304,6 +304,14 @@ static struct hash_algo hash_algo[] = { .hash_update = hash_update_crc16_ccitt, .hash_finish = hash_finish_crc16_ccitt, }, +#if CONFIG_IS_ENABLED(CRC8)
{
.name = "crc8",
.digest_size = 1,
.chunk_size = CHUNKSZ_CRC32,
.hash_func_ws = crc8_wd_buf,
},
+#endif #if CONFIG_IS_ENABLED(CRC32) { .name = "crc32", diff --git a/include/u-boot/crc.h b/include/u-boot/crc.h index 5174bd7ac41..b2badaf6a97 100644 --- a/include/u-boot/crc.h +++ b/include/u-boot/crc.h @@ -25,6 +25,9 @@ */ unsigned int crc8(unsigned int crc_start, const unsigned char *vptr, int len);
+void crc8_wd_buf(const unsigned char *input, unsigned int len,
unsigned char output[1], unsigned int chunk_sz);
/* lib/crc16.c - 16 bit CRC with polynomial x^16 + x^15 + x^2 + 1 */ uint16_t crc16(uint16_t crc, const unsigned char *buffer, size_t len);
diff --git a/lib/crc8.c b/lib/crc8.c index 20d46d16147..811e19917b4 100644 --- a/lib/crc8.c +++ b/lib/crc8.c @@ -32,3 +32,9 @@ unsigned int crc8(unsigned int crc, const unsigned char *vptr, int len)
return crc;
}
+void crc8_wd_buf(const unsigned char *input, unsigned int len,
unsigned char output[1], unsigned int chunk_sz)
+{
*output = crc8(0, input, len);
+}
2.34.1
Regards, Simon

On Thu, Aug 29, 2024 at 08:57:43AM -0600, Simon Glass wrote:
This includes various patches towards implementing the VBE abrec bootmeth in U-Boot. It mostly focuses on SPL tweaks and adjusting what fatures are available in VPL.
A lot of this series makes me really want to stop and rework how we configure / build things. Having say ZLIB, SPL_ZLIB, TPL_ZLIB and VPL_ZLIB really isn't good.

Hi Tom,
On Thu, 29 Aug 2024 at 12:33, Tom Rini trini@konsulko.com wrote:
On Thu, Aug 29, 2024 at 08:57:43AM -0600, Simon Glass wrote:
This includes various patches towards implementing the VBE abrec bootmeth in U-Boot. It mostly focuses on SPL tweaks and adjusting what fatures are available in VPL.
A lot of this series makes me really want to stop and rework how we configure / build things. Having say ZLIB, SPL_ZLIB, TPL_ZLIB and VPL_ZLIB really isn't good.
Yup and I had a solution or that a year or two ago and it died.
Any revival of that, or new solution will have to wait until VBE abrec is done. I hope there is only one more series after this, then a small one for the VBE thing itself.
Regards, Simon

Hi Simon,
On 29/08/2024 15:57, Simon Glass wrote:
This includes various patches towards implementing the VBE abrec bootmeth in U-Boot. It mostly focuses on SPL tweaks and adjusting what fatures are available in VPL.
It would be helpful if you could include some context as to what VBE is, how this series fits in to previous ones, etc... If this is part of some overarching changes then having some common background justification / context which is copy-pasted to all the relevant patch series would make sense imo.
Thanks and regards,
Simon Glass (19): image: Add a prototype for fit_image_get_phase() serial: ns16550: Allow clocks to be missing boot: Allow FIT to fall back from best-match option bootstd: Avoid sprintf() in SPL when creating bootdevs boot: Respect the load_op in fit_image_load() malloc: Show amount of used space when memory runs out malloc: Provide a simple malloc for VPL Support setting a maximum size for the VPL image spl: Report a loader failure spl: Allow serial to be disabled in any XPL phase spl: Support a relocated stack in any XPL phase spl: Drop use of uintptr_t spl: Drop a duplicate variable in boot_from_devices() spl: Add some more debugging to load_simple_fit() spl: lib: Allow for decompression in any SPL build boot: Allow use of FIT in TPL and VPL lib: Allow crc8 in TPL and VPL boot: Imply CRC8 with VBE hash: Plumb crc8 into the hash functions
boot/Kconfig | 71 ++++++++++++++++++++++++++++++++++- boot/Makefile | 4 +- boot/bootdev-uclass.c | 10 ++++- boot/image-fit.c | 29 ++++++++------ common/hash.c | 8 ++++ common/malloc_simple.c | 3 +- common/spl/Kconfig.vpl | 17 +++++++++ common/spl/spl.c | 15 +++++--- common/spl/spl_atf.c | 36 +++++++++--------- common/spl/spl_fit.c | 12 +++++- common/spl/spl_legacy.c | 8 ++-- configs/sandbox_vpl_defconfig | 3 +- drivers/serial/ns16550.c | 2 +- include/image.h | 16 +++++++- include/spl.h | 28 +++++++------- include/u-boot/crc.h | 3 ++ lib/Kconfig | 53 ++++++++++++++++++++++++++ lib/Makefile | 8 ++-- lib/crc8.c | 6 +++ 19 files changed, 263 insertions(+), 69 deletions(-)

Hi Caleb,
On Fri, 30 Aug 2024 at 14:18, Caleb Connolly caleb.connolly@linaro.org wrote:
Hi Simon,
On 29/08/2024 15:57, Simon Glass wrote:
This includes various patches towards implementing the VBE abrec bootmeth in U-Boot. It mostly focuses on SPL tweaks and adjusting what fatures are available in VPL.
It would be helpful if you could include some context as to what VBE is, how this series fits in to previous ones, etc... If this is part of some overarching changes then having some common background justification / context which is copy-pasted to all the relevant patch series would make sense imo.
Yes, please see [1]. It is essentially an effort to replace UEFI with something simpler, more efficient, better-specified and easier to test.
Regards, Simon
Thanks and regards,
Simon Glass (19): image: Add a prototype for fit_image_get_phase() serial: ns16550: Allow clocks to be missing boot: Allow FIT to fall back from best-match option bootstd: Avoid sprintf() in SPL when creating bootdevs boot: Respect the load_op in fit_image_load() malloc: Show amount of used space when memory runs out malloc: Provide a simple malloc for VPL Support setting a maximum size for the VPL image spl: Report a loader failure spl: Allow serial to be disabled in any XPL phase spl: Support a relocated stack in any XPL phase spl: Drop use of uintptr_t spl: Drop a duplicate variable in boot_from_devices() spl: Add some more debugging to load_simple_fit() spl: lib: Allow for decompression in any SPL build boot: Allow use of FIT in TPL and VPL lib: Allow crc8 in TPL and VPL boot: Imply CRC8 with VBE hash: Plumb crc8 into the hash functions
boot/Kconfig | 71 ++++++++++++++++++++++++++++++++++- boot/Makefile | 4 +- boot/bootdev-uclass.c | 10 ++++- boot/image-fit.c | 29 ++++++++------ common/hash.c | 8 ++++ common/malloc_simple.c | 3 +- common/spl/Kconfig.vpl | 17 +++++++++ common/spl/spl.c | 15 +++++--- common/spl/spl_atf.c | 36 +++++++++--------- common/spl/spl_fit.c | 12 +++++- common/spl/spl_legacy.c | 8 ++-- configs/sandbox_vpl_defconfig | 3 +- drivers/serial/ns16550.c | 2 +- include/image.h | 16 +++++++- include/spl.h | 28 +++++++------- include/u-boot/crc.h | 3 ++ lib/Kconfig | 53 ++++++++++++++++++++++++++ lib/Makefile | 8 ++-- lib/crc8.c | 6 +++ 19 files changed, 263 insertions(+), 69 deletions(-)
-- // Caleb (they/them)
participants (4)
-
Caleb Connolly
-
Peter Robinson
-
Simon Glass
-
Tom Rini