[U-Boot] [PATCH 0/3] 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.
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 | 43 +++++++++++++++++++++++++++++++++++++++++++ include/image.h | 1 + 4 files changed, 60 insertions(+), 24 deletions(-)
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.
Signed-off-by: Bryan Wu pengw@nvidia.com --- common/image.c | 43 +++++++++++++++++++++++++++++++++++++++++++ include/image.h | 1 + 2 files changed, 44 insertions(+)
diff --git a/common/image.c b/common/image.c index 11b3cf5..4e2816a 100644 --- a/common/image.c +++ b/common/image.c @@ -643,6 +643,49 @@ int genimg_get_comp_id(const char *name)
#ifndef USE_HOSTCC /** + * genimg_get_kernel_addr - get the real kernel address + * @img_addr: a string might contain real image address + * + * 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) +{ +#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 * diff --git a/include/image.h b/include/image.h index 3e8f78d..ca2fe86 100644 --- a/include/image.h +++ b/include/image.h @@ -424,6 +424,7 @@ enum fit_load_op { #define IMAGE_FORMAT_FIT 0x02 /* new, libfdt based format */ #define IMAGE_FORMAT_ANDROID 0x03 /* Android boot image */
+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);
Hi Bryan,
On 31 July 2014 18:39, Bryan Wu cooloney@gmail.com wrote:
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.
Signed-off-by: Bryan Wu pengw@nvidia.com
common/image.c | 43 +++++++++++++++++++++++++++++++++++++++++++ include/image.h | 1 + 2 files changed, 44 insertions(+)
diff --git a/common/image.c b/common/image.c index 11b3cf5..4e2816a 100644 --- a/common/image.c +++ b/common/image.c @@ -643,6 +643,49 @@ int genimg_get_comp_id(const char *name)
#ifndef USE_HOSTCC /**
- genimg_get_kernel_addr - get the real kernel address
- @img_addr: a string might contain real image address
- 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- */
I know you are being consistent, but actually I think this should be in the header file - you could move some of the other functions comments there too if you like (in a separate patch).
Also you should document the behaviour when @img_addr is NULL.
+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;
I don't think we need the tabs after 'char'.
+#endif
nit: Remove this blank line.
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
diff --git a/include/image.h b/include/image.h index 3e8f78d..ca2fe86 100644 --- a/include/image.h +++ b/include/image.h @@ -424,6 +424,7 @@ enum fit_load_op { #define IMAGE_FORMAT_FIT 0x02 /* new, libfdt based format */ #define IMAGE_FORMAT_ANDROID 0x03 /* Android boot image */
+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); -- 1.9.1
Regards, Simon
On Thu, Jul 31, 2014 at 05:39:58PM -0700, Bryan Wu wrote:
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.
Signed-off-by: Bryan Wu pengw@nvidia.com
After doing the following to fix warnings: diff --git a/common/image.c b/common/image.c index 4e2816a..a2999c0 100644 --- a/common/image.c +++ b/common/image.c @@ -668,13 +668,13 @@ ulong genimg_get_kernel_addr(char * const img_addr) load_addr); #if defined(CONFIG_FIT) } else if (fit_parse_conf(img_addr, load_addr, &kernel_addr, - fit_uname_config)) { + &fit_uname_config)) { debug("* kernel: config '%s' from image at 0x%08lx\n", - *fit_uname_config, kernel_addr); + fit_uname_config, kernel_addr); } else if (fit_parse_subimage(img_addr, load_addr, &kernel_addr, - fit_uname_kernel)) { + &fit_uname_kernel)) { debug("* kernel: subimage '%s' from image at 0x%08lx\n", - *fit_uname_kernel, kernel_addr); + fit_uname_kernel, kernel_addr); #endif } else { kernel_addr = simple_strtoul(img_addr, NULL, 16);
Applied to u-boot/master, thanks!
On Sun, Aug 10, 2014 at 3:21 PM, Tom Rini trini@ti.com wrote:
On Thu, Jul 31, 2014 at 05:39:58PM -0700, Bryan Wu wrote:
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.
Signed-off-by: Bryan Wu pengw@nvidia.com
After doing the following to fix warnings: diff --git a/common/image.c b/common/image.c index 4e2816a..a2999c0 100644 --- a/common/image.c +++ b/common/image.c @@ -668,13 +668,13 @@ ulong genimg_get_kernel_addr(char * const img_addr) load_addr); #if defined(CONFIG_FIT) } else if (fit_parse_conf(img_addr, load_addr, &kernel_addr,
fit_uname_config)) {
&fit_uname_config)) {
My bad, thanks for fixing this.
-Bryan
debug("* kernel: config '%s' from image at 0x%08lx\n",
*fit_uname_config, kernel_addr);
fit_uname_config, kernel_addr); } else if (fit_parse_subimage(img_addr, load_addr, &kernel_addr,
fit_uname_kernel)) {
&fit_uname_kernel)) { debug("* kernel: subimage '%s' from image at 0x%08lx\n",
*fit_uname_kernel, kernel_addr);
fit_uname_kernel, kernel_addr);#endif } else { kernel_addr = simple_strtoul(img_addr, NULL, 16);
Applied to u-boot/master, thanks!
-- Tom
Trying 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().
Signed-off-by: Bryan Wu pengw@nvidia.com --- common/cmd_pxe.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/common/cmd_pxe.c b/common/cmd_pxe.c index ba48692..59b3598 100644 --- a/common/cmd_pxe.c +++ b/common/cmd_pxe.c @@ -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; }
On Thu, Jul 31, 2014 at 05:39:59PM -0700, Bryan Wu wrote:
Trying 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().
Signed-off-by: Bryan Wu pengw@nvidia.com
Applied to u-boot/master, thanks!
Use the new API which is originally taken out from boot_get_kernel of bootm.c
Signed-off-by: Bryan Wu pengw@nvidia.com --- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index 7ec2ed8..aee68cd 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -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 at 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(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,
On 07/31/2014 06:40 PM, Bryan Wu wrote:
Use the new API which is originally taken out from boot_get_kernel of bootm.c
diff --git a/common/bootm.c b/common/bootm.c
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);
I'd be tempted to try and rework patch 1/3 so that it could "return" the fit_uname_* values too. That way, you wouldn't need to call fit_parse_*() a second time here. This probably isn't a big issue though.
The series, Tested-by: Stephen Warren swarren@nvidia.com Reviewed-by: Stephen Warren swarren@nvidia.com
On Fri, Aug 1, 2014 at 12:18 PM, Stephen Warren swarren@wwwdotorg.org wrote:
On 07/31/2014 06:40 PM, Bryan Wu wrote:
Use the new API which is originally taken out from boot_get_kernel of bootm.c
diff --git a/common/bootm.c b/common/bootm.c
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);I'd be tempted to try and rework patch 1/3 so that it could "return" the fit_uname_* values too. That way, you wouldn't need to call fit_parse_*() a second time here. This probably isn't a big issue though.
I actually tried to do that at beginning but it makes a little bit complicated for the caller like pxe code. I believe most of users don't care about those 2 fit uname arguments but only the kernel address. So I think we can make it simpler firstly.
Thanks, -Bryan
Hi Bryan,
On 31 July 2014 18:40, Bryan Wu cooloney@gmail.com wrote:
Use the new API which is originally taken out from boot_get_kernel of bootm.c
Signed-off-by: Bryan Wu pengw@nvidia.com
common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index 7ec2ed8..aee68cd 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -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 at 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(argv[0]);
Do you think it would be clearer to say
img_addr = genimg_get_kernel_addr(arg < 1 ? NULL : argv[0]);
I don't think it matters in practice, just a little nervous about people going off the end of the args.
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,-- 1.9.1
Regards, Simon
On Thu, Jul 31, 2014 at 05:40:00PM -0700, Bryan Wu wrote:
Use the new API which is originally taken out from boot_get_kernel of bootm.c
Signed-off-by: Bryan Wu pengw@nvidia.com Tested-by: Stephen Warren swarren@nvidia.com Reviewed-by: Stephen Warren swarren@nvidia.com
After doing the following to fix warnings: diff --git a/common/bootm.c b/common/bootm.c index aee68cd..76d811c 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -789,9 +789,9 @@ static const void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc, #if defined(CONFIG_FIT) case IMAGE_FORMAT_FIT: if (!fit_parse_conf(argv[0], load_addr, &img_addr, - fit_uname_config)) + &fit_uname_config)) fit_parse_subimage(argv[0], load_addr, &img_addr, - fit_uname_kernel); + &fit_uname_kernel); os_noffset = fit_image_load(images, img_addr, &fit_uname_kernel, &fit_uname_config, IH_ARCH_DEFAULT, IH_TYPE_KERNEL, Applied to u-boot/master, thanks!
participants (4)
-
Bryan Wu -
Simon Glass -
Stephen Warren -
Tom Rini