[PATCH v2 1/2] cmd: source: Use script from default config
When sourcing FIT images, source the script node from the "bootscr" property within the default configuration. If board_fit_config_name_match is overridden, this will determine the selected configuration rather than the default one.
The old behaviour is inconsistent with the FIT image specification which does not mention a "default" property in the "/images" node.
Signed-off-by: Sven Schwermer sven@svenschwermer.de ---
(no changes since v1)
boot/Makefile | 8 ++------ cmd/source.c | 22 +++++++++++++--------- include/image.h | 1 + 3 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/boot/Makefile b/boot/Makefile index 2938c3f145..51181b10fc 100644 --- a/boot/Makefile +++ b/boot/Makefile @@ -24,14 +24,10 @@ obj-$(CONFIG_ANDROID_AB) += android_ab.o obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.o image-android-dt.o obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += image-fdt.o obj-$(CONFIG_$(SPL_TPL_)FIT_SIGNATURE) += fdt_region.o -obj-$(CONFIG_$(SPL_TPL_)FIT) += image-fit.o -obj-$(CONFIG_$(SPL_)MULTI_DTB_FIT) += boot_fit.o common_fit.o +obj-$(CONFIG_$(SPL_TPL_)FIT) += common_fit.o image-fit.o +obj-$(CONFIG_$(SPL_)MULTI_DTB_FIT) += boot_fit.o obj-$(CONFIG_$(SPL_TPL_)IMAGE_SIGN_INFO) += image-sig.o obj-$(CONFIG_$(SPL_TPL_)FIT_SIGNATURE) += image-fit-sig.o 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 diff --git a/cmd/source.c b/cmd/source.c index 81e015b64e..b78cac7d5c 100644 --- a/cmd/source.c +++ b/cmd/source.c @@ -26,19 +26,23 @@
#if defined(CONFIG_FIT) /** - * get_default_image() - Return default property from /images + * get_default_bootscr() - Return default boot script unit name. * - * Return: Pointer to value of default property (or NULL) + * The default configuration is used unless board_fit_config_name_match + * is overridden. + * + * Return: Pointer to value of bootscr property (or NULL) */ -static const char *get_default_image(const void *fit) +static const char *get_default_bootscr(const void *fit) { - int images_noffset; + int noffset;
- images_noffset = fdt_path_offset(fit, FIT_IMAGES_PATH); - if (images_noffset < 0) + /* get default or board-specific configuration node */ + noffset = fit_find_config_node(fit); + if (noffset < 0) return NULL;
- return fdt_getprop(fit, images_noffset, FIT_DEFAULT_PROP, NULL); + return fdt_getprop(fit, noffset, FIT_BOOTSCR_PROP, NULL); } #endif
@@ -113,7 +117,7 @@ int image_source_script(ulong addr, const char *fit_uname) }
if (!fit_uname) - fit_uname = get_default_image(fit_hdr); + fit_uname = get_default_bootscr(fit_hdr);
if (!fit_uname) { puts("No FIT subimage unit name\n"); @@ -128,7 +132,7 @@ int image_source_script(ulong addr, const char *fit_uname) }
if (!fit_image_check_type (fit_hdr, noffset, IH_TYPE_SCRIPT)) { - puts ("Not a image image\n"); + puts("Not a script image\n"); return 1; }
diff --git a/include/image.h b/include/image.h index fd662e74b4..67c70952bd 100644 --- a/include/image.h +++ b/include/image.h @@ -936,6 +936,7 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size, #define FIT_KERNEL_PROP "kernel" #define FIT_RAMDISK_PROP "ramdisk" #define FIT_FDT_PROP "fdt" +#define FIT_BOOTSCR_PROP "bootscr" #define FIT_LOADABLE_PROP "loadables" #define FIT_DEFAULT_PROP "default" #define FIT_SETUP_PROP "setup"
"script@1" is not a valid node name if the "reg" property is missing. The yocto build system embeds the boot script as an image that's pointed to by the "bootscr" property within the default configuration. That is what we use here as the fallback when sourcing the "script@1" fails. This implementation should be backward-compatible with existing ITS files.
Signed-off-by: Sven Schwermer sven@svenschwermer.de ---
Changes in v2: - Fixed patman issues
drivers/usb/gadget/f_sdp.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/f_sdp.c b/drivers/usb/gadget/f_sdp.c index e48aa2f90d..d97c017d90 100644 --- a/drivers/usb/gadget/f_sdp.c +++ b/drivers/usb/gadget/f_sdp.c @@ -865,8 +865,13 @@ static int sdp_handle_in_ep(struct spl_image_info *spl_image) spl_parse_image_header(&spl_image, header); jump_to_image_no_args(&spl_image); #else - /* In U-Boot, allow jumps to scripts */ - image_source_script(sdp_func->jmp_address, "script@1"); + /* + * In U-Boot, allow jumps to scripts. Run/retry with default + * configuration if FIT is disabled or script@1 is not found. + */ + if (!IS_ENABLED(CONFIG_FIT) || + image_source_script(sdp_func->jmp_address, "script@1")) + image_source_script(sdp_func->jmp_address, NULL); #endif }
On Sat, Jan 01, 2022 at 07:45:39PM +0100, Sven Schwermer wrote:
When sourcing FIT images, source the script node from the "bootscr" property within the default configuration. If board_fit_config_name_match is overridden, this will determine the selected configuration rather than the default one.
The old behaviour is inconsistent with the FIT image specification which does not mention a "default" property in the "/images" node.
Signed-off-by: Sven Schwermer sven@svenschwermer.de
This ends up being a noticeable size growth on a large number of platforms. Can you please elaborate on the use case here, with some links to the OE layers in question that generate these images? Thanks!
Hi Tom,
I didn't think this would result in a size increase. Could you elaborate?
When generating FIT images with Yocto, you won't get a /images/default property which is inconsistent with the FIT spec. This is an example of how Yocto spits out FIT images which I believe is consistent with the spec:
/dts-v1/; // magic: 0xd00dfeed // totalsize: 0x17a334 (1549108) // off_dt_struct: 0x38 // off_dt_strings: 0x179f64 // off_mem_rsvmap: 0x28 // version: 17 // last_comp_version: 16 // boot_cpuid_phys: 0x0 // size_dt_strings: 0x74 // size_dt_struct: 0x179f2c
/ { timestamp = <0x61a42a3c>; description = "Kernel fitImage for Poky (Yocto Project Reference Distro)/5.14.21+gitAUTOINC+4f4ad2c808_9d5572038e/mys-6ulx"; #address-cells = <0x00000001>; images { kernel-1 { description = "Linux kernel"; data = ... type = "kernel"; arch = "arm"; os = "linux"; compression = "none"; load = <0x82000000>; entry = <0x82000000>; hash-1 { value = <0x582363ee 0x6f47a5c6 0xd55b5ae8 0xaf9d6057 0xc5f4281b 0x2ee22850 0x859691cd 0xdb09e38d>; algo = "sha256"; }; }; fdt-imx6ull-myir-mys-6ulx-eval.dtb { description = "Flattened Device Tree blob"; data = ... type = "flat_dt"; arch = "arm"; compression = "none"; hash-1 { value = <0x5e8b26d4 0xda4bf28c 0x8683bd81 0xaa5fce3c 0x15cf569a 0xb6c66ccf 0xe65f56e1 0x38392bfb>; algo = "sha256"; }; }; bootscr-boot.sh { description = "U-boot script"; data = ... type = "script"; arch = "arm"; compression = "none"; hash-1 { value = <0xf5b7edab 0x6567bce5 0x349c3f10 0xbab82b14 0x062a25e3 0x1dd37bdd 0x83dc2744 0x3326d252>; algo = "sha256"; }; }; }; configurations { default = "conf-imx6ull-myir-mys-6ulx-eval.dtb"; conf-imx6ull-myir-mys-6ulx-eval.dtb { description = "1 Linux kernel, FDT blob, u-boot script"; kernel = "kernel-1"; fdt = "fdt-imx6ull-myir-mys-6ulx-eval.dtb"; bootscr = "bootscr-boot.sh"; hash-1 { algo = "sha256"; }; }; }; };
Best regards, Sven
On 1/18/22 14:29, Tom Rini wrote:
On Sat, Jan 01, 2022 at 07:45:39PM +0100, Sven Schwermer wrote:
When sourcing FIT images, source the script node from the "bootscr" property within the default configuration. If board_fit_config_name_match is overridden, this will determine the selected configuration rather than the default one.
The old behaviour is inconsistent with the FIT image specification which does not mention a "default" property in the "/images" node.
Signed-off-by: Sven Schwermer sven@svenschwermer.de
This ends up being a noticeable size growth on a large number of platforms. Can you please elaborate on the use case here, with some links to the OE layers in question that generate these images? Thanks!
On Sat, Jan 22, 2022 at 09:36:58PM +0100, Sven Schwermer wrote:
Hi Tom,
I didn't think this would result in a size increase. Could you elaborate?
Alright. So for background, if you build with tools/buildman/buildman rather than just "make" directly there's some handy tools like "bloat" detection, which shows function size changes from build A to build B. Doing something like this will show the changes for pine64_plus for example export SOURCE_DATE_EPOCH=`date +%s` ./tools/buildman/buildman -o /tmp/pine64_plus -devl -SBC pine64_plus ./tools/buildman/buildman -o /tmp/pine64_plus -devl -SsB pine64_plus
That shows in the end: aarch64: (for 1/1 boards) all +551.0 rodata +107.0 text +444.0 pine64_plus : all +551 rodata +107 text +444 u-boot: add: 2/0, grow: 0/-1 bytes: 452/-8 (444) function old new delta fit_find_config_node - 264 +264 board_fit_config_name_match - 188 +188 image_source_script 520 512 -8
And it's that growth, over 722 configs, that I'm concerned with.
Hi Tom,
Thanks for the pointers. I understand your concerns about the growth in size. I'll look into it.
Best regards, Sven
On 1/24/22 19:08, Tom Rini wrote:
On Sat, Jan 22, 2022 at 09:36:58PM +0100, Sven Schwermer wrote:
Hi Tom,
I didn't think this would result in a size increase. Could you elaborate?
Alright. So for background, if you build with tools/buildman/buildman rather than just "make" directly there's some handy tools like "bloat" detection, which shows function size changes from build A to build B. Doing something like this will show the changes for pine64_plus for example export SOURCE_DATE_EPOCH=`date +%s` ./tools/buildman/buildman -o /tmp/pine64_plus -devl -SBC pine64_plus ./tools/buildman/buildman -o /tmp/pine64_plus -devl -SsB pine64_plus
That shows in the end: aarch64: (for 1/1 boards) all +551.0 rodata +107.0 text +444.0 pine64_plus : all +551 rodata +107 text +444 u-boot: add: 2/0, grow: 0/-1 bytes: 452/-8 (444) function old new delta fit_find_config_node - 264 +264 board_fit_config_name_match - 188 +188 image_source_script 520 512 -8
And it's that growth, over 722 configs, that I'm concerned with.
participants (2)
-
Sven Schwermer -
Tom Rini