introduce genimg_get_kernel_addr()
v2 cover introduce genimg_get_kernel_addr()
When trying to fix the error message issue in pxe sysboot, we found it's need a real kernel address for further image format detection.
So I take out such code from boot_get_kernel() of bootm.c and create the new API functin genimg_get_kernel_addr(). Then convert pxe/sysboot and bootm.c to use it.
v2:
- move function description to header file
- fix coding style issue
- use argc < 1 check when calling genimg_get_kernel_addr() in bootm.c
Bryan Wu (3):
image: introduce genimg_get_kernel_addr()
pxe: detect image format before calling bootm/bootz
bootm: use genimg_get_kernel_addr()
common/bootm.c | 25 +++++-------------------- common/cmd_pxe.c | 15 +++++++++++---- common/image.c | 32 ++++++++++++++++++++++++++++++++ include/image.h | 12 ++++++++++++ 4 files changed, 60 insertions(+), 24 deletions(-)
Signature · 1 line
1.9.1
PATCH v2 1/3 image: introduce genimg_get_kernel_addr()
Kernel address is normally stored as a string argument of bootm or bootz. This function is taken out from boot_get_kernel() of bootm.c, which can be reused by others.
--- common/image.c | 32 ++++++++++++++++++++++++++++++++ include/image.h | 12 ++++++++++++ 2 files changed, 44 insertions(+)
common/image.c +32 -0
@@ -642,6 +642,38 @@ int genimg_get_comp_id(const char *name) }#ifndef USE_HOSTCC +ulong genimg_get_kernel_addr (char *const img_addr) +{ +#if defined(CONFIG_FIT ) +const char *fit_uname_config =NULL ; +const char *fit_uname_kernel =NULL ; +#endif +ulong kernel_addr; + +/* find out kernel image address */ +if (!img_addr) { + kernel_addr = load_addr; +debug ("* kernel: default image load address = 0x%08lx\n" , + load_addr); +#if defined(CONFIG_FIT ) + } else if (fit_parse_conf(img_addr, load_addr, &kernel_addr, + fit_uname_config)) { +debug ("* kernel: config '%s' from image at 0x%08lx\n" , + *fit_uname_config, kernel_addr); + }else if (fit_parse_subimage (img_addr, load_addr, &kernel_addr, + fit_uname_kernel)) { +debug ("* kernel: subimage '%s' from image at 0x%08lx\n" , + *fit_uname_kernel, kernel_addr); +#endif + }else { + kernel_addr =simple_strtoul (img_addr,NULL ,16 ); +debug ("* kernel: cmdline image address = 0x%08lx\n" , + kernel_addr); + } + +return kernel_addr; +} +/** * genimg_get_format - get image format type * @img_addr: image start address
include/image.h +12 -0
@@ -424,6 +424,18 @@ enum fit_load_op {#define IMAGE_FORMAT_FIT 0x02/* new, libfdt based format */ #define IMAGE_FORMAT_ANDROID 0x03/* Android boot image */ +/** +* genimg_get_kernel_addr - get the real kernel address +* @img_addr: a string might contain real image address. If img_addr is NULL, +* return kernel_addr as load_addr. +* +* genimg_get_kernel_addr() get the real kernel start address from a string +* which is normally the first argv of bootm/bootz +* +* returns: +* kernel start address +*/ +ulong genimg_get_kernel_addr (char *const img_addr);int genimg_get_format (const void *img_addr);int genimg_has_config (bootm_headers_t *images);ulong genimg_get_image (ulong img_addr);
Signature · 1 line
1.9.1
PATCH v2 1/3 image: introduce genimg_get_kernel_addr()
Quoted 7 lines · Bryan Wu
Kernel address is normally stored as a string argument of bootm or bootz. This function is taken out from
boot_get_kernel()ofbootm.c, which can be reused by others.Signed-off-by Bryan Wu <[email protected]>Tested-by Stephen Warren <[email protected]>Reviewed-by Stephen Warren <[email protected]>
PATCH v2 2/3 pxe: detect image format before calling bootm/bootz
In reply to Bryan WuTrying bootm for zImage will print out several error message which is not necessary for this case. So detect image format firstly, only try bootm for legacy and FIT format image then try bootz for others.
This patch needs new function genimg_get_kernel_addr().
--- common/cmd_pxe.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
common/cmd_pxe.c +11 -4
@@ -1,5 +1,6 @@/* * Copyright 2010-2011 Calxeda, Inc. +* Copyright (c) 2014, NVIDIA CORPORATION. All rights reserved. * * SPDX-License-Identifier: GPL-2.0+ */
@@ -609,6 +610,8 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)char *bootargs;int bootm_argc =3 ;int len =0 ; +ulong kernel_addr; +void *buf;label_print (label);
@@ -771,11 +774,15 @@ static int label_boot(cmd_tbl_t *cmdtp, struct pxe_label *label)if (bootm_argv[3 ]) bootm_argc =4 ; -do_bootm (cmdtp,0 , bootm_argc, bootm_argv); - + kernel_addr =genimg_get_kernel_addr (bootm_argv[1 ]); + buf =map_sysmem (kernel_addr,0 ); +/* Try bootm for legacy and FIT format image */ +if (genimg_get_format (buf) !=IMAGE_FORMAT_INVALID ) +do_bootm (cmdtp,0 , bootm_argc, bootm_argv);#ifdef CONFIG_CMD_BOOTZ -/* Try booting a zImage if do_bootm returns */ -do_bootz (cmdtp,0 , bootm_argc, bootm_argv); +/* Try booting a zImage */ + else + do_bootz(cmdtp, 0, bootm_argc, bootm_argv);#endif return 1 ; }
Signature · 1 line
1.9.1
PATCH v2 3/3 bootm: use genimg_get_kernel_addr()
In reply to Bryan WuUse the new API which is originally taken out from boot_get_kernel of bootm.c
--- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
common/bootm.c +5 -20
@@ -731,26 +731,7 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,int os_noffset;#endif -/* find out kernel image address */ -if (argc <1 ) { - img_addr = load_addr; -debug ("* kernel: default image load address = 0x%08lx\n" , - load_addr); -#if defined(CONFIG_FIT ) - } else if (fit_parse_conf(argv[0], load_addr, &img_addr, - &fit_uname_config)) { -debug ("* kernel: config '%s' from image at 0x%08lx\n" , - fit_uname_config, img_addr); - }else if (fit_parse_subimage (argv[0 ], load_addr, &img_addr, - &fit_uname_kernel)) { -debug ("* kernel: subimage '%s' from image@0x%08lx\n" , - fit_uname_kernel, img_addr); -#endif - }else { - img_addr =simple_strtoul (argv[0 ],NULL ,16 ); -debug ("* kernel: cmdline image address = 0x%08lx\n" , - img_addr); - } + img_addr =genimg_get_kernel_addr (argc <1 ?NULL : argv[0 ]);bootstage_mark (BOOTSTAGE_ID_CHECK_MAGIC );
@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#endif #if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
Signature · 1 line
1.9.1
PATCH v2 3/3 bootm: use genimg_get_kernel_addr()
Quoted 6 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.cSigned-off-by Bryan Wu <[email protected]>Tested-by Stephen Warren <[email protected]>Reviewed-by Stephen Warren <[email protected]>
PATCH v2 3/3 bootm: use genimg_get_kernel_addr()
In reply to Bryan WuQuoted 14 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.cSigned-off-by Bryan Wu <[email protected]>Tested-by Stephen Warren <[email protected]>Reviewed-by Stephen Warren <[email protected]>--- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)common/bootm.c +0
-0
<snip>
Quoted 4 lines
common/bootm.c +0
-0bootstage_mark (BOOTSTAGE_ID_CHECK_MAGIC );
common/bootm.c static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, · line 797
Quoted 10 lines
common/bootm.c +4
-0@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#endif #if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL,
Did anyone test this addition? It breaks my booting. The variable img_addr get changed for calling fit_parse_conf(). If I don't use load_addr variable and run "bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.
Why is this change needed? What does it fix?
York
PATCH v2 3/3 bootm: use genimg_get_kernel_addr()
Quoted 37 lines · York Sun
Quoted 14 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.cSigned-off-by Bryan Wu <[email protected]>Tested-by Stephen Warren <[email protected]>Reviewed-by Stephen Warren <[email protected]>--- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)common/bootm.c +0
-0<snip>
Quoted 4 lines
bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);common/bootm.c static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, · line 797
Quoted 10 lines
common/bootm.c +4
-0@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#endif #if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL,Did anyone test this addition? It breaks my booting. The variable
img_addrget changed for callingfit_parse_conf(). If I don't useload_addrvariable and run "bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.
Quoted 4 lines
Why is this change needed? What does it fix?
York
I think Tom merged one with the fixing:
----
+ if (!fit_parse_conf(argv[0], load_addr, &img_addr,
+ &fit_uname_config))
+ fit_parse_subimage(argv[0], load_addr, &img_addr,
+ &fit_uname_kernel);
----
Can you try that?
-Bryan
PATCH v2 3/3 bootm: use genimg_get_kernel_addr()
Sorry for top posting from my phone.
I tested with the latest code merged, not your original patch.
York
________________________________
From: Bryan Wu
Sent: Thu, 14/08/2014 18:05
To: Sun York-R58495 <[email protected]>
CC: Tom Rini <[email protected]>; Stephen Warren <[email protected]>; u-boot at lists.denx.de; simon Glass <[email protected]>; Basu Arnab-B45036 <[email protected]>
Subject: Re: [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
Quoted 37 lines · York Sun
Quoted 14 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.cSigned-off-by Bryan Wu <[email protected]>Tested-by Stephen Warren <[email protected]>Reviewed-by Stephen Warren <[email protected]>--- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)common/bootm.c +0
-0<snip>
Quoted 4 lines
bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);common/bootm.c static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, · line 797
Quoted 10 lines
common/bootm.c +4
-0@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#endif #if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL,Did anyone test this addition? It breaks my booting. The variable
img_addrget changed for callingfit_parse_conf(). If I don't useload_addrvariable and run "bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.
Quoted 4 lines
Why is this change needed? What does it fix?
York
I think Tom merged one with the fixing:
----
+ if (!fit_parse_conf(argv[0], load_addr, &img_addr,
+ &fit_uname_config))
+ fit_parse_subimage(argv[0], load_addr, &img_addr,
+ &fit_uname_kernel);
----
Can you try that?
-Bryan
PATCH v2 3/3 bootm: use genimg_get_kernel_addr()
In reply to York SunQuoted 20 lines · York Sun
Quoted 14 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.cSigned-off-by Bryan Wu <[email protected]>Tested-by Stephen Warren <[email protected]>Reviewed-by Stephen Warren <[email protected]>--- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)common/bootm.c +0
-0<snip>
Quoted 2 lines
bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
Quoted 16 lines
common/bootm.c static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, · line 797
Quoted 12 lines
common/bootm.c +4
-0@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#endif #if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL,Did anyone test this addition? It breaks my booting. The variable
img_addrget changed for callingfit_parse_conf(). If I don't useload_addrvariable and run "bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.
How exactly are you booting? I tested a fit with bootm addr#conf at 1 or so.
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/20140814/b7f54ff8/attachment.pgp>
PATCH v2 3/3 bootm: use genimg_get_kernel_addr()
Tom,
I tested with "bootm 806f0000". My FIT image is loaded there. I don't have load_addr variable. My default load_addr (from CONFIG macro) is different from this address.
York
________________________________
From: Tom Rini
Sent: Thu, 14/08/2014 18:38
To: Sun York-R58495 <[email protected]>
CC: Bryan Wu <[email protected]>; sjg at chromium.org; swarren at wwwdotorg.org; u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
Quoted 20 lines · York Sun
Quoted 14 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.cSigned-off-by Bryan Wu <[email protected]>Tested-by Stephen Warren <[email protected]>Reviewed-by Stephen Warren <[email protected]>--- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)common/bootm.c +0
-0<snip>
Quoted 2 lines
bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);
Quoted 16 lines
common/bootm.c static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, · line 797
Quoted 12 lines
common/bootm.c +4
-0@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#endif #if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL,Did anyone test this addition? It breaks my booting. The variable
img_addrget changed for callingfit_parse_conf(). If I don't useload_addrvariable and run "bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.
How exactly are you booting? I tested a fit with bootm addr#conf at 1 or so.
--
Tom
PATCH v2 3/3 bootm: use genimg_get_kernel_addr()
Hi York,
Quoted 5 lines · York Sun
Tom,
I tested with "bootm
806f0000". My FIT image is loaded there. I don't haveload_addrvariable. My defaultload_addr(from CONFIG macro) is different from this address.
Yes unfortunately this is completely wrong. I did not catch this in my review unfortunately, perhaps because the & was removed in the code I reviewed.
fit_uname_config and fit_uname_kernel must be passed back from genimg_get_kernel_addr() - that function will need two new parameters.
Regards,
Simon
Quoted 59 lines
York
________________________________
From: Tom Rini
Sent: Thu, 14/08/2014 18:38
To: Sun York-R58495 <[email protected]>
CC: Bryan Wu <[email protected]>; sjg at chromium.org; swarren at wwwdotorg.org; u-boot at lists.denx.deSubject: Re: [U-Boot] [PATCH v2 3/3] bootm: use
genimg_get_kernel_addr()Quoted 32 lines · York Sun
Quoted 14 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.cSigned-off-by Bryan Wu <[email protected]>Tested-by Stephen Warren <[email protected]>Reviewed-by Stephen Warren <[email protected]>--- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)common/bootm.c +0
-0<snip>
Quoted 14 lines
bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);common/bootm.c +4
-0@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#endif #if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config,
IH_ARCH_DEFAULT,IH_TYPE_KERNEL,Quoted 8 lines
Quoted 1 line
Did anyone test this addition? It breaks my booting. The variable
img_addrget
changed for callingfit_parse_conf(). If I don't useload_addrvariable and run
"bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.How exactly are you booting? I tested a fit with bootm addr#conf at 1 or so.
--
Tom
PATCH v2 3/3 bootm: use genimg_get_kernel_addr()
Quoted 9 lines · Simon Glass
Hi York,
Quoted 5 lines · York Sun
Tom,
I tested with "bootm
806f0000". My FIT image is loaded there. I don't haveload_addrvariable. My defaultload_addr(from CONFIG macro) is different from this address.
Quoted 7 lines
Yes unfortunately this is completely wrong. I did not catch this in my review unfortunately, perhaps because the & was removed in the code I reviewed.
fit_uname_configandfit_uname_kernelmust be passed back fromgenimg_get_kernel_addr()- that function will need two new parameters.
I'm working on a fix to add this 2 new parameters.
-Bryan
Quoted 60 lines
Quoted 60 lines
York
________________________________
From: Tom Rini
Sent: Thu, 14/08/2014 18:38
To: Sun York-R58495 <[email protected]>
CC: Bryan Wu <[email protected]>; sjg at chromium.org; swarren at wwwdotorg.org; u-boot at lists.denx.deSubject: Re: [U-Boot] [PATCH v2 3/3] bootm: use
genimg_get_kernel_addr()Quoted 32 lines · York Sun
Quoted 14 lines · Bryan Wu
Use the new API which is originally taken out from
boot_get_kernelofbootm.cSigned-off-by Bryan Wu <[email protected]>Tested-by Stephen Warren <[email protected]>Reviewed-by Stephen Warren <[email protected]>--- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)common/bootm.c +0
-0<snip>
Quoted 14 lines
bootstage_mark(BOOTSTAGE_ID_CHECK_MAGIC);common/bootm.c +4
-0@@ -807,6 +788,10 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,#endif #if defined(CONFIG_FIT )case IMAGE_FORMAT_FIT : +if (!fit_parse_conf (argv[0 ], load_addr, &img_addr, + fit_uname_config)) +fit_parse_subimage (argv[0 ], load_addr, &img_addr, + fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config,
IH_ARCH_DEFAULT,IH_TYPE_KERNEL,Quoted 8 lines
Quoted 1 line
Did anyone test this addition? It breaks my booting. The variable
img_addrget
changed for callingfit_parse_conf(). If I don't useload_addrvariable and run
"bootm <addr>" with a FIT image, this will fail. It took me hours to figure out.How exactly are you booting? I tested a fit with bootm addr#conf at 1 or so.
--
Tom