[U-Boot] [PATCH 0/4] Correct some remaining bootm problems
The tortuous refactoring of bootm and fit_image_load() has thrown up four more issues:
- Support for bootm (without arguments) on many OSes is broken since the OS functions do not handle the PREP stage, which is now required - In the case of loading a FIT image with a bootm-selected kernel configuration, this is not used for ramdisk and fdt but must be manually specified for each. - The Elf image loader arguments are out by 1 - There is an extra OK message in the case of loading an uncompressed kernel
This series corrects these problems. Verification was done using some sandbox tests written for the occassion. These will be sent to the list during the next merge window (or earlier if anyone wants to try them).
For the record the tests run are:
$ ./test/image/test-fit.py -u b/sandbox/u-boot FIT Tests ========= Kernel load Kernel + FDT load Kernel + FDT + Ramdisk load Kernel + FDT + Ramdisk load, Config 2
Tests passed Caveat: this is only a sanity check - test coverage is poor $ ./test/image/test-legacy.py -u b/sandbox/u-boot Legacy Image Tests ================== Testing 22 image types ['netbsd', 'lynxos', 'rtems', 'ose', 'plan9', 'vxworks', 'qnx', 'integrity'] netbsd: False netbsd: True lynxos: False lynxos: True rtems: False rtems: True ose: False ose: True plan9: False plan9: True vxworks: False vxworks: True qnx: False qnx: True integrity: False integrity: True
Tests passed Caveat: this is only a sanity check - test coverage is poor
Simon Glass (4): bootm: Remove extra OK message blackfin: x86: bootm: Handle PREP stage of bootm bootm: Use selected configuration for ramdisk and fdt bootm: Correct the arguments for the ELF image loader
arch/blackfin/lib/boot.c | 2 ++ arch/x86/lib/bootm.c | 2 ++ common/cmd_bootm.c | 20 ++++++++++++++++++-- common/cmd_elf.c | 6 +++--- common/image-fdt.c | 4 ++-- common/image-fit.c | 6 +++++- common/image.c | 4 ++-- include/image.h | 7 ++++--- 8 files changed, 38 insertions(+), 13 deletions(-)
This is not needed as we already print 'OK' later in all cases.
Signed-off-by: Simon Glass sjg@chromium.org --- common/cmd_bootm.c | 1 - 1 file changed, 1 deletion(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index a783cea..7a7c760 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -396,7 +396,6 @@ static int bootm_load_os(bootm_headers_t *images, unsigned long *load_end, memmove_wd(load_buf, image_buf, image_len, CHUNKSZ); } *load_end = load + image_len; - puts("OK\n"); break; #ifdef CONFIG_GZIP case IH_COMP_GZIP:
The OS function is now always called with the PREP stage. Adjust the remaining bootm OS functions to deal with this correctly.
Signed-off-by: Simon Glass sjg@chromium.org --- arch/blackfin/lib/boot.c | 2 ++ arch/x86/lib/bootm.c | 2 ++ common/cmd_bootm.c | 16 ++++++++++++++++ 3 files changed, 20 insertions(+)
diff --git a/arch/blackfin/lib/boot.c b/arch/blackfin/lib/boot.c index 768a882..5644d58 100644 --- a/arch/blackfin/lib/boot.c +++ b/arch/blackfin/lib/boot.c @@ -42,6 +42,8 @@ int do_bootm_linux(int flag, int argc, char * const argv[], bootm_headers_t *ima int (*appl) (char *cmdline); char *cmdline;
+ if (flag & BOOTM_STATE_OS_PREP) + return 0; if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1;
diff --git a/arch/x86/lib/bootm.c b/arch/x86/lib/bootm.c index 0d3250c..b84e35a 100644 --- a/arch/x86/lib/bootm.c +++ b/arch/x86/lib/bootm.c @@ -48,6 +48,8 @@ int do_bootm_linux(int flag, int argc, char * const argv[], size_t len; #endif
+ if (flag & BOOTM_STATE_OS_PREP) + return 0; if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1;
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 7a7c760..075e7dc 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -1473,6 +1473,8 @@ static int do_bootm_netbsd(int flag, int argc, char * const argv[], char *consdev; char *cmdline;
+ if (flag & BOOTM_STATE_OS_PREP) + return 0; if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1;
@@ -1552,6 +1554,8 @@ static int do_bootm_lynxkdi(int flag, int argc, char * const argv[], { image_header_t *hdr = &images->legacy_hdr_os_copy;
+ if (flag & BOOTM_STATE_OS_PREP) + return 0; if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1;
@@ -1574,6 +1578,8 @@ static int do_bootm_rtems(int flag, int argc, char * const argv[], { void (*entry_point)(bd_t *);
+ if (flag & BOOTM_STATE_OS_PREP) + return 0; if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1;
@@ -1607,6 +1613,8 @@ static int do_bootm_ose(int flag, int argc, char * const argv[], { void (*entry_point)(void);
+ if (flag & BOOTM_STATE_OS_PREP) + return 0; if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1;
@@ -1641,6 +1649,8 @@ static int do_bootm_plan9(int flag, int argc, char * const argv[], void (*entry_point)(void); char *s;
+ if (flag & BOOTM_STATE_OS_PREP) + return 0; if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1;
@@ -1688,6 +1698,8 @@ static int do_bootm_vxworks(int flag, int argc, char * const argv[], { char str[80];
+ if (flag & BOOTM_STATE_OS_PREP) + return 0; if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1;
@@ -1711,6 +1723,8 @@ static int do_bootm_qnxelf(int flag, int argc, char * const argv[], char *local_args[2]; char str[16];
+ if (flag & BOOTM_STATE_OS_PREP) + return 0; if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1;
@@ -1736,6 +1750,8 @@ static int do_bootm_integrity(int flag, int argc, char * const argv[], { void (*entry_point)(void);
+ if (flag & BOOTM_STATE_OS_PREP) + return 0; if ((flag != 0) && (flag != BOOTM_STATE_OS_GO)) return 1;
On Wed Jul 10, 2013 at 11:08:09PM -0700, Simon Glass wrote:
The OS function is now always called with the PREP stage. Adjust the remaining bootm OS functions to deal with this correctly.
Signed-off-by: Simon Glass sjg@chromium.org
arch/blackfin/lib/boot.c | 2 ++ arch/x86/lib/bootm.c | 2 ++ common/cmd_bootm.c | 16 ++++++++++++++++ 3 files changed, 20 insertions(+)
diff --git a/arch/blackfin/lib/boot.c b/arch/blackfin/lib/boot.c index 768a882..5644d58 100644 --- a/arch/blackfin/lib/boot.c +++ b/arch/blackfin/lib/boot.c @@ -42,6 +42,8 @@ int do_bootm_linux(int flag, int argc, char * const argv[], bootm_headers_t *ima int (*appl) (char *cmdline); char *cmdline;
- if (flag & BOOTM_STATE_OS_PREP)
return 0;
Minor nit; A blank line between the two if conditions would be good. Applies for all the hunks in this patch.
-sughosh
If a specific configuraion is selected by the bootm command, e.g. with 'bootm 84000000#recoveryconf' we must honour this for not just the kernel, but also the ramdisk and FDT.
In the conversion to using a common fit_image_load() function for loading images from FITs (commits a51ec63 and 53f375f) this feature was lost. Reinstate it by passing the selected configuration back from fit_image_load() to boot_get_kernel(), then use this configuration (which is stored in images->fit_uname_cfg) in both boot_get_ramdisk() and boot_get_fdt().
Signed-off-by: Simon Glass sjg@chromium.org --- common/cmd_bootm.c | 3 ++- common/image-fdt.c | 4 ++-- common/image-fit.c | 6 +++++- common/image.c | 4 ++-- include/image.h | 7 ++++--- 5 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index 075e7dc..c8bb33e 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -989,7 +989,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, case IMAGE_FORMAT_FIT: os_noffset = fit_image_load(images, FIT_KERNEL_PROP, img_addr, - &fit_uname_kernel, fit_uname_config, + &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL, BOOTSTAGE_ID_FIT_KERNEL_START, FIT_LOAD_IGNORED, os_data, os_len); @@ -998,6 +998,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
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; break; #endif diff --git a/common/image-fdt.c b/common/image-fdt.c index d99f444..203404a 100644 --- a/common/image-fdt.c +++ b/common/image-fdt.c @@ -243,7 +243,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, ulong load, load_end; void *buf; #if defined(CONFIG_FIT) - const char *fit_uname_config = NULL; + const char *fit_uname_config = images->fit_uname_cfg; const char *fit_uname_fdt = NULL; ulong default_addr; int fdt_noffset; @@ -367,7 +367,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch, fdt_noffset = fit_image_load(images, FIT_FDT_PROP, fdt_addr, &fit_uname_fdt, - fit_uname_config, + &fit_uname_config, arch, IH_TYPE_FLATDT, BOOTSTAGE_ID_FIT_FDT_START, FIT_LOAD_OPTIONAL, &load, &len); diff --git a/common/image-fit.c b/common/image-fit.c index b75e119..e28dd05 100644 --- a/common/image-fit.c +++ b/common/image-fit.c @@ -1478,12 +1478,13 @@ int fit_get_node_from_config(bootm_headers_t *images, const char *prop_name, }
int fit_image_load(bootm_headers_t *images, const char *prop_name, ulong addr, - const char **fit_unamep, const char *fit_uname_config, + const char **fit_unamep, const char **fit_uname_configp, int arch, int image_type, int bootstage_id, enum fit_load_op load_op, ulong *datap, ulong *lenp) { int cfg_noffset, noffset; const char *fit_uname; + const char *fit_uname_config; const void *fit; const void *buf; size_t size; @@ -1493,6 +1494,7 @@ int fit_image_load(bootm_headers_t *images, const char *prop_name, ulong addr,
fit = map_sysmem(addr, 0); fit_uname = fit_unamep ? *fit_unamep : NULL; + fit_uname_config = fit_uname_configp ? *fit_uname_configp : NULL; printf("## Loading %s from FIT Image at %08lx ...\n", prop_name, addr);
bootstage_mark(bootstage_id + BOOTSTAGE_SUB_FORMAT); @@ -1658,6 +1660,8 @@ int fit_image_load(bootm_headers_t *images, const char *prop_name, ulong addr, *lenp = len; if (fit_unamep) *fit_unamep = (char *)fit_uname; + if (fit_uname_configp) + *fit_uname_configp = (char *)fit_uname_config;
return noffset; } diff --git a/common/image.c b/common/image.c index 1be384f..327006e 100644 --- a/common/image.c +++ b/common/image.c @@ -811,7 +811,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, char *end; #endif #if defined(CONFIG_FIT) - const char *fit_uname_config = NULL; + const char *fit_uname_config = images->fit_uname_cfg; const char *fit_uname_ramdisk = NULL; ulong default_addr; int rd_noffset; @@ -907,7 +907,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, case IMAGE_FORMAT_FIT: rd_noffset = fit_image_load(images, FIT_RAMDISK_PROP, rd_addr, &fit_uname_ramdisk, - fit_uname_config, arch, + &fit_uname_config, arch, IH_TYPE_RAMDISK, BOOTSTAGE_ID_FIT_RD_START, FIT_LOAD_REQUIRED, &rd_data, &rd_len); diff --git a/include/image.h b/include/image.h index 9c3e46f..7b0108f 100644 --- a/include/image.h +++ b/include/image.h @@ -439,8 +439,9 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, * @param fit_unamep On entry this is the requested image name * (e.g. "kernel@1") or NULL to use the default. On exit * points to the selected image name - * @param fit_uname_config Requested configuration name, or NULL for the - * default + * @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. * @param arch Expected architecture (IH_ARCH_...) * @param image_type Required image type (IH_TYPE_...). If this is * IH_TYPE_KERNEL then we allow IH_TYPE_KERNEL_NOLOAD @@ -453,7 +454,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images, * @param lenp Returns length of loaded image */ int fit_image_load(bootm_headers_t *images, const char *prop_name, ulong addr, - const char **fit_unamep, const char *fit_uname_config, + const char **fit_unamep, const char **fit_uname_configp, int arch, int image_type, int bootstage_id, enum fit_load_op load_op, ulong *datap, ulong *lenp);
The arguments were out of place since commit 983c72f, since this file was missed and not tested. Correct this.
Signed-off-by: Simon Glass sjg@chromium.org --- common/cmd_elf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/common/cmd_elf.c b/common/cmd_elf.c index ab9c7e3..f741f6b 100644 --- a/common/cmd_elf.c +++ b/common/cmd_elf.c @@ -156,16 +156,16 @@ int do_bootvx(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) * If we don't know where the image is then we're done. */
- if (argc < 2) + if (argc < 1) addr = load_addr; else - addr = simple_strtoul(argv[1], NULL, 16); + addr = simple_strtoul(argv[0], NULL, 16);
#if defined(CONFIG_CMD_NET) /* * Check to see if we need to tftp the image ourselves before starting */ - if ((argc == 2) && (strcmp(argv[1], "tftp") == 0)) { + if ((argc == 1) && (strcmp(argv[0], "tftp") == 0)) { if (NetLoop(TFTPGET) <= 0) return 1; printf("Automatic boot of VxWorks image at address 0x%08lx ...\n",
On Wed, Jul 10, 2013 at 11:08:07PM -0700, Simon Glass wrote:
The tortuous refactoring of bootm and fit_image_load() has thrown up four more issues:
- Support for bootm (without arguments) on many OSes is broken since the
OS functions do not handle the PREP stage, which is now required
- In the case of loading a FIT image with a bootm-selected kernel
configuration, this is not used for ramdisk and fdt but must be manually specified for each.
- The Elf image loader arguments are out by 1
- There is an extra OK message in the case of loading an uncompressed
kernel
This series corrects these problems. Verification was done using some sandbox tests written for the occassion. These will be sent to the list during the next merge window (or earlier if anyone wants to try them).
Applied to u-boot/master, thanks!
participants (3)
-
Simon Glass -
Sughosh Ganu -
Tom Rini