Correct some remaining bootm problems

PATCH New Simon Glass 3 participants 7 messages Jul 11, 2013 – Jul 12, 2013 x86

lore

Share & tools

lore

Simon Glass

author

2013-07-11 06:08 UTC #0 lore raw

cover 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(-)

Signature · 1 line

1.8.3

Simon Glass

author

2013-07-11 06:08 UTC #1 lore raw

PATCH 1/4 bootm: Remove extra OK message

This is not needed as we already print 'OK' later in all cases.

Signed-off-by Simon Glass <[email protected]>

---
 common/cmd_bootm.c | 1 -
 1 file changed, 1 deletion(-)

common/cmd_bootm.c +0 -1

index a783cea..7a7c760 100644
@@ -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:

Signature · 1 line

1.8.3

Simon Glass

author

2013-07-11 06:08 UTC #2 lore raw

PATCH 2/4 blackfin: x86: bootm: Handle PREP stage of bootm

In reply to Simon Glass

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 <[email protected]>

---
 arch/blackfin/lib/boot.c |  2 ++
 arch/x86/lib/bootm.c     |  2 ++
 common/cmd_bootm.c       | 16 ++++++++++++++++
 3 files changed, 20 insertions(+)

arch/blackfin/lib/boot.c +2 -0

index 768a882..5644d58 100644
@@ -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;

arch/x86/lib/bootm.c +2 -0

index 0d3250c..b84e35a 100644
@@ -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;

common/cmd_bootm.c +16 -0

index 7a7c760..075e7dc 100644
@@ -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;

Signature · 1 line

1.8.3

Sughosh Ganu

2013-07-12 08:28 UTC #3 lore raw

PATCH 2/4 blackfin: x86: bootm: Handle PREP stage of bootm

Quoted 15 lines · Simon Glass

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 <[email protected]>
---
 arch/blackfin/lib/boot.c |  2 ++
 arch/x86/lib/bootm.c     |  2 ++
 common/cmd_bootm.c       | 16 ++++++++++++++++
 3 files changed, 20 insertions(+)
arch/blackfin/lib/boot.c +0 -0
index 768a882..5644d58 100644

arch/blackfin/lib/boot.c int do_bootm_linux(int flag, int argc, char * const argv[], bootm_headers_t *ima · line 48

Quoted 7 lines

arch/blackfin/lib/boot.c +2 -0

@@ -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;

Minor nit; A blank line between the two if conditions would be good. Applies for all the hunks in this patch.

-sughosh

Simon Glass

author

2013-07-11 06:08 UTC #4 lore raw

PATCH 3/4 bootm: Use selected configuration for ramdisk and fdt

In reply to Simon Glass

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 <[email protected]>

---
 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(-)

common/cmd_bootm.c +2 -1

index 075e7dc..c8bb33e 100644
@@ -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

common/image-fdt.c +2 -2

index d99f444..203404a 100644
@@ -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);

common/image-fit.c +5 -1

index b75e119..e28dd05 100644
@@ -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;
 }

common/image.c +2 -2

index 1be384f..327006e 100644
@@ -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);

include/image.h +4 -3

index 9c3e46f..7b0108f 100644
@@ -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 at 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 at 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);

Signature · 1 line

1.8.3

Simon Glass

author

2013-07-11 06:08 UTC #5 lore raw

PATCH 4/4 bootm: Correct the arguments for the ELF image loader

In reply to Simon Glass

The arguments were out of place since commit 983c72f, since this file was missed and not tested. Correct this.

Signed-off-by Simon Glass <[email protected]>

---
 common/cmd_elf.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

common/cmd_elf.c +3 -3

index ab9c7e3..f741f6b 100644
@@ -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@address 0x%08lx ...\n",

Signature · 1 line

1.8.3

Tom Rini

2013-07-12 21:23 UTC #6 lore raw

cover Correct some remaining bootm problems

In reply to Simon Glass

Quoted 12 lines · Simon Glass

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

Quoted 3 lines

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!

Signature · 8 lines

Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130712/0a7b08fd/attachment.pgp>