[PATCH v2 0/6] SPL: FIT: Fix some omissions of SPL_LOAD_FIT_FULL and bootm
Although this series focuses on SPL_LOAD_FIT_FULL, some of the fixes will also apply to bootm, as they are sharing significant amounts of code.
Originally SPL_LOAD_FIT_FULL could not start either a linux FIT or a u-boot image. It didn't even take FIT images generated automatically by mkimage, as part of the u-boot build!!! The goal is to at the very least, be able to boot autogenerated mkimage FITs.
This brings us much more in line with SPL_LOAD_FIT, and the documentation. It's not perfect, and the fpga 'compatible =' support is still not implemented. That's all I currently have time for before someone notices I'm working on u-boot again.
Changes since v1: - Update commit message of "spl: LOAD_FIT_FULL: Fix selection of the "fdt" node"
Alexandru Gagniuc (6): spl: LOAD_FIT_FULL: Fix selection of the "fdt" node spl: LOAD_FIT_FULL: Do not hard-code os to IH_OS_U_BOOT spl: LOAD_FIT_FULL: Relocate FDT for u-boot payloads spl: LOAD_FIT_FULL: Support 'kernel' and 'firmware' properties image-fit: Accept IH_TYPE_FIRMWARE in fit_image_load() as valid image-fit: Accept OP-TEE images when booting a FIT
common/image-fit.c | 4 ++++ common/spl/spl.c | 40 +++++++++++++++++++++++++++++++++------- 2 files changed, 37 insertions(+), 7 deletions(-)
The correct FDT to use is described by the "fdt" property of the configuration node. When the fit_unamep argument to fit_image_load() is "fdt", we get the "/images/fdt" node. This is incorrect, as it ignores the "fdt" property of the config node, and in most cases, the "/images/fdt" node doesn't exist.
Use NULL for the 'fit_unamep' argument. With NULL, fit_image_load() uses the IH_TYPE_FLATDT value to read the config property "fdt", which points to the correct FDT node(s).
fit_image_load() should probably be split into a function that reads an image by name, and one that reads an image by config reference. I don't make those decisions, I just point out the craziness.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com Reviewed-by: Simon Glass sjg@chromium.org --- common/spl/spl.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index e3d84082f4..986cfbc6fd 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -201,7 +201,6 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, { bootm_headers_t images; const char *fit_uname_config = NULL; - const char *fit_uname_fdt = FIT_FDT_PROP; const char *uname; ulong fw_data = 0, dt_data = 0, img_data = 0; ulong fw_len = 0, dt_len = 0, img_len = 0; @@ -230,8 +229,7 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, #ifdef CONFIG_SPL_FIT_SIGNATURE images.verify = 1; #endif - ret = fit_image_load(&images, (ulong)header, - &fit_uname_fdt, &fit_uname_config, + ret = fit_image_load(&images, (ulong)header, NULL, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_FLATDT, -1, FIT_LOAD_OPTIONAL, &dt_data, &dt_len); if (ret >= 0)
The information on the OS should be contained in the FIT, as the self-explanatory "os" property of a node under /images. Hard-coding this to U_BOOT might send us down the wrong path later in the boot process.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/spl/spl.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 986cfbc6fd..8f6c8dba6f 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -220,8 +220,9 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, spl_image->size = fw_len; spl_image->entry_point = fw_data; spl_image->load_addr = fw_data; - spl_image->os = IH_OS_U_BOOT; - spl_image->name = "U-Boot"; + if (!fit_image_get_os(header, ret, &spl_image->os)) + spl_image->os = IH_OS_INVALID; + spl_image->name = genimg_get_os_name(spl_image->os);
debug(SPL_TPL_PROMPT "payload image: %32s load addr: 0x%lx size: %d\n", spl_image->name, spl_image->load_addr, spl_image->size);
U-Boot expects the FDT to be located right after the _end linker symbol (see fdtdec.c: board_fdt_blob_setup())
The "basic" LOAD_FIT path is aware of this limitation, and relocates the FDT at the expected location. Guessing the expected location probably only works reliably on 32-bit arm, and it feels like a hack. One proposal would be to pass the FDT address to u-boot (e.g. using 'r2' on arm platforms).
The variable is named "fdt_hack" to remind future contributors that, "hey! we should fix the underlying problem". However, that is beyond the scope of this patch.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/spl/spl.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index 8f6c8dba6f..e63f05bb33 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -201,6 +201,7 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, { bootm_headers_t images; const char *fit_uname_config = NULL; + uintptr_t 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; @@ -233,9 +234,18 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, ret = fit_image_load(&images, (ulong)header, NULL, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_FLATDT, -1, FIT_LOAD_OPTIONAL, &dt_data, &dt_len); - if (ret >= 0) + if (ret >= 0) { spl_image->fdt_addr = (void *)dt_data;
+ if (spl_image->os == IH_OS_U_BOOT) { + /* HACK: U-boot expects FDT at a specific address */ + fdt_hack = spl_image->load_addr + spl_image->size; + fdt_hack = (fdt_hack + 3) & ~3; + debug("Relocating FDT to %p\n", spl_image->fdt_addr); + memcpy((void *)fdt_hack, spl_image->fdt_addr, dt_len); + } + } + conf_noffset = fit_conf_get_node((const void *)header, fit_uname_config); if (conf_noffset <= 0)
The 'firmware' property of a config node takes precedence over the 'kernel' property. 'standalone' is deprecated. However, give users a couple of releases where 'standalone' still works, but warns loudly.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com --- common/spl/spl.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/common/spl/spl.c b/common/spl/spl.c index e63f05bb33..da4751b4ac 100644 --- a/common/spl/spl.c +++ b/common/spl/spl.c @@ -214,7 +214,24 @@ static int spl_load_fit_image(struct spl_image_info *spl_image, ret = fit_image_load(&images, (ulong)header, NULL, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_STANDALONE, -1, - FIT_LOAD_REQUIRED, &fw_data, &fw_len); + FIT_LOAD_OPTIONAL, &fw_data, &fw_len); + if (ret >= 0) { + printf("DEPRECATED: 'standalone = ' property."); + printf("Please use either 'firmware =' or 'kernel ='\n"); + } else { + ret = fit_image_load(&images, (ulong)header, NULL, + &fit_uname_config, IH_ARCH_DEFAULT, + IH_TYPE_FIRMWARE, -1, FIT_LOAD_OPTIONAL, + &fw_data, &fw_len); + } + + if (ret < 0) { + ret = fit_image_load(&images, (ulong)header, NULL, + &fit_uname_config, IH_ARCH_DEFAULT, + IH_TYPE_KERNEL, -1, FIT_LOAD_OPTIONAL, + &fw_data, &fw_len); + } + if (ret < 0) return ret;
Consider the following FIT:
images { whipple {}; }; configurations { conf-1 { firmware = "whipple"; }; };
Getting the 'firmware' image with fit_image_load() is not possible, as it doesn't understand 'firmware =' properties. Although one could pass IH_TYPE_FIRMWARE for 'image_type', this needs to be converted to a "firmware" string for FDT lookup -- exactly what this change does.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com Reviewed-by: Simon Glass sjg@chromium.org --- common/image-fit.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/common/image-fit.c b/common/image-fit.c index 94501b1071..970e3f89da 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1955,6 +1955,8 @@ static const char *fit_get_image_type_property(int type) return FIT_FDT_PROP; case IH_TYPE_KERNEL: return FIT_KERNEL_PROP; + case IH_TYPE_FIRMWARE: + return FIT_FIRMWARE_PROP; case IH_TYPE_RAMDISK: return FIT_RAMDISK_PROP; case IH_TYPE_X86_SETUP:
OP-TEE images are normally packaged with type = "tee; os = "tee";
However, fit_image_load() thinks that is somehow invalid. However if they were declared as type = "kernel", os = "linux", fit_image_load() would happily accept them and allow the boot to continue. There is no technical limitation to excluding "tee".
Allowing "tee" images is useful in a boot flow where OP-TEE is executed before linux.
In fact, I think it's unintuitive for a "load"ing function to also do parsing and contain a bunch ad-hoc heuristics that only its caller might know. But I don't make the rules, I just write fixes. In more polite terms: refactoring the fit_image API is beyond the scope of this change.
Signed-off-by: Alexandru Gagniuc mr.nuke.me@gmail.com Reviewed-by: Simon Glass sjg@chromium.org --- common/image-fit.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/common/image-fit.c b/common/image-fit.c index 970e3f89da..ee51f5e76f 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -2089,6 +2089,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, bootstage_mark(bootstage_id + BOOTSTAGE_SUB_CHECK_ALL); type_ok = fit_image_check_type(fit, noffset, image_type) || fit_image_check_type(fit, noffset, IH_TYPE_FIRMWARE) || + fit_image_check_type(fit, noffset, IH_TYPE_TEE) || (image_type == IH_TYPE_KERNEL && fit_image_check_type(fit, noffset, IH_TYPE_KERNEL_NOLOAD));
@@ -2096,6 +2097,7 @@ int fit_image_load(bootm_headers_t *images, ulong addr, image_type == IH_TYPE_FPGA || fit_image_check_os(fit, noffset, IH_OS_LINUX) || fit_image_check_os(fit, noffset, IH_OS_U_BOOT) || + fit_image_check_os(fit, noffset, IH_OS_TEE) || fit_image_check_os(fit, noffset, IH_OS_OPENRTOS) || fit_image_check_os(fit, noffset, IH_OS_EFI) || fit_image_check_os(fit, noffset, IH_OS_VXWORKS);
participants (1)
-
Alexandru Gagniuc