[U-Boot] [PATCH v2 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.
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(-)
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 Tested-by: Stephen Warren swarren@nvidia.com Reviewed-by: Stephen Warren swarren@nvidia.com --- common/image.c | 32 ++++++++++++++++++++++++++++++++ include/image.h | 12 ++++++++++++ 2 files changed, 44 insertions(+)
diff --git a/common/image.c b/common/image.c index 11b3cf5..36fe487 100644 --- a/common/image.c +++ b/common/image.c @@ -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 diff --git a/include/image.h b/include/image.h index 3e8f78d..049a400 100644 --- a/include/image.h +++ b/include/image.h @@ -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);
On 4 August 2014 18:43, 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 Tested-by: Stephen Warren swarren@nvidia.com Reviewed-by: Stephen Warren swarren@nvidia.com
Reviewed-by: Simon Glass sjg@chromium.org
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 Tested-by: Stephen Warren swarren@nvidia.com Reviewed-by: Stephen Warren swarren@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; }
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 --- common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index 7ec2ed8..621bff2 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(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,
On 4 August 2014 18:43, 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 Tested-by: Stephen Warren swarren@nvidia.com Reviewed-by: Stephen Warren swarren@nvidia.com
Reviewed-by: Simon Glass sjg@chromium.org
On 08/04/2014 05:43 PM, 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
common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index 7ec2ed8..621bff2 100644 --- a/common/bootm.c +++ b/common/bootm.c
<snip>
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);
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
On Thu, Aug 14, 2014 at 6:00 PM, York Sun yorksun@freescale.com wrote:
On 08/04/2014 05:43 PM, 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
common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index 7ec2ed8..621bff2 100644 --- a/common/bootm.c +++ b/common/bootm.c
<snip>
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,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
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
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 yorksun@freescale.com CC: Tom Rini trini@ti.com; Stephen Warren swarren@wwwdotorg.org; u-boot@lists.denx.de; simon Glass sjg@chromium.org; Basu Arnab-B45036 arnab.basu@freescale.com Subject: Re: [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
On Thu, Aug 14, 2014 at 6:00 PM, York Sun yorksun@freescale.com wrote:
On 08/04/2014 05:43 PM, 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
common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index 7ec2ed8..621bff2 100644 --- a/common/bootm.c +++ b/common/bootm.c
<snip>
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,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
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
On Thu, Aug 14, 2014 at 06:00:31PM -0700, York Sun wrote:
On 08/04/2014 05:43 PM, 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
common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index 7ec2ed8..621bff2 100644 --- a/common/bootm.c +++ b/common/bootm.c
<snip>
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);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.
How exactly are you booting? I tested a fit with bootm addr#conf@1 or so.
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 yorksun@freescale.com CC: Bryan Wu cooloney@gmail.com; sjg@chromium.org; swarren@wwwdotorg.org; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
On Thu, Aug 14, 2014 at 06:00:31PM -0700, York Sun wrote:
On 08/04/2014 05:43 PM, 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
common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index 7ec2ed8..621bff2 100644 --- a/common/bootm.c +++ b/common/bootm.c
<snip>
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,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.
How exactly are you booting? I tested a fit with bootm addr#conf@1 or so.
-- Tom
Hi York,
On 14 August 2014 19:42, York Sun yorksun@freescale.com wrote:
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.
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
York
From: Tom Rini Sent: Thu, 14/08/2014 18:38 To: Sun York-R58495 yorksun@freescale.com CC: Bryan Wu cooloney@gmail.com; sjg@chromium.org; swarren@wwwdotorg.org; u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
On Thu, Aug 14, 2014 at 06:00:31PM -0700, York Sun wrote:
On 08/04/2014 05:43 PM, 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
common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index 7ec2ed8..621bff2 100644 --- a/common/bootm.c +++ b/common/bootm.c
<snip>
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,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.
How exactly are you booting? I tested a fit with bootm addr#conf@1 or so.
-- Tom
On Thu, Aug 14, 2014 at 8:48 PM, Simon Glass sjg@chromium.org wrote:
Hi York,
On 14 August 2014 19:42, York Sun yorksun@freescale.com wrote:
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.
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.
I'm working on a fix to add this 2 new parameters.
-Bryan
York
From: Tom Rini Sent: Thu, 14/08/2014 18:38 To: Sun York-R58495 yorksun@freescale.com CC: Bryan Wu cooloney@gmail.com; sjg@chromium.org; swarren@wwwdotorg.org; u-boot@lists.denx.de
Subject: Re: [U-Boot] [PATCH v2 3/3] bootm: use genimg_get_kernel_addr()
On Thu, Aug 14, 2014 at 06:00:31PM -0700, York Sun wrote:
On 08/04/2014 05:43 PM, 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
common/bootm.c | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index 7ec2ed8..621bff2 100644 --- a/common/bootm.c +++ b/common/bootm.c
<snip>
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,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.
How exactly are you booting? I tested a fit with bootm addr#conf@1 or so.
-- Tom
participants (4)
-
Bryan Wu -
Simon Glass -
Tom Rini -
York Sun