[PATCH 00/19] Support FITs with load-only configs
FIT is growing a new feature where a load-only configuration can be used to obtain and set up a devicetree prior to loading the OS. This series implements this feature in U-Boot. Some refactoring is necessary to permit this, on top of a previous series. Also the FIT tests have the annoying property that they all run at once, so this series also adjusts these. Further work will integrate this properly into a bootmeth, etc. Simon Glass (19): doc: Flesh out the documentation for bootm boot: Drop unnecessary assignment in bootm_find_images() boot: Add a function comment for fit_parse_spec() boot: Split out FIT-config handling from select_image() boot: Correct comment for fit_image_get_phase() boot: Move FIT_PHASE_PROP to the image-node properties boot: Update fit_image_load() to support load-only FITs boot: Detect the lack of an OS in a load-only FIT test: Update fit test to fix a few Python warnings test: Convert FIT test to use a class test: Convert FIT test into separate pieces boot: Finish the plumbing for the load-only FIT test: Add a test for the simple case of a load-only FIT boot: Update boot_get_ramdisk() to return an error boot: Update boot_get_ramdisk() return arguments boot: Tidy up boot_get_fdt() boot: Update boot_get_fdt() return arguments boot: Support bootm restart test: Add a test for loading a second FIT boot/bootm.c | 53 ++++- boot/image-board.c | 46 ++-- boot/image-fdt.c | 12 +- boot/image-fit.c | 208 ++++++++++------ cmd/bootm.c | 5 +- doc/usage/cmd/bootm.rst | 110 ++++++++- include/bootm.h | 5 +- include/bootstage.h | 1 + include/image.h | 69 +++--- test/py/tests/test_fit.py | 488 +++++++++++++++++++++++++------------- 10 files changed, 670 insertions(+), 327 deletions(-) -- 2.43.0 base-commit: 190d459c5292bfc5e1b7e6d820f88a02d24c9471 branch: loada
This lacks details about the subcommands, so add a description of the boot flow and the subcommands which are available. Signed-off-by: Simon Glass <sjg@chromium.org> --- doc/usage/cmd/bootm.rst | 102 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 100 insertions(+), 2 deletions(-) diff --git a/doc/usage/cmd/bootm.rst b/doc/usage/cmd/bootm.rst index e409ebc193b..1f59895568e 100644 --- a/doc/usage/cmd/bootm.rst +++ b/doc/usage/cmd/bootm.rst @@ -11,8 +11,9 @@ Synopsis :: - bootm [fit_addr]#<conf>[#extra-conf] - bootm [[fit_addr]:<os_subimg>] [[<fit_addr2>]:<rd_subimg2>] [[<fit_addr3>]:<fdt_subimg>] + bootm [start] [<fit_addr>]#<conf>[#<extra-conf>] + bootm [start] [[<fit_addr>]:<os_subimg>] [[<fit_addr2>]:<rd_subimg2>] [[<fit_addr3>]:<fdt_subimg>] + bootm <subcmd> bootm <addr1> [[<addr2> [<addr3>]] # Legacy boot @@ -46,6 +47,103 @@ rd_subimg fdt_subimg FDT sub-image to boot +Bootm steps +~~~~~~~~~~~ + +The bootm command follows a predefined set of states to complete a boot. The +usual case, if a subcommand is omitted, is that bootm runs a complete boot, +working through each state one by one, in sequence. Some states are skipped +depending on the boot type. + +Note that the bootm command automatically adds `findos` and `findother` states +when the `start` state begins. These states are documented here but cannot be +individually selected. + +The states are described below: + +start + Start the boot process afresh, recording the image(s) to be booted. + +preload + Deal with any preload step, sometimes used to do a full signature check of + the FIT, before looking at any of the data within. + + This cannot be selected from the bootm command, as it is implicit in + `start`. + +findos + Find the OS within the FIT, etc. and set up the images.os fields + + This cannot be selected from the bootm command, as it is implicit in + `start`. + +findother + Find other files that may be needed, including any ramdisk, devicetree, + FPGA or loadables. After this, the images.rd... and images.ft fields are + set up. + + For each loadable, the appropriate handler is called (as declared by the + U_BOOT_FIT_LOADABLE_HANDLER() macro). There is no record kept of which + loadables were loaded, other than that used by :doc:`../upl`. + + This step is only active if the image type is kernel, kernel_noload or + multi, **and** the OS is Linux, VxWorks, EFI or TEE. + + This cannot be selected from the bootm command, as it is implicit in + `start`. + +measure + This measures the loaded files, if `CONFIG_MEASURED_BOOT` is enabled. + + This cannot be selected from the bootm command. Currently it is only used + when using bootm without a subcommand. + +loados + Load the OS itself to its final location. This may involve copying or + decompressing it. + +ramdisk + Load the ramdisk to its final location, if necessary. This typically + involves copying it out of the FIT. + +fdt + Load the devicetree to its final location. This typically involves copying + or decompressing it from the FIT. + +cmdline + Set up the command line for the OS, e.g. using the `bootargs` environment + variable, perhaps adding some more pieces from an `extlinux.conf` entry. + +bdt + Set up the board information for the OS (seldom used these days). + +prep + Prepare to boot the OS, e.g. setting up any tables or data structures that + are required. After this the OS itself is ready to boot. + +fake + This is only used for testing and only available when `CONFIG_TRACE` is + enabled. It fakes a boot of the OS, performs all the normal steps right up + to the point where U-Boot is about to jump to the OS. It then runs a list + of commands from the `fakegocmd` environment variable. Note that the + machine may not be stable after this occurs. + + This can be useful for debugging slow booting, for example. See + :doc:`/develop/trace` for more details. + +go + Start the OS, after performing any last-minute tasks. At this point, the OS + should be running and U-Boot's task is completed. + +Subcommands +~~~~~~~~~~~ + +Except as noted above, it is possible to perform the bootm processing piecemeal. +The first command must be `bootm start` after which the others can be used, +normally in the order they are documented above. This can aid debugging but it +can also help to see what happens at each stage and the state of U-Boot and +memory after each stage. + See below for legacy boot. Booting using :doc:`../fit/index` is recommended. Note on current image address -- 2.43.0
Since 'select' is already assigned in the declarations for this function, there is no need to do it again. Drop the duplicate assignment. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/bootm.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/boot/bootm.c b/boot/bootm.c index 0e15cdd0015..02bc81eb9e5 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -531,9 +531,6 @@ int bootm_find_images(ulong img_addr, const char *conf_ramdisk, } } - if (conf_ramdisk) - select = conf_ramdisk; - /* find ramdisk */ ret = boot_get_ramdisk(select, &images, IH_INITRD_ARCH, &images.rd_start, &images.rd_end); -- 2.43.0
Add a little documentation so that the purpose of this function is clear. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/image-fit.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/boot/image-fit.c b/boot/image-fit.c index 7f6b24f6198..da6d38d57bb 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -43,6 +43,18 @@ DECLARE_GLOBAL_DATA_PTR; /* New uImage format routines */ /*****************************************************************************/ #ifndef USE_HOSTCC + +/** + * fit_parse_spec() - Helper for parsing a FIT specifier + * + * @spec: FIT specifier, e.g. "", "1000", "#conf-1", "2000#conf-1", + * "3000:image-1" + * @sepc: Separator character, either '#' or ':' + * @addr_curr: Existing default address to return if none is provided in @spec + * @addr: Returns the address at @spec, or @addr_curr if none + * @name: Returns the name after the separator, or NULL if none + * Return: 1 if separator was found, 0 if not + */ static int fit_parse_spec(const char *spec, char sepc, ulong addr_curr, ulong *addr, const char **name) { -- 2.43.0
This code is complicated enough that it deserves its own function. Split it out. Fix up a missing argument from the select_image() docs while here. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/image-fit.c | 121 ++++++++++++++++++++++++++++++----------------- 1 file changed, 78 insertions(+), 43 deletions(-) diff --git a/boot/image-fit.c b/boot/image-fit.c index da6d38d57bb..ff6201306f4 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -2066,6 +2066,75 @@ static const char *fit_get_image_type_property(int ph_type) return "unknown"; } +/** + * select_from_config() - Select an image from a configuration + * + * @fit: Pointer to FIT + * @images: Boot images structure + * @fit_uname_config: Requested configuration name (e.g. "conf-1") or NULL to + * use the default + * @prop_name: Property name (in the configuration node) indicating the image + * to load + * @ph_type: Required image type (IH_TYPE_...) and phase (IH_PHASE_...) + * @bootstage_id: ID of starting bootstage to use for progress updates + * @fit_unamep: Returns name of selected image node, on success + * @fit_base_uname_configp: Returns config name selected, on success + * Return: node offset of image node, on success, else -ve error code + */ +static int select_from_config(const void *fit, struct bootm_headers *images, + const char *fit_uname_config, + const char *prop_name, int ph_type, + int bootstage_id, const char **fit_unamep, + const char **fit_base_uname_configp) +{ + int cfg_noffset, noffset; + int ret; + + /* + * no image node unit name, try to get config + * node first. If config unit node name is NULL + * fit_conf_get_node() will try to find default config node + */ + bootstage_mark(bootstage_id + BOOTSTAGE_SUB_NO_UNIT_NAME); + 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_configp = fdt_get_name(fit, cfg_noffset, NULL); + printf(" Using '%s' configuration\n", *fit_base_uname_configp); + /* Remember this config */ + if (image_ph_type(ph_type) == IH_TYPE_KERNEL) + images->fit_uname_cfg = *fit_base_uname_configp; + + if (FIT_IMAGE_ENABLE_VERIFY && images->verify) { + puts(" Verifying Hash Integrity ... "); + if (fit_config_verify(fit, cfg_noffset)) { + puts("Bad Data Hash\n"); + bootstage_error(bootstage_id + + BOOTSTAGE_SUB_HASH); + return -EACCES; + } + puts("OK\n"); + } + + bootstage_mark(BOOTSTAGE_ID_FIT_CONFIG); + + noffset = fit_conf_get_prop_node(fit, cfg_noffset, prop_name, + image_ph_phase(ph_type)); + *fit_unamep = fit_get_name(fit, noffset, NULL); + + return noffset; +} + /** * select_image() - Select the image to load * @@ -2078,6 +2147,8 @@ static const char *fit_get_image_type_property(int ph_type) * the selected image name. Note that fit_unamep cannot be NULL * @fit_uname_config: Requested configuration name (e.g. "conf-1") or NULL to * use the default + * @prop_name: Property name (in the configuration node) indicating the image + * to load * @ph_type: Required image type (IH_TYPE_...) and phase (IH_PHASE_...) * @bootstage_id: ID of starting bootstage to use for progress updates * @fit_base_uname_configp: Returns config name selected, or NULL if *fit_unamep @@ -2089,8 +2160,8 @@ static int select_image(const void *fit, struct bootm_headers *images, const char *prop_name, int ph_type, int bootstage_id, const char **fit_base_uname_configp) { - int cfg_noffset, noffset; const char *fit_uname; + int noffset; int ret; fit_uname = fit_unamep ? *fit_unamep : NULL; @@ -2112,48 +2183,10 @@ static int select_image(const void *fit, struct bootm_headers *images, bootstage_mark(bootstage_id + BOOTSTAGE_SUB_UNIT_NAME); noffset = fit_image_get_node(fit, fit_uname); } else { - /* - * no image node unit name, try to get config - * node first. If config unit node name is NULL - * fit_conf_get_node() will try to find default config node - */ - bootstage_mark(bootstage_id + BOOTSTAGE_SUB_NO_UNIT_NAME); - 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_configp = fdt_get_name(fit, cfg_noffset, NULL); - printf(" Using '%s' configuration\n", - *fit_base_uname_configp); - /* Remember this config */ - if (image_ph_type(ph_type) == IH_TYPE_KERNEL) - images->fit_uname_cfg = *fit_base_uname_configp; - - if (FIT_IMAGE_ENABLE_VERIFY && images->verify) { - puts(" Verifying Hash Integrity ... "); - if (fit_config_verify(fit, cfg_noffset)) { - puts("Bad Data Hash\n"); - bootstage_error(bootstage_id + - BOOTSTAGE_SUB_HASH); - return -EACCES; - } - puts("OK\n"); - } - - bootstage_mark(BOOTSTAGE_ID_FIT_CONFIG); - - noffset = fit_conf_get_prop_node(fit, cfg_noffset, prop_name, - image_ph_phase(ph_type)); - *fit_unamep = fit_get_name(fit, noffset, NULL); + noffset = select_from_config(fit, images, fit_uname_config, + prop_name, ph_type, bootstage_id, + &fit_uname, + fit_base_uname_configp); } if (noffset < 0) { @@ -2169,6 +2202,8 @@ static int select_image(const void *fit, struct bootm_headers *images, return ret; } + *fit_unamep = fit_uname; + return noffset; } -- 2.43.0
This function has a comment in both C and header files. Drop the one in the C file and tweak the header-file one for clarity. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/image-fit.c | 12 ------------ include/image.h | 4 ++-- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/boot/image-fit.c b/boot/image-fit.c index ff6201306f4..cc6aa6afd36 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -810,18 +810,6 @@ int fit_image_get_comp(const void *fit, int noffset, uint8_t *comp) return 0; } -/** - * fit_image_get_phase() - get the phase for a configuration node - * @fit: pointer to the FIT format image header - * @offset: configuration-node offset - * @phasep: returns the phase - * - * Finds the phase property in a given configuration node. If the property is - * found, its (string) value is translated to the numeric id which is returned - * to the caller. - * - * Returns: 0 on success, -ENOENT if missing, -EINVAL for invalid value - */ int fit_image_get_phase(const void *fit, int offset, enum image_phase_t *phasep) { const void *data; diff --git a/include/image.h b/include/image.h index 0402cf92219..d7e081a0a5b 100644 --- a/include/image.h +++ b/include/image.h @@ -1259,10 +1259,10 @@ int fit_image_get_data(const void *fit, int noffset, const void **data, size_t *size); /** - * fit_image_get_phase() - Get the phase from a FIT image + * fit_image_get_phase() - Get the phase from an image in a FIT * * @fit: FIT to read from - * @offset: offset node to read + * @offset: offset of the image-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 -- 2.43.0
This property is used in image nodes, not configuration nodes, so move it to the right place. Signed-off-by: Simon Glass <sjg@chromium.org> --- include/image.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/image.h b/include/image.h index d7e081a0a5b..1b8764d9906 100644 --- a/include/image.h +++ b/include/image.h @@ -1176,6 +1176,7 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size, #define FIT_COMP_PROP "compression" #define FIT_ENTRY_PROP "entry" #define FIT_LOAD_PROP "load" +#define FIT_PHASE_PROP "phase" /* configuration node */ #define FIT_KERNEL_PROP "kernel" @@ -1188,7 +1189,6 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size, #define FIT_FIRMWARE_PROP "firmware" #define FIT_STANDALONE_PROP "standalone" #define FIT_SCRIPT_PROP "script" -#define FIT_PHASE_PROP "phase" #define FIT_MAX_HASH_LEN HASH_MAX_DIGEST_SIZE -- 2.43.0
The load-only property in a configuration node indicates that there is not necessarily a kernel or firmware image in the configuration. Handle this case and return an error code. Fix the 'NOEXEC' typo while here. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/image-fit.c | 63 ++++++++++++++++++++++++++++++++---------------- include/image.h | 9 ++++--- 2 files changed, 48 insertions(+), 24 deletions(-) diff --git a/boot/image-fit.c b/boot/image-fit.c index cc6aa6afd36..224205b48f2 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -2118,6 +2118,18 @@ static int select_from_config(const void *fit, struct bootm_headers *images, noffset = fit_conf_get_prop_node(fit, cfg_noffset, prop_name, image_ph_phase(ph_type)); + if (noffset < 0) { + /* + * see if this is a load-only configuration, in which case we + * allow the image to be missing. Note that the configuration + * was verified above, so that other images can be loaded. + * + * Return a special error message to indicate this. + */ + if (fdt_getprop(fit, cfg_noffset, FIT_LOAD_ONLY_PROP, NULL)) + return -ENOPKG; + } + *fit_unamep = fit_get_name(fit, noffset, NULL); return noffset; @@ -2178,6 +2190,11 @@ static int select_image(const void *fit, struct bootm_headers *images, } if (noffset < 0) { + if (noffset == -ENOPKG) { + printf(" Detected load-only image: skipping '%s'\n", + prop_name); + return -ENOPKG; + } printf("Could not find subimage node type '%s'\n", prop_name); bootstage_error(bootstage_id + BOOTSTAGE_SUB_SUBNODE); return -ENOENT; @@ -2494,32 +2511,36 @@ int fit_image_load(struct bootm_headers *images, ulong addr, noffset = select_image(fit, images, &fit_uname, fit_uname_config, prop_name, ph_type, bootstage_id, &fit_base_uname_config); - if (noffset < 0) - return noffset; + if (noffset >= 0) { - ret = check_allowed(fit, noffset, images, image_type, arch, - bootstage_id); - if (ret) - return ret; + ret = check_allowed(fit, noffset, images, image_type, arch, + bootstage_id); + if (ret) + return ret; - ret = obtain_data(fit, noffset, prop_name, bootstage_id, &buf, &len); - if (ret) - return ret; + ret = obtain_data(fit, noffset, prop_name, bootstage_id, &buf, &len); + if (ret) + return ret; - ret = handle_load_op(fit, noffset, prop_name, buf, len, image_type, - load_op, bootstage_id, &load); - if (ret) - return ret; + ret = handle_load_op(fit, noffset, prop_name, buf, len, image_type, + load_op, bootstage_id, &load); + if (ret) + return ret; - upl_add_image(fit, noffset, load, len); + upl_add_image(fit, noffset, load, len); - *datap = load; - *lenp = len; - if (fit_unamep) - *fit_unamep = (char *)fit_uname; - if (fit_uname_configp) - *fit_uname_configp = (char *)(fit_uname_config ? : - fit_base_uname_config); + *datap = load; + *lenp = len; + } + + /* note that fit_uname will always be NULL if noffset == -ENOPKG */ + if (noffset >= 0 || noffset == -ENOPKG) { + if (fit_unamep) + *fit_unamep = (char *)fit_uname; + if (fit_uname_configp) + *fit_uname_configp = (char *)(fit_uname_config ? : + fit_base_uname_config); + } return noffset; } diff --git a/include/image.h b/include/image.h index 1b8764d9906..254d1731b96 100644 --- a/include/image.h +++ b/include/image.h @@ -812,10 +812,11 @@ int boot_get_fdt_fit(struct bootm_headers *images, ulong addr, * @param addr Address of FIT in memory * @param fit_unamep On entry this is the requested image name * (e.g. "kernel") or NULL to use the default. On exit - * points to the selected image name + * points to the selected image name on success * @param fit_uname_configp On entry this is the requested configuration * name (e.g. "conf-1") or NULL to use the default. On - * exit points to the selected configuration name. + * exit points to the selected configuration name, on + * success or if -ENOPKG is returned * @param arch Expected architecture (IH_ARCH_...) * @param image_ph_type Required image type (IH_TYPE_...). If this is * IH_TYPE_KERNEL then we allow IH_TYPE_KERNEL_NOLOAD @@ -833,7 +834,8 @@ int boot_get_fdt_fit(struct bootm_headers *images, ulong addr, * -EACCES - hash, signature or decryptions failure * -EBADF - invalid OS or image type, or cannot get image load-address * -EXDEV - memory overwritten / overlap - * -NOEXEC - image decompression error, or invalid FDT + * -ENOEXEC - image decompression error, or invalid FDT + * -ENOPKG - image is missing, but this is a load-only image */ int fit_image_load(struct bootm_headers *images, ulong addr, const char **fit_unamep, const char **fit_uname_configp, @@ -1189,6 +1191,7 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size, #define FIT_FIRMWARE_PROP "firmware" #define FIT_STANDALONE_PROP "standalone" #define FIT_SCRIPT_PROP "script" +#define FIT_LOAD_ONLY_PROP "load-only" #define FIT_MAX_HASH_LEN HASH_MAX_DIGEST_SIZE -- 2.43.0
Detect this condition and set a flag in the images struct to remember that no OS is being loaded. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/bootm.c | 14 +++++++++++++- include/image.h | 3 +++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/boot/bootm.c b/boot/bootm.c index 02bc81eb9e5..5caeb94a998 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -193,13 +193,20 @@ static int boot_get_kernel(const char *addr_fit, struct bootm_headers *images, IH_ARCH_DEFAULT, IH_TYPE_KERNEL, BOOTSTAGE_ID_FIT_KERNEL_START, FIT_LOAD_IGNORED, os_data, os_len); - if (os_noffset < 0) + if (os_noffset < 0 && os_noffset != -ENOPKG) return -ENOENT; images->fit_hdr_os = map_sysmem(img_addr, 0); images->fit_uname_os = fit_uname_kernel; images->fit_uname_cfg = fit_uname_config; images->fit_noffset_os = os_noffset; + if (os_noffset == -ENOPKG) { + log_debug("no_os: hdr_os %lx uname_os '%s' uname_cfg '%s' noffset_os %dE\n", + img_addr, images->fit_uname_os, + images->fit_uname_cfg, + images->fit_noffset_os); + return -ENOPKG; + } break; #endif #ifdef CONFIG_ANDROID_BOOT_IMAGE @@ -335,6 +342,11 @@ static int bootm_find_os(const char *cmd_name, const char *addr_fit) ret = boot_get_kernel(addr_fit, &images, &images.os.image_start, &images.os.image_len, &os_hdr); if (ret) { + /* no OS present, but that is OK */ + if (ret == -ENOPKG) { + images.no_os = true; + return 0; + } if (ret == -EPROTOTYPE) printf("Wrong Image Type for %s command\n", cmd_name); diff --git a/include/image.h b/include/image.h index 254d1731b96..0c5575adbbf 100644 --- a/include/image.h +++ b/include/image.h @@ -455,6 +455,9 @@ struct bootm_headers { int verify; /* env_get("verify")[0] != 'n' */ enum bootm_state state; + + /* true if this is a load-only FIT with no OS */ + bool no_os; }; extern struct bootm_headers images; -- 2.43.0
Fix some warnings and disable ones that cannot be fixed. Signed-off-by: Simon Glass <sjg@chromium.org> --- test/py/tests/test_fit.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py index 756b6809600..759ade39bde 100755 --- a/test/py/tests/test_fit.py +++ b/test/py/tests/test_fit.py @@ -1,13 +1,14 @@ # SPDX-License-Identifier: GPL-2.0+ # Copyright (c) 2013, Google Inc. -# -# Sanity check of the FIT handling in U-Boot + +"""Sanity check of the FIT handling in U-Boot""" import os -import pytest import struct -import utils + +import pytest import fit_util +import utils # Define a base ITS which we can adjust using % and a dictionary base_its = ''' @@ -160,8 +161,8 @@ def test_fit_base(ubman): fname = make_fname(filename) data = '' for i in range(100): - data += '%s %d was seldom used in the middle ages\n' % (text, i) - with open(fname, 'w') as fd: + data += f'{text} {i} was seldom used in the middle ages\n' + with open(fname, 'w', encoding='ascii') as fd: print(data, file=fd) return fname @@ -202,6 +203,7 @@ def test_fit_base(ubman): 'third_line:') '30' """ + # pylint: disable=W0612 __tracebackhide__ = True for line in '\n'.join(text).splitlines(): pos = line.find(match) @@ -209,6 +211,7 @@ def test_fit_base(ubman): return line[:pos] + line[pos + len(match):] pytest.fail("Expected '%s' but not found in output") + return '<no-match>' def check_equal(expected_fname, actual_fname, failure_msg): """Check that a file matches its expected contents -- 2.43.0
Move this test over to use a class instead of a function, so we can easily split it into some related tests. Move the TODO so it is part of the comment for the class. Tidy up some function comments while we are here. Signed-off-by: Simon Glass <sjg@chromium.org> --- test/py/tests/test_fit.py | 230 +++++++++++++++++++++----------------- 1 file changed, 127 insertions(+), 103 deletions(-) diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py index 759ade39bde..79298a3bb4e 100755 --- a/test/py/tests/test_fit.py +++ b/test/py/tests/test_fit.py @@ -11,7 +11,7 @@ import fit_util import utils # Define a base ITS which we can adjust using % and a dictionary -base_its = ''' +BASE_ITS = ''' /dts-v1/; / { @@ -81,7 +81,7 @@ base_its = ''' ''' # Define a base FDT - currently we don't use anything in this -base_fdt = ''' +BASE_FDT = ''' /dts-v1/; / { @@ -104,7 +104,7 @@ base_fdt = ''' # This is the U-Boot script that is run for each test. First load the FIT, # then run the 'bootm' command, then save out memory from the places where # we expect 'bootm' to write things. Then quit. -base_script = ''' +BASE_SCRIPT = ''' mw 0 0 180000 host load hostfs 0 %(fit_addr)x %(fit)s fdt addr %(fit_addr)x @@ -120,45 +120,68 @@ host save hostfs 0 %(loadables2_addr)x %(loadables2_out)s %(loadables2_size)x @pytest.mark.boardspec('sandbox') @pytest.mark.buildconfigspec('fit_signature') @pytest.mark.requiredtool('dtc') -def test_fit_base(ubman): - def make_fname(leaf): +class TestFitImage: + """Test class for FIT image handling in U-Boot + + TODO: Almost everything: + - hash algorithms - invalid hash/contents should be detected + - signature algorithms - invalid sig/contents should be detected + - compression + - checking that errors are detected like: + - image overwriting + - missing images + - invalid configurations + - incorrect os/arch/type fields + - empty data + - images too large/small + - invalid FDT (e.g. putting a random binary in instead) + - default configuration selection + - bootm command line parameters should have desired effect + - run code coverage to make sure we are testing all the code + """ + + def make_fname(self, ubman, leaf): """Make a temporary filename Args: - leaf: Leaf name of file to create (within temporary directory) + ubman (ConsoleBase): U-Boot fixture + leaf (str): Leaf name of file to create (within temporary directory) + Return: - Temporary filename + str: Temporary filename """ return os.path.join(ubman.config.build_dir, leaf) - def filesize(fname): + def filesize(self, fname): """Get the size of a file Args: - fname: Filename to check + fname (str): Filename to check + Return: - Size of file in bytes + int: Size of file in bytes """ return os.stat(fname).st_size - def read_file(fname): + def read_file(self, fname): """Read the contents of a file Args: - fname: Filename to read - Returns: - Contents of file as a string + fname (str): Filename to read + + Return: + str: Contents of file """ with open(fname, 'rb') as fd: return fd.read() - def make_ramdisk(filename, text): + def make_ramdisk(self, ubman, filename, text): """Make a sample ramdisk with test data Returns: - Filename of ramdisk created + str: Filename of ramdisk created """ - fname = make_fname(filename) + fname = self.make_fname(ubman, filename) data = '' for i in range(100): data += f'{text} {i} was seldom used in the middle ages\n' @@ -166,11 +189,12 @@ def test_fit_base(ubman): print(data, file=fd) return fname - def make_compressed(filename): + def make_compressed(self, ubman, filename): + """Compress a file using gzip""" utils.run_and_log(ubman, ['gzip', '-f', '-k', filename]) return filename + '.gz' - def find_matching(text, match): + def find_matching(self, text, match): """Find a match in a line of text, and return the unmatched line portion This is used to extract a part of a line from some text. The match string @@ -184,10 +208,12 @@ def test_fit_base(ubman): to use regex and return groups. Args: - text: Text to check (list of strings, one for each command issued) - match: String to search for + text (list of str): Text to check, one for each command issued + match (str): String to search for + Return: - String containing unmatched portion of line + str: unmatched portion of line + Exceptions: ValueError: If match is not found @@ -213,7 +239,7 @@ def test_fit_base(ubman): pytest.fail("Expected '%s' but not found in output") return '<no-match>' - def check_equal(expected_fname, actual_fname, failure_msg): + def check_equal(self, expected_fname, actual_fname, failure_msg): """Check that a file matches its expected contents This is always used on out-buffers whose size is decided by the test @@ -222,59 +248,50 @@ def test_fit_base(ubman): expected data. Args: - expected_fname: Filename containing expected contents - actual_fname: Filename containing actual contents - failure_msg: Message to print on failure + expected_fname (str): Filename containing expected contents + actual_fname (str): Filename containing actual contents + failure_msg (str): Message to print on failure """ - expected_data = read_file(expected_fname) - actual_data = read_file(actual_fname) + expected_data = self.read_file(expected_fname) + actual_data = self.read_file(actual_fname) if len(expected_data) < len(actual_data): actual_data = actual_data[:len(expected_data)] assert expected_data == actual_data, failure_msg - def check_not_equal(expected_fname, actual_fname, failure_msg): + def check_not_equal(self, expected_fname, actual_fname, failure_msg): """Check that a file does not match its expected contents Args: - expected_fname: Filename containing expected contents - actual_fname: Filename containing actual contents - failure_msg: Message to print on failure + expected_fname (str): Filename containing expected contents + actual_fname (str): Filename containing actual contents + failure_msg (str): Message to print on failure """ - expected_data = read_file(expected_fname) - actual_data = read_file(actual_fname) + expected_data = self.read_file(expected_fname) + actual_data = self.read_file(actual_fname) assert expected_data != actual_data, failure_msg - def run_fit_test(mkimage): + def test_fit_operations(self, ubman): """Basic sanity check of FIT loading in U-Boot - TODO: Almost everything: - - hash algorithms - invalid hash/contents should be detected - - signature algorithms - invalid sig/contents should be detected - - compression - - checking that errors are detected like: - - image overwriting - - missing images - - invalid configurations - - incorrect os/arch/type fields - - empty data - - images too large/small - - invalid FDT (e.g. putting a random binary in instead) - - default configuration selection - - bootm command line parameters should have desired effect - - run code coverage to make sure we are testing all the code + This test covers various FIT configurations and verifies correct behavior. + + Args: + ubman (ConsoleBase): U-Boot fixture """ + mkimage = os.path.join(ubman.config.build_dir, 'tools/mkimage') + # Set up invariant files - fdt_data = fit_util.make_dtb(ubman, base_fdt, 'u-boot') + fdt_data = fit_util.make_dtb(ubman, BASE_FDT, 'u-boot') kernel = fit_util.make_kernel(ubman, 'test-kernel.bin', 'kernel') - ramdisk = make_ramdisk('test-ramdisk.bin', 'ramdisk') + ramdisk = self.make_ramdisk(ubman, 'test-ramdisk.bin', 'ramdisk') loadables1 = fit_util.make_kernel(ubman, 'test-loadables1.bin', 'lenrek') - loadables2 = make_ramdisk('test-loadables2.bin', 'ksidmar') - kernel_out = make_fname('kernel-out.bin') - fdt = make_fname('u-boot.dtb') - fdt_out = make_fname('fdt-out.dtb') - ramdisk_out = make_fname('ramdisk-out.bin') - loadables1_out = make_fname('loadables1-out.bin') - loadables2_out = make_fname('loadables2-out.bin') + loadables2 = self.make_ramdisk(ubman, 'test-loadables2.bin', 'ksidmar') + kernel_out = self.make_fname(ubman, 'kernel-out.bin') + fdt = self.make_fname(ubman, 'u-boot.dtb') + fdt_out = self.make_fname(ubman, 'fdt-out.dtb') + ramdisk_out = self.make_fname(ubman, 'ramdisk-out.bin') + loadables1_out = self.make_fname(ubman, 'loadables1-out.bin') + loadables2_out = self.make_fname(ubman, 'loadables2-out.bin') # Set up basic parameters with default values params = { @@ -283,32 +300,32 @@ def test_fit_base(ubman): 'kernel' : kernel, 'kernel_out' : kernel_out, 'kernel_addr' : 0x40000, - 'kernel_size' : filesize(kernel), + 'kernel_size' : self.filesize(kernel), 'kernel_config' : 'kernel = "kernel-1";', 'fdt' : fdt, 'fdt_out' : fdt_out, 'fdt_addr' : 0x80000, - 'fdt_size' : filesize(fdt_data), + 'fdt_size' : self.filesize(fdt_data), 'fdt_load' : '', 'ramdisk' : ramdisk, 'ramdisk_out' : ramdisk_out, 'ramdisk_addr' : 0xc0000, - 'ramdisk_size' : filesize(ramdisk), + 'ramdisk_size' : self.filesize(ramdisk), 'ramdisk_load' : '', 'ramdisk_config' : '', 'loadables1' : loadables1, 'loadables1_out' : loadables1_out, 'loadables1_addr' : 0x100000, - 'loadables1_size' : filesize(loadables1), + 'loadables1_size' : self.filesize(loadables1), 'loadables1_load' : '', 'loadables2' : loadables2, 'loadables2_out' : loadables2_out, 'loadables2_addr' : 0x140000, - 'loadables2_size' : filesize(loadables2), + 'loadables2_size' : self.filesize(loadables2), 'loadables2_load' : '', 'loadables_config' : '', @@ -316,35 +333,35 @@ def test_fit_base(ubman): } # Make a basic FIT and a script to load it - fit = fit_util.make_fit(ubman, mkimage, base_its, params) + fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) params['fit'] = fit - cmd = base_script % params + cmd = BASE_SCRIPT % params # First check that we can load a kernel # We could perhaps reduce duplication with some loss of readability with ubman.log.section('Kernel load'): output = ubman.run_command_list(cmd.splitlines()) - check_equal(kernel, kernel_out, 'Kernel not loaded') - check_not_equal(fdt_data, fdt_out, - 'FDT loaded but should be ignored') - check_not_equal(ramdisk, ramdisk_out, - 'Ramdisk loaded but should not be') + self.check_equal(kernel, kernel_out, 'Kernel not loaded') + self.check_not_equal(fdt_data, fdt_out, + 'FDT loaded but should be ignored') + self.check_not_equal(ramdisk, ramdisk_out, + 'Ramdisk loaded but should not be') # Find out the offset in the FIT where U-Boot has found the FDT - line = find_matching(output, 'Booting using the fdt blob at ') + line = self.find_matching(output, 'Booting using the fdt blob at ') fit_offset = int(line, 16) - params['fit_addr'] fdt_magic = struct.pack('>L', 0xd00dfeed) - data = read_file(fit) + data = self.read_file(fit) # Now find where it actually is in the FIT (skip the first word) real_fit_offset = data.find(fdt_magic, 4) assert fit_offset == real_fit_offset, ( - 'U-Boot loaded FDT from offset %#x, FDT is actually at %#x' % - (fit_offset, real_fit_offset)) + 'U-Boot loaded FDT from offset %#x, FDT is actually at %#x' % + (fit_offset, real_fit_offset)) - # Check if bootargs strings substitution works + # Check bootargs string substitution output = ubman.run_command_list([ - 'env set bootargs \\\"\'my_boot_var=${foo}\'\\\"', + 'env set bootargs \\"\'my_boot_var=${foo}\'\\"', 'env set foo bar', 'bootm prep', 'env print bootargs']) @@ -353,56 +370,63 @@ def test_fit_base(ubman): # Now a kernel and an FDT with ubman.log.section('Kernel + FDT load'): params['fdt_load'] = 'load = <%#x>;' % params['fdt_addr'] - fit = fit_util.make_fit(ubman, mkimage, base_its, params) + fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) + params['fit'] = fit + cmd = BASE_SCRIPT % params output = ubman.run_command_list(cmd.splitlines()) - check_equal(kernel, kernel_out, 'Kernel not loaded') - check_equal(fdt_data, fdt_out, 'FDT not loaded') - check_not_equal(ramdisk, ramdisk_out, - 'Ramdisk loaded but should not be') + self.check_equal(kernel, kernel_out, 'Kernel not loaded') + self.check_equal(fdt_data, fdt_out, 'FDT not loaded') + self.check_not_equal(ramdisk, ramdisk_out, + 'Ramdisk loaded but should not be') # Try a ramdisk with ubman.log.section('Kernel + FDT + Ramdisk load'): params['ramdisk_config'] = 'ramdisk = "ramdisk-1";' params['ramdisk_load'] = 'load = <%#x>;' % params['ramdisk_addr'] - fit = fit_util.make_fit(ubman, mkimage, base_its, params) + fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) + params['fit'] = fit + cmd = BASE_SCRIPT % params output = ubman.run_command_list(cmd.splitlines()) - check_equal(ramdisk, ramdisk_out, 'Ramdisk not loaded') + self.check_equal(ramdisk, ramdisk_out, 'Ramdisk not loaded') # Configuration with some Loadables with ubman.log.section('Kernel + FDT + Ramdisk load + Loadables'): params['loadables_config'] = 'loadables = "kernel-2", "ramdisk-2";' params['loadables1_load'] = ('load = <%#x>;' % - params['loadables1_addr']) + params['loadables1_addr']) params['loadables2_load'] = ('load = <%#x>;' % - params['loadables2_addr']) - fit = fit_util.make_fit(ubman, mkimage, base_its, params) + params['loadables2_addr']) + fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) + params['fit'] = fit + cmd = BASE_SCRIPT % params output = ubman.run_command_list(cmd.splitlines()) - check_equal(loadables1, loadables1_out, - 'Loadables1 (kernel) not loaded') - check_equal(loadables2, loadables2_out, - 'Loadables2 (ramdisk) not loaded') + self.check_equal(loadables1, loadables1_out, + 'Loadables1 (kernel) not loaded') + self.check_equal(loadables2, loadables2_out, + 'Loadables2 (ramdisk) not loaded') # Kernel, FDT and Ramdisk all compressed with ubman.log.section('(Kernel + FDT + Ramdisk) compressed'): params['compression'] = 'gzip' - params['kernel'] = make_compressed(kernel) - params['fdt'] = make_compressed(fdt) - params['ramdisk'] = make_compressed(ramdisk) - fit = fit_util.make_fit(ubman, mkimage, base_its, params) + params['kernel'] = self.make_compressed(ubman, kernel) + params['fdt'] = self.make_compressed(ubman, fdt) + params['ramdisk'] = self.make_compressed(ubman, ramdisk) + fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) + params['fit'] = fit + cmd = BASE_SCRIPT % params output = ubman.run_command_list(cmd.splitlines()) - check_equal(kernel, kernel_out, 'Kernel not loaded') - check_equal(fdt_data, fdt_out, 'FDT not loaded') - check_not_equal(ramdisk, ramdisk_out, 'Ramdisk got decompressed?') - check_equal(ramdisk + '.gz', ramdisk_out, 'Ramdisk not loaded') + self.check_equal(kernel, kernel_out, 'Kernel not loaded') + self.check_equal(fdt_data, fdt_out, 'FDT not loaded') + self.check_not_equal(ramdisk, ramdisk_out, 'Ramdisk got decompressed?') + self.check_equal(ramdisk + '.gz', ramdisk_out, 'Ramdisk not loaded') # Try without a kernel with ubman.log.section('No kernel + FDT'): params['kernel_config'] = '' params['ramdisk_config'] = '' params['ramdisk_load'] = '' - fit = fit_util.make_fit(ubman, mkimage, base_its, params) + fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) + params['fit'] = fit + cmd = BASE_SCRIPT % params output = ubman.run_command_list(cmd.splitlines()) assert "can't get kernel image!" in '\n'.join(output) - - mkimage = ubman.config.build_dir + '/tools/mkimage' - run_fit_test(mkimage) -- 2.43.0
Use a separate function for each test, so that they appear separately in the results. Move the comment about what tests are covered to the top. Create an 'fsetup' fixture which provides what each test needs. Adjust the helpers to fit better with this new scheme. This should make the test more straightforward and easier to extend in future. Signed-off-by: Simon Glass <sjg@chromium.org> --- test/py/tests/test_fit.py | 275 +++++++++++++++++++++++--------------- 1 file changed, 167 insertions(+), 108 deletions(-) diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py index 79298a3bb4e..7f84eb1a114 100755 --- a/test/py/tests/test_fit.py +++ b/test/py/tests/test_fit.py @@ -239,7 +239,7 @@ class TestFitImage: pytest.fail("Expected '%s' but not found in output") return '<no-match>' - def check_equal(self, expected_fname, actual_fname, failure_msg): + def check_equal(self, params, expected_fname, actual_fname, failure_msg): """Check that a file matches its expected contents This is always used on out-buffers whose size is decided by the test @@ -248,35 +248,40 @@ class TestFitImage: expected data. Args: - expected_fname (str): Filename containing expected contents - actual_fname (str): Filename containing actual contents + params (dict): Parameters dict from which to get the filenames + expected_fname (str): Key for filename containing expected contents + actual_fname (str): Key for filename containing actual contents failure_msg (str): Message to print on failure """ - expected_data = self.read_file(expected_fname) - actual_data = self.read_file(actual_fname) + expected_data = self.read_file(params[expected_fname]) + actual_data = self.read_file(params[actual_fname]) if len(expected_data) < len(actual_data): actual_data = actual_data[:len(expected_data)] assert expected_data == actual_data, failure_msg - def check_not_equal(self, expected_fname, actual_fname, failure_msg): + def check_not_equal(self, params, expected_fname, actual_fname, + failure_msg): """Check that a file does not match its expected contents Args: - expected_fname (str): Filename containing expected contents - actual_fname (str): Filename containing actual contents + params (dict): Parameters dict from which to get the filenames + expected_fname (str): Key for filename containing expected contents + actual_fname (str): Key for filename containing actual contents failure_msg (str): Message to print on failure """ - expected_data = self.read_file(expected_fname) - actual_data = self.read_file(actual_fname) + expected_data = self.read_file(params[expected_fname]) + actual_data = self.read_file(params[actual_fname]) assert expected_data != actual_data, failure_msg - def test_fit_operations(self, ubman): - """Basic sanity check of FIT loading in U-Boot - - This test covers various FIT configurations and verifies correct behavior. + @pytest.fixture() + def fsetup(self, ubman): + """A fixture to set up files and parameters for FIT tests Args: ubman (ConsoleBase): U-Boot fixture + + Yields: + dict: Parameters needed by the test """ mkimage = os.path.join(ubman.config.build_dir, 'tools/mkimage') @@ -332,101 +337,155 @@ class TestFitImage: 'compression' : 'none', } + # Yield all necessary components to the tests + params.update({ + 'mkimage': mkimage, + 'fdt_data': fdt_data, + }) + yield params + + def prepare(self, ubman, fsetup, **kwargs): + """Build a FIT with given params + + Uses the base parameters to create a FIT and a script to test it, + + For the use of eval here to evaluate f-strings, see: + https://stackoverflow.com/questions/42497625/how-to-postpone-defer-the-evalu... + + Args: + ubman (ConsoleBase): U-Boot fixture + fsetup (dict): Information about the test setup + kwargs (dict): Changes to make to the default params + key (str): parameter to change + val (str): f-string to evaluate to get the parameter value + + Return: + tuple: + list of str: Commands to run for the test + dict: Parameters used by the test + str: Filename of the FIT that was created + """ + params = fsetup.copy() + for name, template in kwargs.items(): + # pylint: disable=W0123 + params[name] = eval(f'f"""{template}"""') + # Make a basic FIT and a script to load it - fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) + fit = fit_util.make_fit(ubman, params['mkimage'], BASE_ITS, params) params['fit'] = fit - cmd = BASE_SCRIPT % params - - # First check that we can load a kernel - # We could perhaps reduce duplication with some loss of readability - with ubman.log.section('Kernel load'): - output = ubman.run_command_list(cmd.splitlines()) - self.check_equal(kernel, kernel_out, 'Kernel not loaded') - self.check_not_equal(fdt_data, fdt_out, - 'FDT loaded but should be ignored') - self.check_not_equal(ramdisk, ramdisk_out, - 'Ramdisk loaded but should not be') - - # Find out the offset in the FIT where U-Boot has found the FDT - line = self.find_matching(output, 'Booting using the fdt blob at ') - fit_offset = int(line, 16) - params['fit_addr'] - fdt_magic = struct.pack('>L', 0xd00dfeed) - data = self.read_file(fit) - - # Now find where it actually is in the FIT (skip the first word) - real_fit_offset = data.find(fdt_magic, 4) - assert fit_offset == real_fit_offset, ( - 'U-Boot loaded FDT from offset %#x, FDT is actually at %#x' % - (fit_offset, real_fit_offset)) + cmds = BASE_SCRIPT % params + + return cmds.splitlines(), params, fit + + def test_fit_kernel_load(self, ubman, fsetup): + """Test loading a FIT image with only a kernel""" + cmds, params, fit = self.prepare(ubman, fsetup) + + # Run and verify + output = ubman.run_command_list(cmds) + self.check_equal(params, 'kernel', 'kernel_out', 'Kernel not loaded') + self.check_not_equal(params, 'fdt_data', 'fdt_out', + 'FDT loaded but should be ignored') + self.check_not_equal(params, 'ramdisk','ramdisk_out', + 'Ramdisk loaded but should not be') + + # Find out the offset in the FIT where U-Boot has found the FDT + line = self.find_matching(output, 'Booting using the fdt blob at ') + fit_offset = int(line, 16) - params['fit_addr'] + fdt_magic = struct.pack('>L', 0xd00dfeed) + data = self.read_file(fit) + + # Now find where it actually is in the FIT (skip the first word) + real_fit_offset = data.find(fdt_magic, 4) + assert fit_offset == real_fit_offset, ( + f'U-Boot loaded FDT from offset {fit_offset:#x}, ' + 'FDT is actually at {real_fit_offset:#x}') # Check bootargs string substitution - output = ubman.run_command_list([ - 'env set bootargs \\"\'my_boot_var=${foo}\'\\"', - 'env set foo bar', - 'bootm prep', - 'env print bootargs']) - assert 'bootargs="my_boot_var=bar"' in output, "Bootargs strings not substituted" - - # Now a kernel and an FDT - with ubman.log.section('Kernel + FDT load'): - params['fdt_load'] = 'load = <%#x>;' % params['fdt_addr'] - fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) - params['fit'] = fit - cmd = BASE_SCRIPT % params - output = ubman.run_command_list(cmd.splitlines()) - self.check_equal(kernel, kernel_out, 'Kernel not loaded') - self.check_equal(fdt_data, fdt_out, 'FDT not loaded') - self.check_not_equal(ramdisk, ramdisk_out, - 'Ramdisk loaded but should not be') - - # Try a ramdisk - with ubman.log.section('Kernel + FDT + Ramdisk load'): - params['ramdisk_config'] = 'ramdisk = "ramdisk-1";' - params['ramdisk_load'] = 'load = <%#x>;' % params['ramdisk_addr'] - fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) - params['fit'] = fit - cmd = BASE_SCRIPT % params - output = ubman.run_command_list(cmd.splitlines()) - self.check_equal(ramdisk, ramdisk_out, 'Ramdisk not loaded') - - # Configuration with some Loadables - with ubman.log.section('Kernel + FDT + Ramdisk load + Loadables'): - params['loadables_config'] = 'loadables = "kernel-2", "ramdisk-2";' - params['loadables1_load'] = ('load = <%#x>;' % - params['loadables1_addr']) - params['loadables2_load'] = ('load = <%#x>;' % - params['loadables2_addr']) - fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) - params['fit'] = fit - cmd = BASE_SCRIPT % params - output = ubman.run_command_list(cmd.splitlines()) - self.check_equal(loadables1, loadables1_out, - 'Loadables1 (kernel) not loaded') - self.check_equal(loadables2, loadables2_out, - 'Loadables2 (ramdisk) not loaded') - - # Kernel, FDT and Ramdisk all compressed - with ubman.log.section('(Kernel + FDT + Ramdisk) compressed'): - params['compression'] = 'gzip' - params['kernel'] = self.make_compressed(ubman, kernel) - params['fdt'] = self.make_compressed(ubman, fdt) - params['ramdisk'] = self.make_compressed(ubman, ramdisk) - fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) - params['fit'] = fit - cmd = BASE_SCRIPT % params - output = ubman.run_command_list(cmd.splitlines()) - self.check_equal(kernel, kernel_out, 'Kernel not loaded') - self.check_equal(fdt_data, fdt_out, 'FDT not loaded') - self.check_not_equal(ramdisk, ramdisk_out, 'Ramdisk got decompressed?') - self.check_equal(ramdisk + '.gz', ramdisk_out, 'Ramdisk not loaded') - - # Try without a kernel - with ubman.log.section('No kernel + FDT'): - params['kernel_config'] = '' - params['ramdisk_config'] = '' - params['ramdisk_load'] = '' - fit = fit_util.make_fit(ubman, mkimage, BASE_ITS, params) - params['fit'] = fit - cmd = BASE_SCRIPT % params - output = ubman.run_command_list(cmd.splitlines()) - assert "can't get kernel image!" in '\n'.join(output) + output = ubman.run_command_list([ + 'env set bootargs \\"\'my_boot_var=${foo}\'\\"', + 'env set foo bar', + 'bootm prep', + 'env print bootargs']) + assert 'bootargs="my_boot_var=bar"' in output, \ + "Bootargs strings not substituted" + + def test_fit_kernel_fdt_load(self, ubman, fsetup): + """Test loading a FIT image with a kernel and FDT""" + cmds, params, _ = self.prepare( + ubman, fsetup, fdt_load="load = <{params['fdt_addr']:#x}>;") + + # Run and verify + ubman.run_command_list(cmds) + self.check_equal(params, 'kernel', 'kernel_out', 'Kernel not loaded') + self.check_equal(params, 'fdt_data', 'fdt_out', 'FDT not loaded') + self.check_not_equal(params, 'ramdisk', 'ramdisk_out', + 'Ramdisk loaded but should not be') + + def test_fit_kernel_fdt_ramdisk_load(self, ubman, fsetup): + """Test loading a FIT image with kernel, FDT, and ramdisk""" + cmds, params, _ = self.prepare( + ubman, fsetup, + fdt_load="load = <{params['fdt_addr']:#x}>;", + ramdisk_config='ramdisk = "ramdisk-1";', + ramdisk_load="load = <{params['ramdisk_addr']:#x}>;") + + # Run and verify + ubman.run_command_list(cmds) + self.check_equal(params, 'ramdisk', 'ramdisk_out', 'Ramdisk not loaded') + + def test_fit_loadables_load(self, ubman, fsetup): + """Test a configuration with 'loadables' properties""" + # Configure for FDT, ramdisk, and loadables + cmds, params, _ = self.prepare( + ubman, fsetup, + fdt_load="load = <{params['fdt_addr']:#x}>;", + ramdisk_config='ramdisk = "ramdisk-1";', + ramdisk_load="load = <{params['ramdisk_addr']:#x}>;", + loadables_config='loadables="kernel-2", "ramdisk-2";', + loadables1_load="load = <{params['loadables1_addr']:#x}>;", + loadables2_load="load = <{params['loadables2_addr']:#x}>;") + + # Run and verify + ubman.run_command_list(cmds) + self.check_equal(params, 'loadables1', 'loadables1_out', + 'Loadables1 (kernel) not loaded') + self.check_equal(params, 'loadables2', 'loadables2_out', + 'Loadables2 (ramdisk) not loaded') + + def test_fit_compressed_images_load(self, ubman, fsetup): + """Test loading compressed kernel, FDT, and ramdisk images""" + # Configure for FDT and ramdisk load + cmds, params, _ = self.prepare( + ubman, fsetup, + fdt_load="load = <{params['fdt_addr']:#x}>;", + ramdisk_config='ramdisk = "ramdisk-1";', + ramdisk_load="load = <{params['ramdisk_addr']:#x}>;", + + # Configure for compression + compression='gzip', + kernel=self.make_compressed(ubman, fsetup['kernel']), + fdt=self.make_compressed(ubman, fsetup['fdt']), + ramdisk=self.make_compressed(ubman, fsetup['ramdisk'])) + + # Run and verify + ubman.run_command_list(cmds) + self.check_equal(fsetup, 'kernel', 'kernel_out', 'Kernel not loaded') + self.check_equal(fsetup, 'fdt_data', 'fdt_out', 'FDT not loaded') + self.check_not_equal(fsetup, 'ramdisk', 'ramdisk_out', + 'Ramdisk got decompressed?') + self.check_equal(params, 'ramdisk', 'ramdisk_out', 'Ramdisk not loaded') + + def test_fit_no_kernel_load(self, ubman, fsetup): + """Test that 'bootm' fails when no kernel is specified""" + # Configure for FDT load but no kernel or ramdisk + cmds = self.prepare( + ubman, fsetup, + fdt_load="load = <{params['fdt_addr']:#x}>;", + kernel_config='', + ramdisk_config='', + ramdisk_load='')[0] + + # Run and verify failure + output = ubman.run_command_list(cmds) + assert "can't get kernel image!" in '\n'.join(output) -- 2.43.0
Detect the lack of OS and deal with it through the rest of the bootm logic. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/bootm.c | 21 ++++++++++++++------- include/bootm.h | 5 +++-- 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/boot/bootm.c b/boot/bootm.c index 5caeb94a998..d19268ccd84 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -599,15 +599,19 @@ int bootm_find_images(ulong img_addr, const char *conf_ramdisk, static int bootm_find_other(ulong img_addr, const char *conf_ramdisk, const char *conf_fdt) { + bool type_ok, os_ok; + log_debug("find_other type %x os %x\n", images.os.type, images.os.os); - if ((images.os.type == IH_TYPE_KERNEL || - images.os.type == IH_TYPE_KERNEL_NOLOAD || - images.os.type == IH_TYPE_MULTI) && - (images.os.os == IH_OS_LINUX || images.os.os == IH_OS_VXWORKS || - images.os.os == IH_OS_EFI || images.os.os == IH_OS_TEE)) { + + type_ok = images.os.type == IH_TYPE_KERNEL || + images.os.type == IH_TYPE_KERNEL_NOLOAD || + images.os.type == IH_TYPE_MULTI; + os_ok = images.os.os == IH_OS_LINUX || images.os.os == IH_OS_VXWORKS || + images.os.os == IH_OS_EFI || images.os.os == IH_OS_TEE; + + if (images.no_os || (type_ok && os_ok)) return bootm_find_images(img_addr, conf_ramdisk, conf_fdt, 0, 0); - } return 0; } @@ -1075,7 +1079,7 @@ int bootm_run_states(struct bootm_info *bmi, int states) bootm_measure(images); /* Load the OS */ - if (!ret && (states & BOOTM_STATE_LOADOS)) { + if (!ret && (states & BOOTM_STATE_LOADOS) && !images->no_os) { if (IS_ENABLED(CONFIG_EVENT)) { struct event_os_load data; @@ -1113,6 +1117,9 @@ int bootm_run_states(struct bootm_info *bmi, int states) } #endif + if (images->no_os) + return -ENOPKG; + /* From now on, we need the OS boot function */ if (ret) return ret; diff --git a/include/bootm.h b/include/bootm.h index 7c74b6c406d..9ddf9fd364d 100644 --- a/include/bootm.h +++ b/include/bootm.h @@ -201,10 +201,11 @@ int bootm_measure(struct bootm_headers *images); * * @bmi: bootm information * @states Mask containing states to run (BOOTM_STATE_...) - * Return: 0 if ok, something else on error. Some errors will cause this + * Return: 0 if ok, -ENOPKG if a 'load-only' FIT was loaded and there is no OS + * to load, something else on error. Some errors will cause this * function to perform a reboot! If states contains BOOTM_STATE_OS_GO * then the intent is to boot an OS, so this function will not return - * unless the image type is standalone. + * unless the image type is standalone or this is a=. */ int bootm_run_states(struct bootm_info *bmi, int states); -- 2.43.0
When a load-only FIT without a kernel is used, we should be able to load the ramdisk and other images. Add a test for this case. Signed-off-by: Simon Glass <sjg@chromium.org> --- test/py/tests/test_fit.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py index 7f84eb1a114..8954a2ce562 100755 --- a/test/py/tests/test_fit.py +++ b/test/py/tests/test_fit.py @@ -489,3 +489,26 @@ class TestFitImage: # Run and verify failure output = ubman.run_command_list(cmds) assert "can't get kernel image!" in '\n'.join(output) + + def test_fit_no_kernel_load_only(self, ubman, fsetup): + """Test that 'bootm' handles a load-only FIT""" + cmds = self.prepare( + ubman, fsetup, + kernel_config='load-only;', + fdt_load="load = <{params['fdt_addr']:#x}>;", + ramdisk_config='ramdisk = "ramdisk-1";', + ramdisk_load="load = <{params['ramdisk_addr']:#x}>;", + loadables_config='loadables = "kernel-2", "ramdisk-2";', + loadables1_load="load = <{params['loadables1_addr']:#x}>;", + loadables2_load="load = <{params['loadables2_addr']:#x}>;")[0] + + lines = ubman.run_command_list(cmds) + output = '\n'.join(lines) + assert "can't get kernel image!" not in output + assert "Detected load-only image: skipping 'kernel'" in output + self.check_equal(fsetup, 'fdt_data', 'fdt_out', 'FDT not loaded') + self.check_equal(fsetup, 'ramdisk', 'ramdisk_out', 'Ramdisk not loaded') + self.check_equal(fsetup, 'loadables1', 'loadables1_out', + 'Loadables1 (kernel) not loaded') + self.check_equal(fsetup, 'loadables2', 'loadables2_out', + 'Loadables2 (ramdisk) not loaded') -- 2.43.0
This function returns 1 on error (at least in some cases). Change it to use an error code, so that more information can be provided. For now it mostly returns either 0 or -ENOPKG Rename the return arguments, dropping the rd_ prefix and adding a 'p' (for pointer) suffix to return arguments so that is clear that this is what they are. Update the comments for clarity and to fit with the usual style. Avoid documenting the return arguments in two places. Provide a common path for all error returns. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/image-board.c | 47 +++++++++++++++++++++++----------------------- include/image.h | 25 ++++++++++-------------- 2 files changed, 33 insertions(+), 39 deletions(-) diff --git a/boot/image-board.c b/boot/image-board.c index 8e6b2974dcf..9ce996ff9a4 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -463,12 +463,13 @@ static int select_ramdisk(struct bootm_headers *images, const char *select, u8 a } int boot_get_ramdisk(char const *select, struct bootm_headers *images, - uint arch, ulong *rd_start, ulong *rd_end) + uint arch, ulong *startp, ulong *endp) { ulong rd_data, rd_len; + int ret; - *rd_start = 0; - *rd_end = 0; + *startp = 0; + *endp = 0; /* * Look for a '-' which indicates to ignore the @@ -476,16 +477,11 @@ int boot_get_ramdisk(char const *select, struct bootm_headers *images, */ if (select && strcmp(select, "-") == 0) { debug("## Skipping init Ramdisk\n"); - rd_len = 0; - rd_data = 0; + return -ENOPKG; } else if (select || genimg_has_config(images)) { - int ret; - ret = select_ramdisk(images, select, arch, &rd_data, &rd_len); - if (ret == -ENOPKG) - return 0; - else if (ret) - return ret; + if (ret) + goto err; } else if (images->legacy_hdr_valid && image_check_type(&images->legacy_hdr_os_copy, IH_TYPE_MULTI)) { @@ -498,25 +494,28 @@ int boot_get_ramdisk(char const *select, struct bootm_headers *images, (ulong)images->legacy_hdr_os); image_multi_getimg(images->legacy_hdr_os, 1, &rd_data, &rd_len); + if (!rd_data || !rd_len) { + ret = -ENOPKG; + goto err; + } } else { - /* - * no initrd image - */ - bootstage_mark(BOOTSTAGE_ID_NO_RAMDISK); - rd_len = 0; - rd_data = 0; + /* no initrd image */ + ret = -ENOPKG; + goto err; } - if (!rd_data) { - debug("## No init Ramdisk\n"); - } else { - *rd_start = rd_data; - *rd_end = rd_data + rd_len; - } + *startp = rd_data; + *endp = rd_data + rd_len; debug(" ramdisk start = 0x%08lx, ramdisk end = 0x%08lx\n", - *rd_start, *rd_end); + *startp, *endp); return 0; + +err: + bootstage_mark(BOOTSTAGE_ID_NO_RAMDISK); + debug("## No init Ramdisk\n"); + + return ret; } /** diff --git a/include/image.h b/include/image.h index 0c5575adbbf..1dba7387d5a 100644 --- a/include/image.h +++ b/include/image.h @@ -733,27 +733,22 @@ int boot_get_fpga(struct bootm_headers *images); /** * boot_get_ramdisk() - Locate the ramdisk * + * Finds a valid ramdisk image if possible, from these ramdisk sources: + * - multicomponent kernel/ramdisk image + * - commandline-provided address of dedicated ramdisk image + * * @select: address or name of ramdisk to use, or NULL for default * @images: pointer to the bootm images structure * @arch: expected ramdisk architecture - * @rd_start: pointer to a ulong variable, will hold ramdisk start address - * @rd_end: pointer to a ulong variable, will hold ramdisk end - * - * boot_get_ramdisk() is responsible for finding a valid ramdisk image. - * Currently supported are the following ramdisk sources: - * - multicomponent kernel/ramdisk image, - * - commandline provided address of decicated ramdisk image. - * - * returns: - * 0, if ramdisk image was found and valid, or skiped - * rd_start and rd_end are set to ramdisk start/end addresses if - * ramdisk image is found and valid + * @startp: returns ramdisk start address, or 0 if none + * @endp: returns ramdisk end on success, or 0 if none * - * 1, if ramdisk image is found but corrupted, or invalid - * rd_start and rd_end are set to 0 if no ramdisk exists + * Return: 0 if ramdisk image was found and valid, or skipped; + * -ENOPKG if ramdisk image is found but corrupted, or invalid; + * other error code on other error */ int boot_get_ramdisk(char const *select, struct bootm_headers *images, - uint arch, ulong *rd_start, ulong *rd_end); + uint arch, ulong *startp, ulong *endp); /** * boot_get_loadable() - load a list of binaries to memory -- 2.43.0
If an error is returned, leave the return arguments alone. There is no point in setting them to zero, since the caller already knows (from the error code) that they are not being returned. This makes things simpler for callers which have an existing ramdisk. Handle the -ENOPKG error code in bootm_find_images() Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/bootm.c | 2 +- boot/image-board.c | 3 --- include/image.h | 4 ++-- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/boot/bootm.c b/boot/bootm.c index d19268ccd84..3431652e072 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -546,7 +546,7 @@ int bootm_find_images(ulong img_addr, const char *conf_ramdisk, /* find ramdisk */ ret = boot_get_ramdisk(select, &images, IH_INITRD_ARCH, &images.rd_start, &images.rd_end); - if (ret) { + if (ret && ret != -ENOPKG) { puts("Ramdisk image is corrupt or invalid\n"); return 1; } diff --git a/boot/image-board.c b/boot/image-board.c index 9ce996ff9a4..a0d2a7405e1 100644 --- a/boot/image-board.c +++ b/boot/image-board.c @@ -468,9 +468,6 @@ int boot_get_ramdisk(char const *select, struct bootm_headers *images, ulong rd_data, rd_len; int ret; - *startp = 0; - *endp = 0; - /* * Look for a '-' which indicates to ignore the * ramdisk argument diff --git a/include/image.h b/include/image.h index 1dba7387d5a..673c30f25e9 100644 --- a/include/image.h +++ b/include/image.h @@ -740,8 +740,8 @@ int boot_get_fpga(struct bootm_headers *images); * @select: address or name of ramdisk to use, or NULL for default * @images: pointer to the bootm images structure * @arch: expected ramdisk architecture - * @startp: returns ramdisk start address, or 0 if none - * @endp: returns ramdisk end on success, or 0 if none + * @startp: returns ramdisk start address, on success + * @endp: returns ramdisk end on success, on success * * Return: 0 if ramdisk image was found and valid, or skipped; * -ENOPKG if ramdisk image is found but corrupted, or invalid; -- 2.43.0
Tidy up the function comment. Shorten the names of the return arguments since it is clear that they relate to devicetree. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/image-fdt.c | 13 ++++++------- include/image.h | 24 +++++++++--------------- 2 files changed, 15 insertions(+), 22 deletions(-) diff --git a/boot/image-fdt.c b/boot/image-fdt.c index 3d5b6f9e2dc..fc5a3d8ff4a 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -439,14 +439,13 @@ static int select_fdt(struct bootm_headers *images, const char *select, u8 arch, } int boot_get_fdt(void *buf, const char *select, uint arch, - struct bootm_headers *images, char **of_flat_tree, - ulong *of_size) + struct bootm_headers *images, char **startp, ulong *sizep) { char *fdt_blob = NULL; ulong fdt_addr; - *of_flat_tree = NULL; - *of_size = 0; + *startp = NULL; + *sizep = 0; if (select || genimg_has_config(images)) { int ret; @@ -529,10 +528,10 @@ int boot_get_fdt(void *buf, const char *select, uint arch, goto no_fdt; } - *of_flat_tree = fdt_blob; - *of_size = fdt_totalsize(fdt_blob); + *startp = fdt_blob; + *sizep = fdt_totalsize(fdt_blob); debug(" of_flat_tree at 0x%08lx size 0x%08lx\n", - (ulong)*of_flat_tree, *of_size); + (ulong)*startp, *sizep); return 0; diff --git a/include/image.h b/include/image.h index 673c30f25e9..d38320f69cb 100644 --- a/include/image.h +++ b/include/image.h @@ -887,29 +887,23 @@ int fit_get_node_from_config(struct bootm_headers *images, /** * boot_get_fdt() - locate FDT devicetree to use for booting * + * Finds a valid flat device tree image if possible, from these sources: + * - multicomponent kernel/ramdisk/FDT image + * - commandline provided address of decicated FDT image + * * @buf: Pointer to image * @select: FDT to select (this is normally argv[2] of the bootm command) * @arch: architecture (IH_ARCH_...) * @images: pointer to the bootm images structure - * @of_flat_tree: pointer to a char* variable, will hold fdt start address - * @of_size: pointer to a ulong variable, will hold fdt length - * - * boot_get_fdt() is responsible for finding a valid flat device tree image. - * Currently supported are the following FDT sources: - * - multicomponent kernel/ramdisk/FDT image, - * - commandline provided address of decicated FDT image. - * - * Return: - * 0, if fdt image was found and valid, or skipped - * of_flat_tree and of_size are set to fdt start address and length if - * fdt image is found and valid + * @startp: returns the fdt start address, if 0 if none + * @sizep: returns the fdt length, or 0 if none * + * Return: 0 if fdt image was found and valid, or skipped; * 1, if fdt image is found but corrupted - * of_flat_tree and of_size are set to 0 if no fdt exists */ int boot_get_fdt(void *buf, const char *select, uint arch, - struct bootm_headers *images, char **of_flat_tree, - ulong *of_size); + struct bootm_headers *images, char **startp, + ulong *sizep); void boot_fdt_add_mem_rsv_regions(void *fdt_blob); int boot_relocate_fdt(char **of_flat_tree, ulong *of_size); -- 2.43.0
If an error is returned, leave the return arguments alone. There is no point in setting them to zero, since the caller already knows (from the error code) that they are not being returned. This makes things simpler for callers which have an existing devicetree. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/image-fdt.c | 3 --- include/image.h | 4 ++-- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/boot/image-fdt.c b/boot/image-fdt.c index fc5a3d8ff4a..6859a7c848c 100644 --- a/boot/image-fdt.c +++ b/boot/image-fdt.c @@ -444,9 +444,6 @@ int boot_get_fdt(void *buf, const char *select, uint arch, char *fdt_blob = NULL; ulong fdt_addr; - *startp = NULL; - *sizep = 0; - if (select || genimg_has_config(images)) { int ret; diff --git a/include/image.h b/include/image.h index d38320f69cb..46b91888f5b 100644 --- a/include/image.h +++ b/include/image.h @@ -895,8 +895,8 @@ int fit_get_node_from_config(struct bootm_headers *images, * @select: FDT to select (this is normally argv[2] of the bootm command) * @arch: architecture (IH_ARCH_...) * @images: pointer to the bootm images structure - * @startp: returns the fdt start address, if 0 if none - * @sizep: returns the fdt length, or 0 if none + * @startp: returns the fdt start address, on success + * @sizep: returns the fdt length, on success * * Return: 0 if fdt image was found and valid, or skipped; * 1, if fdt image is found but corrupted -- 2.43.0
Normally bootm proceeds sequentially through the various states, from 'start' to 'go'. In some cases we want to load the devicetree from one FIT and the kernel and ramdisk from another. This requires two bootm commands. Of course it is possible to just do a second 'bootm start' to load the kernel. But that removes all record of the devicetree from the boot_images information, so it is then not provided to the OS. Add a 'restart' subcommand which allows more images to be loaded, without erasing those already loaded. Signed-off-by: Simon Glass <sjg@chromium.org> --- boot/bootm.c | 13 +++++++++++++ cmd/bootm.c | 5 ++++- doc/usage/cmd/bootm.rst | 12 ++++++++++-- include/bootstage.h | 1 + include/image.h | 2 ++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/boot/bootm.c b/boot/bootm.c index 3431652e072..46320363270 100644 --- a/boot/bootm.c +++ b/boot/bootm.c @@ -257,6 +257,16 @@ static int bootm_start(void) return 0; } +static int bootm_restart(void) +{ + images.no_os = false; + + bootstage_mark_name(BOOTSTAGE_ID_BOOTM_RESTART, "bootm_restart"); + images.state = BOOTM_STATE_START; + + return 0; +} + static ulong bootm_data_addr(const char *addr_str) { ulong addr; @@ -1059,6 +1069,9 @@ int bootm_run_states(struct bootm_info *bmi, int states) if (states & BOOTM_STATE_START) ret = bootm_start(); + if (states & BOOTM_STATE_RESTART) + ret = bootm_restart(); + if (!ret && (states & BOOTM_STATE_PRE_LOAD)) ret = bootm_pre_load(bmi->addr_img); diff --git a/cmd/bootm.c b/cmd/bootm.c index bee683d0580..23b3d56c101 100644 --- a/cmd/bootm.c +++ b/cmd/bootm.c @@ -41,6 +41,7 @@ static int do_imls(struct cmd_tbl *cmdtp, int flag, int argc, * function pointer */ static struct cmd_tbl cmd_bootm_sub[] = { U_BOOT_CMD_MKENT(start, 0, 1, (void *)BOOTM_STATE_START, "", ""), + U_BOOT_CMD_MKENT(restart, 0, 1, (void *)BOOTM_STATE_RESTART, "", ""), U_BOOT_CMD_MKENT(loados, 0, 1, (void *)BOOTM_STATE_LOADOS, "", ""), #ifdef CONFIG_CMD_BOOTM_PRE_LOAD U_BOOT_CMD_MKENT(preload, 0, 1, (void *)BOOTM_STATE_PRE_LOAD, "", ""), @@ -85,7 +86,9 @@ static int do_bootm_subcommand(struct cmd_tbl *cmdtp, int flag, int argc, if (c) { state = (long)c->cmd; - if (state == BOOTM_STATE_START) + if (state == BOOTM_STATE_RESTART) + images.state = BOOTM_STATE_START; + if (state == BOOTM_STATE_START || state == BOOTM_STATE_RESTART) state |= BOOTM_STATE_PRE_LOAD | BOOTM_STATE_FINDOS | BOOTM_STATE_FINDOTHER; #if defined(CONFIG_CMD_BOOTM_PRE_LOAD) diff --git a/doc/usage/cmd/bootm.rst b/doc/usage/cmd/bootm.rst index 1f59895568e..5af331873ce 100644 --- a/doc/usage/cmd/bootm.rst +++ b/doc/usage/cmd/bootm.rst @@ -11,8 +11,8 @@ Synopsis :: - bootm [start] [<fit_addr>]#<conf>[#<extra-conf>] - bootm [start] [[<fit_addr>]:<os_subimg>] [[<fit_addr2>]:<rd_subimg2>] [[<fit_addr3>]:<fdt_subimg>] + bootm [start|restart] [<fit_addr>]#<conf>[#<extra-conf>] + bootm [start|restart] [[<fit_addr>]:<os_subimg>] [[<fit_addr2>]:<rd_subimg2>] [[<fit_addr3>]:<fdt_subimg>] bootm <subcmd> bootm <addr1> [[<addr2> [<addr3>]] # Legacy boot @@ -64,6 +64,14 @@ The states are described below: start Start the boot process afresh, recording the image(s) to be booted. +restart + Start the boot process again, but keeping the current state. This allows + multiple FITs to be loaded, for example a first FIT containing just the + devicetree and a second containing the OS and any overlays. In this case, + the first `bootm` command will typically use only `start` (and its implicit + states) and `loados`, with the second using `bootm restart` to select the + second FIT. + preload Deal with any preload step, sometimes used to do a full signature check of the FIT, before looking at any of the data within. diff --git a/include/bootstage.h b/include/bootstage.h index 9471c5d770f..ad98bffedc3 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -189,6 +189,7 @@ enum bootstage_id { BOOTSTAGE_ID_BOOTP_START, BOOTSTAGE_ID_BOOTP_STOP, BOOTSTAGE_ID_BOOTM_START, + BOOTSTAGE_ID_BOOTM_RESTART, BOOTSTAGE_ID_BOOTM_HANDOFF, BOOTSTAGE_ID_MAIN_LOOP, BOOTSTAGE_ID_ENTER_CLI_LOOP, diff --git a/include/image.h b/include/image.h index 46b91888f5b..7576ffa1048 100644 --- a/include/image.h +++ b/include/image.h @@ -349,6 +349,7 @@ struct image_info { * enum bootm_state - States which the bootm machine goes through (in order) * * @BOOTM_STATE_START: Set up the state structure (struct bootm_headers) + * @BOOTM_STATE_RESTART: Restart the boot, keeping the existing state * @BOOTM_STATE_PRE_LOAD: Do any neceessary processing before images are read. * For now this just implements a whole-image signature, if enabled. See * CONFIG_IMAGE_PRE_LOAD_SIG @@ -398,6 +399,7 @@ enum bootm_state { BOOTM_STATE_OS_PREP = BIT(10), BOOTM_STATE_OS_FAKE_GO = BIT(11), BOOTM_STATE_OS_GO = BIT(12), + BOOTM_STATE_RESTART = BIT(13), }; /* -- 2.43.0
Check that we can read a FIT containing a devicetree, then boot with a second FIT. This requires a few more FIT parameters and commands to be configurable. Signed-off-by: Simon Glass <sjg@chromium.org> --- test/py/tests/test_fit.py | 59 +++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 5 deletions(-) diff --git a/test/py/tests/test_fit.py b/test/py/tests/test_fit.py index 8954a2ce562..ed18ff68825 100755 --- a/test/py/tests/test_fit.py +++ b/test/py/tests/test_fit.py @@ -72,7 +72,7 @@ BASE_ITS = ''' default = "conf-1"; conf-1 { %(kernel_config)s - fdt = "fdt-1"; + %(fdt_config)s %(ramdisk_config)s %(loadables_config)s }; @@ -105,11 +105,11 @@ BASE_FDT = ''' # then run the 'bootm' command, then save out memory from the places where # we expect 'bootm' to write things. Then quit. BASE_SCRIPT = ''' -mw 0 0 180000 +%(mem_clear)s host load hostfs 0 %(fit_addr)x %(fit)s fdt addr %(fit_addr)x -bootm start %(fit_addr)x -bootm loados +bootm %(start_subcmd)s %(fit_addr)x +bootm loados%(cmd_extra)s host save hostfs 0 %(kernel_addr)x %(kernel_out)s %(kernel_size)x host save hostfs 0 %(fdt_addr)x %(fdt_out)s %(fdt_size)x host save hostfs 0 %(ramdisk_addr)x %(ramdisk_out)s %(ramdisk_size)x @@ -313,6 +313,7 @@ class TestFitImage: 'fdt_addr' : 0x80000, 'fdt_size' : self.filesize(fdt_data), 'fdt_load' : '', + 'fdt_config' : 'fdt = "fdt-1";', 'ramdisk' : ramdisk, 'ramdisk_out' : ramdisk_out, @@ -335,6 +336,11 @@ class TestFitImage: 'loadables_config' : '', 'compression' : 'none', + + 'mem_clear': 'mw 0 0 180000', + 'start_subcmd': 'start', + 'cmd_extra': '', + 'fit_basename': 'test.fit', } # Yield all necessary components to the tests @@ -371,7 +377,8 @@ class TestFitImage: params[name] = eval(f'f"""{template}"""') # Make a basic FIT and a script to load it - fit = fit_util.make_fit(ubman, params['mkimage'], BASE_ITS, params) + fit = fit_util.make_fit(ubman, params['mkimage'], BASE_ITS, params, + basename=params['fit_basename']) params['fit'] = fit cmds = BASE_SCRIPT % params @@ -512,3 +519,45 @@ class TestFitImage: 'Loadables1 (kernel) not loaded') self.check_equal(fsetup, 'loadables2', 'loadables2_out', 'Loadables2 (ramdisk) not loaded') + + def test_fit_no_kernel_second(self, ubman, fsetup): + """Test that 'bootm' handles a load-only FIT and a second FIT""" + cmds = self.prepare( + ubman, fsetup, + kernel_config='load-only;', + # Just load a devicetree for the first FIT + ramdisk_config='', + ramdisk_load='', + fdt_load="load = <{params['fdt_addr']:#x}>;")[0] + + lines = ubman.run_command_list(cmds) + output = '\n'.join(lines) + assert "can't get kernel image!" not in output + assert "Detected load-only image: skipping 'kernel'" in output + self.check_equal(fsetup, 'fdt_data', 'fdt_out', 'FDT not loaded') + + # Now load a second FIT which has a kernel and ramdisk + cmds = self.prepare( + ubman, fsetup, + fdt_config= '', + fdt_load='', + ramdisk_config='ramdisk = "ramdisk-1";', + ramdisk_load="load = <{params['ramdisk_addr']:#x}>;", + # fit=fit + mem_clear='', + + # Run the prep stage so we can check that the FDT-fixups are done + cmd_extra='\nbootm cmdline\nbootm prep', + start_subcmd='restart', + fit_basename='second.fit')[0] + + # Run and verify + lines = ubman.run_command_list(cmds) + output = '\n'.join(lines) + assert "can't get kernel image!" not in output + assert "Detected load-only image: skipping 'kernel'" not in output + self.check_equal(fsetup, 'kernel', 'kernel_out', 'Kernel not loaded') + self.check_equal(fsetup, 'ramdisk', 'ramdisk_out', 'Ramdisk not loaded') + + # Check that the devicetree is still there + self.check_equal(fsetup, 'fdt_data', 'fdt_out', 'FDT not loaded') -- 2.43.0
participants (1)
-
Simon Glass