[U-Boot] [PATCH v2] common: Remove invalid endianess conversion
do_bootm_standanlone() calls ntohl(images->ep) which is wrong because endianess conversion has already been done:
do_bootm() -do_bootm_states() +-bootm_find_os() | -images.ep = image_get_ep(); | -uimage_to_cpu(hdr->ih_ep); -boot_selected_os() -do_bootm_standanlone()
Without this conversion the code works correctly at least on ARM9. Addtionally "appl" need not be dereferenced with the "*" operator.
Signed-off-by: Christian Eggers ceggers@gmx.de --- Changes in v2: - Improve description why the patch is required - (appl)(...) --> appl(...)
Further remarks: It seems there's no real difference between doing "(*func_ptr)(args)" and "func_ptr(args)":
--- arm --- kernel_entry = (void (*)(int, int, uint))images->ep; kernel_entry(0, machid, r2) ---/arm --- --- powerpc --- kernel = (void (*)(bd_t *, ulong, ulong, ulong, ulong, ulong, ulong))images->ep; (*kernel)(kbd, initrd_start, initrd_end, cmd_start, cmd_end, 0, 0); ---/powerpc ---
See also: http://stackoverflow.com/questions/7518815/function-pointer-automatic-derefe...
common/cmd_bootm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index a59ee95..c507e1d 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -514,8 +514,8 @@ static int do_bootm_standalone(int flag, int argc, char * const argv[], setenv_hex("filesize", images->os.image_len); return 0; } - appl = (int (*)(int, char * const []))(ulong)ntohl(images->ep); - (*appl)(argc, argv); + appl = (int (*)(int, char * const []))images->ep; + appl(argc, argv); return 0; }
Dear Christian Eggers
Christian Eggers <ceggers <at> gmx.de> writes:
do_bootm_standanlone() calls ntohl(images->ep) which is wrong because endianess conversion has already been done:
do_bootm() -do_bootm_states() +-bootm_find_os() | -images.ep = image_get_ep(); | -uimage_to_cpu(hdr->ih_ep); -boot_selected_os() -do_bootm_standanlone()
Without this conversion the code works correctly at least on ARM9.
What is the endian of your ARM9 testbed?
I've tested the hello_world standalone image with and without your patch on a T5040 (BE). Behaviour of "bootm" does not change. So I am not sure what problem this patch fixes on PowerPC, if any. The patch might be applicable only to certain archs so you should test it on other archs as well (otherwise this patch might break them).
As a reference, the standalone image was created using the commandline: ../../tools/mkimage -A powerpc -O u-boot -T standalone -C none -a 0x10040000 -e 0x10040000 -n test -d hello_world.bin hello_world.img
(Yes, I modified CONFIG_STANDALONE_LOAD_ADDR for this test.) (Yes, hello_world was modified to not return for this test.)
And executed, on the board: tftp 100000 hello_world.img bootm 0x100000
Addtionally "appl" need not be dereferenced with the "*" operator.
-snipped-
common/cmd_bootm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index a59ee95..c507e1d 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c
Also, your patch does not apply on master.
-snipped-
All the best, Rommel
do_bootm_standanlone() calls ntohl(images->ep) which is wrong because endianess conversion has already been done:
do_bootm() -do_bootm_states() +-bootm_find_os() | -images.ep = image_get_ep(); | -uimage_to_cpu(hdr->ih_ep); -boot_selected_os() -do_bootm_standanlone()
Without this conversion the code works correctly at least on AT91SAM9G45. On big endian systems there should be no difference after applying this patch because uimage_to_cpu(x) and ntohl(x) both expand to 'x'.
Signed-off-by: Christian Eggers ceggers@gmx.de --- CC: sessyargc+u-boot@gmail.com
Changes in v3: - refined patch description - rebase to master Changes in v2: - Improve description why the patch is required - (appl)(...) --> appl(...) --- common/cmd_bootm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c index a59ee95..9751edc 100644 --- a/common/cmd_bootm.c +++ b/common/cmd_bootm.c @@ -514,8 +514,8 @@ static int do_bootm_standalone(int flag, int argc, char * const argv[], setenv_hex("filesize", images->os.image_len); return 0; } - appl = (int (*)(int, char * const []))(ulong)ntohl(images->ep); - (*appl)(argc, argv); + appl = (int (*)(int, char * const []))images->ep; + appl(argc, argv); return 0; }
On Sat, Feb 08, 2014 at 07:27:45PM +0100, Christian Eggers wrote:
do_bootm_standanlone() calls ntohl(images->ep) which is wrong because endianess conversion has already been done:
do_bootm() -do_bootm_states() +-bootm_find_os() | -images.ep = image_get_ep(); | -uimage_to_cpu(hdr->ih_ep); -boot_selected_os() -do_bootm_standanlone()
Without this conversion the code works correctly at least on AT91SAM9G45. On big endian systems there should be no difference after applying this patch because uimage_to_cpu(x) and ntohl(x) both expand to 'x'.
Signed-off-by: Christian Eggers ceggers@gmx.de
Applied to u-boot/master, thanks!
participants (3)
-
Christian Eggers -
Rommel G Custodio -
Tom Rini