[U-Boot] [PATCH] bootm: Reinstate special case for standalone images
For standalone images, bootm had a special case where the OS boot function was NULL but did actually exist. It was just called manually.
This was removed by commit 35fc84fa which checks for the non-existence of this function before the special case is examined.
There is no obvious reason why standalone is handled with a special case. Adjust the code so that standalone has a normal OS boot function. We still need a special case for when the function returns, but at least we can avoid the main problem.
This is intended to fix the reported:
ERROR: booting os 'U-Boot' (17) is not supported
but needs testing.
Signed-off-by: Simon Glass sjg@chromium.org ---
common/cmd_bootm.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index ba73f57..2120aa0 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -77,6 +77,9 @@ static int do_imls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); static void fixup_silent_linux(void); #endif
+static int do_bootm_standalone(int flag, int argc, char * const argv[], + bootm_headers_t *images); + static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], bootm_headers_t *images, ulong *os_data, ulong *os_len); @@ -131,6 +134,7 @@ static boot_os_fn do_bootm_integrity; #endif
static boot_os_fn *boot_os[] = { + [IH_TYPE_STANDALONE] = do_bootm_standalone, #ifdef CONFIG_BOOTM_LINUX [IH_OS_LINUX] = do_bootm_linux, #endif @@ -487,17 +491,18 @@ static int bootm_load_os(bootm_headers_t *images, unsigned long *load_end, return 0; }
-static int bootm_start_standalone(int argc, char * const argv[]) +static int do_bootm_standalone(int flag, int argc, char * const argv[], + bootm_headers_t *images) { char *s; int (*appl)(int, char * const []);
/* Don't start if "autostart" is set to "no" */ if (((s = getenv("autostart")) != NULL) && (strcmp(s, "no") == 0)) { - setenv_hex("filesize", images.os.image_len); + setenv_hex("filesize", images->os.image_len); return 0; } - appl = (int (*)(int, char * const []))(ulong)ntohl(images.ep); + appl = (int (*)(int, char * const []))(ulong)ntohl(images->ep); (*appl)(argc, argv); return 0; } @@ -523,14 +528,12 @@ static cmd_tbl_t cmd_bootm_sub[] = { static int boot_selected_os(int argc, char * const argv[], int state, bootm_headers_t *images, boot_os_fn *boot_fn) { - if (images->os.type == IH_TYPE_STANDALONE) { - /* This may return when 'autostart' is 'no' */ - bootm_start_standalone(argc, argv); - return 0; - } arch_preboot_os(); boot_fn(state, argc, argv, images); - if (state == BOOTM_STATE_OS_FAKE_GO) /* We expect to return */ + + /* Stand-alone may return when 'autostart' is 'no' */ + if (images->os.type == IH_TYPE_STANDALONE || + state == BOOTM_STATE_OS_FAKE_GO) /* We expect to return */ return 0; bootstage_error(BOOTSTAGE_ID_BOOT_OS_RETURNED); #ifdef DEBUG
On 12/07/2013 12:26 AM, Simon Glass wrote:
For standalone images, bootm had a special case where the OS boot function was NULL but did actually exist. It was just called manually.
This was removed by commit 35fc84fa which checks for the non-existence of this function before the special case is examined.
There is no obvious reason why standalone is handled with a special case. Adjust the code so that standalone has a normal OS boot function. We still need a special case for when the function returns, but at least we can avoid the main problem.
This is intended to fix the reported:
ERROR: booting os 'U-Boot' (17) is not supportedbut needs testing.
Signed-off-by: Simon Glass sjg@chromium.org
common/cmd_bootm.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index ba73f57..2120aa0 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -77,6 +77,9 @@ static int do_imls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); static void fixup_silent_linux(void); #endif
+static int do_bootm_standalone(int flag, int argc, char * const argv[],
bootm_headers_t *images);static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], bootm_headers_t *images, ulong *os_data, ulong *os_len); @@ -131,6 +134,7 @@ static boot_os_fn do_bootm_integrity; #endif
static boot_os_fn *boot_os[] = {
- [IH_TYPE_STANDALONE] = do_bootm_standalone,
This should be IH_OS_U_BOOT
#ifdef CONFIG_BOOTM_LINUX [IH_OS_LINUX] = do_bootm_linux, #endif @@ -487,17 +491,18 @@ static int bootm_load_os(bootm_headers_t *images, unsigned long *load_end, return 0; }
-static int bootm_start_standalone(int argc, char * const argv[]) +static int do_bootm_standalone(int flag, int argc, char * const argv[],
bootm_headers_t *images){ char *s; int (*appl)(int, char * const []);
/* Don't start if "autostart" is set to "no" */ if (((s = getenv("autostart")) != NULL) && (strcmp(s, "no") == 0)) {
setenv_hex("filesize", images.os.image_len);
setenv_hex("filesize", images->os.image_len);
- appl = (int (*)(int, char * const []))(ulong)ntohl(images.ep);
- appl = (int (*)(int, char * const []))(ulong)ntohl(images->ep); (*appl)(argc, argv); return 0;
} @@ -523,14 +528,12 @@ static cmd_tbl_t cmd_bootm_sub[] = { static int boot_selected_os(int argc, char * const argv[], int state, bootm_headers_t *images, boot_os_fn *boot_fn) {
- if (images->os.type == IH_TYPE_STANDALONE) {
/* This may return when 'autostart' is 'no' */bootm_start_standalone(argc, argv);return 0;- } arch_preboot_os(); boot_fn(state, argc, argv, images);
- if (state == BOOTM_STATE_OS_FAKE_GO) /* We expect to return */
- /* Stand-alone may return when 'autostart' is 'no' */
- if (images->os.type == IH_TYPE_STANDALONE ||
state == BOOTM_STATE_OS_FAKE_GO) /* We expect to return */#ifdef DEBUG
But anyway I have tested this in zynq. Image generation by this command
./tools/mkimage -n "hello" -A arm -O u-boot -T standalone -C none -a c100000 -d examples/standalone/hello_world.bin -v /tftpboot/hello.ub
on 2013.04 is behaviour
U-Boot> bootm ## Booting kernel from Legacy Image at 00001000 ... Image Name: hello Image Type: ARM U-Boot Standalone Program (uncompressed) Data Size: 594 Bytes = 594 Bytes Load Address: 0c100000 Entry Point: 0c100000 Verifying Checksum ... OK Loading Standalone Program ... OK OK Example expects ABI version 6 Actual U-Boot ABI version 6 Hello World argc = 0 data abort
MAYBE you should read doc/README.arm-unaligned-accesses
pc : [<3ffd8e74>] lr : [<3ffd9574>] sp : 3fbadc20 ip : 00000043 fp : 3ffea2c4 r10: ffffffff r9 : 3fbaddd4 r8 : 3fbadf40 r7 : 0c100232 r6 : ea000014 r5 : ffffffff r4 : 3fbadcaf r3 : ea000014 r2 : ea000014 r1 : ea000013 r0 : ea000014 Flags: nzCv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
resetting ...
on the latest&greatest with fix above
U-Boot 2013.10-00793-gc9ca75a-dirty (Dec 10 2013 - 06:30:03)
I2C: ready Memory: ECC disabled DRAM: 1 GiB MMC: zynq_sdhci: 0 SF: Detected N25Q128A with page size 256 Bytes, erase size 4 KiB, total 16 MiB *** Warning - bad CRC, using default environment
In: serial Out: serial Err: serial Net: Gem.e000b000 Hit any key to stop autoboot: 0 zynq-uboot> set serverip 192.168.0.100 zynq-uboot> set ipaddr 192.168.0.10 zynq-uboot> tftp 1000 hello.ub Gem.e000b000 Waiting for PHY auto negotiation to complete.... done Using Gem.e000b000 device TFTP from server 192.168.0.100; our IP address is 192.168.0.10 Filename 'hello.ub'. Load address: 0x1000 Loading: # 642.6 KiB/s done Bytes transferred = 658 (292 hex) zynq-uboot> bootm ## Booting kernel from Legacy Image at 00001000 ... Image Name: hello Image Type: ARM U-Boot Standalone Program (uncompressed) Data Size: 594 Bytes = 594 Bytes Load Address: 0c100000 Entry Point: 0c100000 Verifying Checksum ... OK Loading Standalone Program ... OK software interrupt pc : [<0000101c>] lr : [<0000101c>] sp : 3fb51d78 ip : 00000000 fp : 00001000 r10: 3ffae998 r9 : 3ffae998 r8 : 3fb51f40 r7 : 00000000 r6 : 00000000 r5 : 3fb527b4 r4 : 3ffae998 r3 : 0000100c r2 : 00000003 r1 : 00000000 r0 : 00000000 Flags: nZCv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
resetting ...
It means there is at least problem on arm even with this patch. Will be great if someone can confirm on any arm platform if this is working or not. Just to be sure that it is not zynq specific problem.
Thanks, Michal
On Tue, Dec 10, 2013 at 06:37:40AM +0100, Michal Simek wrote:
On 12/07/2013 12:26 AM, Simon Glass wrote:
For standalone images, bootm had a special case where the OS boot function was NULL but did actually exist. It was just called manually.
This was removed by commit 35fc84fa which checks for the non-existence of this function before the special case is examined.
There is no obvious reason why standalone is handled with a special case. Adjust the code so that standalone has a normal OS boot function. We still need a special case for when the function returns, but at least we can avoid the main problem.
This is intended to fix the reported:
ERROR: booting os 'U-Boot' (17) is not supportedbut needs testing.
Signed-off-by: Simon Glass sjg@chromium.org
common/cmd_bootm.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index ba73f57..2120aa0 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -77,6 +77,9 @@ static int do_imls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); static void fixup_silent_linux(void); #endif
+static int do_bootm_standalone(int flag, int argc, char * const argv[],
bootm_headers_t *images);static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], bootm_headers_t *images, ulong *os_data, ulong *os_len); @@ -131,6 +134,7 @@ static boot_os_fn do_bootm_integrity; #endif
static boot_os_fn *boot_os[] = {
- [IH_TYPE_STANDALONE] = do_bootm_standalone,
This should be IH_OS_U_BOOT
#ifdef CONFIG_BOOTM_LINUX [IH_OS_LINUX] = do_bootm_linux, #endif @@ -487,17 +491,18 @@ static int bootm_load_os(bootm_headers_t *images, unsigned long *load_end, return 0; }
-static int bootm_start_standalone(int argc, char * const argv[]) +static int do_bootm_standalone(int flag, int argc, char * const argv[],
bootm_headers_t *images){ char *s; int (*appl)(int, char * const []);
/* Don't start if "autostart" is set to "no" */ if (((s = getenv("autostart")) != NULL) && (strcmp(s, "no") == 0)) {
setenv_hex("filesize", images.os.image_len);
setenv_hex("filesize", images->os.image_len);
- appl = (int (*)(int, char * const []))(ulong)ntohl(images.ep);
- appl = (int (*)(int, char * const []))(ulong)ntohl(images->ep); (*appl)(argc, argv); return 0;
} @@ -523,14 +528,12 @@ static cmd_tbl_t cmd_bootm_sub[] = { static int boot_selected_os(int argc, char * const argv[], int state, bootm_headers_t *images, boot_os_fn *boot_fn) {
- if (images->os.type == IH_TYPE_STANDALONE) {
/* This may return when 'autostart' is 'no' */bootm_start_standalone(argc, argv);return 0;- } arch_preboot_os(); boot_fn(state, argc, argv, images);
- if (state == BOOTM_STATE_OS_FAKE_GO) /* We expect to return */
- /* Stand-alone may return when 'autostart' is 'no' */
- if (images->os.type == IH_TYPE_STANDALONE ||
state == BOOTM_STATE_OS_FAKE_GO) /* We expect to return */#ifdef DEBUG
But anyway I have tested this in zynq. Image generation by this command
./tools/mkimage -n "hello" -A arm -O u-boot -T standalone -C none -a c100000 -d examples/standalone/hello_world.bin -v /tftpboot/hello.ub
on 2013.04 is behaviour
U-Boot> bootm ## Booting kernel from Legacy Image at 00001000 ... Image Name: hello Image Type: ARM U-Boot Standalone Program (uncompressed) Data Size: 594 Bytes = 594 Bytes Load Address: 0c100000 Entry Point: 0c100000 Verifying Checksum ... OK Loading Standalone Program ... OK OK Example expects ABI version 6 Actual U-Boot ABI version 6 Hello World argc = 0 data abort
MAYBE you should read doc/README.arm-unaligned-accessespc : [<3ffd8e74>] lr : [<3ffd9574>] sp : 3fbadc20 ip : 00000043 fp : 3ffea2c4 r10: ffffffff r9 : 3fbaddd4 r8 : 3fbadf40 r7 : 0c100232 r6 : ea000014 r5 : ffffffff r4 : 3fbadcaf r3 : ea000014 r2 : ea000014 r1 : ea000013 r0 : ea000014 Flags: nzCv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
resetting ...
on the latest&greatest with fix above
U-Boot 2013.10-00793-gc9ca75a-dirty (Dec 10 2013 - 06:30:03)
I2C: ready Memory: ECC disabled DRAM: 1 GiB MMC: zynq_sdhci: 0 SF: Detected N25Q128A with page size 256 Bytes, erase size 4 KiB, total 16 MiB *** Warning - bad CRC, using default environment
In: serial Out: serial Err: serial Net: Gem.e000b000 Hit any key to stop autoboot: 0 zynq-uboot> set serverip 192.168.0.100 zynq-uboot> set ipaddr 192.168.0.10 zynq-uboot> tftp 1000 hello.ub Gem.e000b000 Waiting for PHY auto negotiation to complete.... done Using Gem.e000b000 device TFTP from server 192.168.0.100; our IP address is 192.168.0.10 Filename 'hello.ub'. Load address: 0x1000 Loading: # 642.6 KiB/s done Bytes transferred = 658 (292 hex) zynq-uboot> bootm ## Booting kernel from Legacy Image at 00001000 ... Image Name: hello Image Type: ARM U-Boot Standalone Program (uncompressed) Data Size: 594 Bytes = 594 Bytes Load Address: 0c100000 Entry Point: 0c100000 Verifying Checksum ... OK Loading Standalone Program ... OK software interrupt pc : [<0000101c>] lr : [<0000101c>] sp : 3fb51d78 ip : 00000000 fp : 00001000 r10: 3ffae998 r9 : 3ffae998 r8 : 3fb51f40 r7 : 00000000 r6 : 00000000 r5 : 3fb527b4 r4 : 3ffae998 r3 : 0000100c r2 : 00000003 r1 : 00000000 r0 : 00000000 Flags: nZCv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
resetting ...
It means there is at least problem on arm even with this patch. Will be great if someone can confirm on any arm platform if this is working or not. Just to be sure that it is not zynq specific problem.
Since patchwork is down I can't send a link to it, but Jeroen posted a patch that fixes examples/standalone/stubs.c to use r9 instead of r8 on lines 43 and 50. I think that's the next thing you need here.
Hi,
On 10 December 2013 07:40, Tom Rini trini@ti.com wrote:
On Tue, Dec 10, 2013 at 06:37:40AM +0100, Michal Simek wrote:
On 12/07/2013 12:26 AM, Simon Glass wrote:
For standalone images, bootm had a special case where the OS boot function was NULL but did actually exist. It was just called manually.
This was removed by commit 35fc84fa which checks for the non-existence of this function before the special case is examined.
There is no obvious reason why standalone is handled with a special case. Adjust the code so that standalone has a normal OS boot function. We still need a special case for when the function returns, but at least we can avoid the main problem.
This is intended to fix the reported:
ERROR: booting os 'U-Boot' (17) is not supportedbut needs testing.
Signed-off-by: Simon Glass sjg@chromium.org
common/cmd_bootm.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index ba73f57..2120aa0 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -77,6 +77,9 @@ static int do_imls(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]); static void fixup_silent_linux(void); #endif
+static int do_bootm_standalone(int flag, int argc, char * const argv[],
bootm_headers_t *images);static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[], bootm_headers_t *images, ulong *os_data, ulong *os_len); @@ -131,6 +134,7 @@ static boot_os_fn do_bootm_integrity; #endif
static boot_os_fn *boot_os[] = {
- [IH_TYPE_STANDALONE] = do_bootm_standalone,
This should be IH_OS_U_BOOT
#ifdef CONFIG_BOOTM_LINUX [IH_OS_LINUX] = do_bootm_linux, #endif @@ -487,17 +491,18 @@ static int bootm_load_os(bootm_headers_t *images, unsigned long *load_end, return 0; }
-static int bootm_start_standalone(int argc, char * const argv[]) +static int do_bootm_standalone(int flag, int argc, char * const argv[],
bootm_headers_t *images){ char *s; int (*appl)(int, char * const []);
/* Don't start if "autostart" is set to "no" */ if (((s = getenv("autostart")) != NULL) && (strcmp(s, "no") == 0)) {
setenv_hex("filesize", images.os.image_len);
setenv_hex("filesize", images->os.image_len); return 0;
- appl = (int (*)(int, char * const []))(ulong)ntohl(images.ep);
- appl = (int (*)(int, char * const []))(ulong)ntohl(images->ep); (*appl)(argc, argv); return 0;
} @@ -523,14 +528,12 @@ static cmd_tbl_t cmd_bootm_sub[] = { static int boot_selected_os(int argc, char * const argv[], int state, bootm_headers_t *images, boot_os_fn *boot_fn) {
- if (images->os.type == IH_TYPE_STANDALONE) {
/* This may return when 'autostart' is 'no' */bootm_start_standalone(argc, argv);return 0;- } arch_preboot_os(); boot_fn(state, argc, argv, images);
- if (state == BOOTM_STATE_OS_FAKE_GO) /* We expect to return */
- /* Stand-alone may return when 'autostart' is 'no' */
- if (images->os.type == IH_TYPE_STANDALONE ||
state == BOOTM_STATE_OS_FAKE_GO) /* We expect to return */ return 0;#ifdef DEBUG
But anyway I have tested this in zynq. Image generation by this command
./tools/mkimage -n "hello" -A arm -O u-boot -T standalone -C none -a c100000 -d examples/standalone/hello_world.bin -v /tftpboot/hello.ub
on 2013.04 is behaviour
U-Boot> bootm ## Booting kernel from Legacy Image at 00001000 ... Image Name: hello Image Type: ARM U-Boot Standalone Program (uncompressed) Data Size: 594 Bytes = 594 Bytes Load Address: 0c100000 Entry Point: 0c100000 Verifying Checksum ... OK Loading Standalone Program ... OK OK Example expects ABI version 6 Actual U-Boot ABI version 6 Hello World argc = 0 data abort
MAYBE you should read doc/README.arm-unaligned-accessespc : [<3ffd8e74>] lr : [<3ffd9574>] sp : 3fbadc20 ip : 00000043 fp : 3ffea2c4 r10: ffffffff r9 : 3fbaddd4 r8 : 3fbadf40 r7 : 0c100232 r6 : ea000014 r5 : ffffffff r4 : 3fbadcaf r3 : ea000014 r2 : ea000014 r1 : ea000013 r0 : ea000014 Flags: nzCv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
resetting ...
on the latest&greatest with fix above
U-Boot 2013.10-00793-gc9ca75a-dirty (Dec 10 2013 - 06:30:03)
I2C: ready Memory: ECC disabled DRAM: 1 GiB MMC: zynq_sdhci: 0 SF: Detected N25Q128A with page size 256 Bytes, erase size 4 KiB, total 16 MiB *** Warning - bad CRC, using default environment
In: serial Out: serial Err: serial Net: Gem.e000b000 Hit any key to stop autoboot: 0 zynq-uboot> set serverip 192.168.0.100 zynq-uboot> set ipaddr 192.168.0.10 zynq-uboot> tftp 1000 hello.ub Gem.e000b000 Waiting for PHY auto negotiation to complete.... done Using Gem.e000b000 device TFTP from server 192.168.0.100; our IP address is 192.168.0.10 Filename 'hello.ub'. Load address: 0x1000 Loading: # 642.6 KiB/s done Bytes transferred = 658 (292 hex) zynq-uboot> bootm ## Booting kernel from Legacy Image at 00001000 ... Image Name: hello Image Type: ARM U-Boot Standalone Program (uncompressed) Data Size: 594 Bytes = 594 Bytes Load Address: 0c100000 Entry Point: 0c100000 Verifying Checksum ... OK Loading Standalone Program ... OK software interrupt pc : [<0000101c>] lr : [<0000101c>] sp : 3fb51d78 ip : 00000000 fp : 00001000 r10: 3ffae998 r9 : 3ffae998 r8 : 3fb51f40 r7 : 00000000 r6 : 00000000 r5 : 3fb527b4 r4 : 3ffae998 r3 : 0000100c r2 : 00000003 r1 : 00000000 r0 : 00000000 Flags: nZCv IRQs off FIQs off Mode SVC_32 Resetting CPU ...
resetting ...
It means there is at least problem on arm even with this patch. Will be great if someone can confirm on any arm platform if this is working or not. Just to be sure that it is not zynq specific problem.
Since patchwork is down I can't send a link to it, but Jeroen posted a patch that fixes examples/standalone/stubs.c to use r9 instead of r8 on lines 43 and 50. I think that's the next thing you need here.
Thanks Tom. That patch is now in the ARM tree, and I have just sent v2 of this patch.
Regards, Simon
participants (3)
-
Michal Simek -
Simon Glass -
Tom Rini