[U-Boot] [PATCH 2/2] image: Don't detect XIP images as overlapping.

[U-Boot] [PATCH 2/2] image: Don't detect XIP images as overlapping.

[Stephen Warren]

bootm_load_os() detects when it writes the decompressed image over
the top of the compressed image. If this happens, the original image
is corrupted. When the original image is a multi-component legacy image,
or a (potentially multi-component) FIT image, this implies that other
components may be corrupted. In turn, this means that booting is unlikely
to be successful.

However, in the case of no image compresssion coupled with an image with
load address equal to where the image is already located (e.g. an XIP
kernel, or IH_TYPE_KERNEL_ANYLOAD), there has been no copy and hence no
corruption, no matter whether it's a single-component legacy image, a
multi-component legacy image, or a FIT image. In this case, disable the
overlap detection, and allow boot to continue.

Without this change, when booting a single-component legacy image that
contains an IH_TYPE_KERNEL_ANYLOAD, bootm_load_os() would have returned
BOOTM_ERR_OVERLAP, but the caller ignores this, and boot continues and
succeeds. Now, the false error is no longer even returned.

Without this change, when booting a FIT image that contains an
IH_TYPE_KERNEL_ANYLOAD, bootm_load_os() would have returned
BOOTM_ERR_OVERLAP, which would then cause the caller to reset the board.
Now, the false error is no longer returned, and boot succeeds.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 common/cmd_bootm.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 2e9df58..8f3c97f 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -319,6 +319,7 @@ static int bootm_load_os(image_info_t os, ulong *load_end, int boot_progress)
 	ulong image_start = os.image_start;
 	ulong image_len = os.image_len;
 	uint unc_len = CONFIG_SYS_BOOTM_LEN;
+	int no_overlap = 0;
 #if defined(CONFIG_LZMA) || defined(CONFIG_LZO)
 	int ret;
 #endif /* defined(CONFIG_LZMA) || defined(CONFIG_LZO) */
@@ -329,6 +330,7 @@ static int bootm_load_os(image_info_t os, ulong *load_end, int boot_progress)
 	case IH_COMP_NONE:
 		if (load == blob_start || load == image_start) {
 			printf("   XIP %s ... ", type_name);
+			no_overlap = 1;
 		} else {
 			printf("   Loading %s ... ", type_name);
 			memmove_wd((void *)load, (void *)image_start,
@@ -423,7 +425,7 @@ static int bootm_load_os(image_info_t os, ulong *load_end, int boot_progress)
 	if (boot_progress)
 		show_boot_progress(7);
 
-	if ((load < blob_end) && (*load_end > blob_start)) {
+	if (!no_overlap && (load < blob_end) && (*load_end > blob_start)) {
 		debug("images.os.start = 0x%lX, images.os.end = 0x%lx\n",
 			blob_start, blob_end);
 		debug("images.os.load = 0x%lx, load_end = 0x%lx\n", load,
-- 
1.7.0.4

[U-Boot] [PATCH 1/2] image: Implement IH_TYPE_KERNEL_NOLOAD

[Stephen Warren]

The legacy uImage format includes an absolute load and entry-point
address. When bootm operates on a kernel uImage in memory that isn't
loaded at the address in the image's load address, U-Boot will copy
the image to its address in the header.

Some kernel images can actually be loaded and used at any arbitrary
address. An example is an ARM Linux kernel zImage file. To represent
this capability, IH_TYPE_KERNEL_NOLOAD is implemented, which operates
just like IH_TYPE_KERNEL, except that the load address header is
ignored, and U-Boot does not copy the image to its load address, but
rather uses it in-place.

This is useful when sharing a single (uImage-wrapped) zImage across
multiple boards with different memory layouts; in this case, a specific
load address need not be picked when creating the uImage, but instead
is selected by the board-specific U-Boot environment used to load and
boot that image.

v2: Rename from IH_TYPE_KERNEL_ANYLOAD to IH_TYPE_KERNEL_NOLOAD.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
This is a third attempt to support ARM Linux zImage's ability to run from
anywhere in RAM.

This patch actually ended up being much smaller (patch file size) than
previous attempts. Both my previous attempts could probably be rewritten
to adjust the image load address in the same way as this patch and hence
also be just as small; I don't recommend using any of my previous patches
wthout rework because of this.

before this patch:

   text    data     bss     dec     hex filename
 166642    3568  217020  387230   5e89e ./u-boot

with this patch:

   text    data     bss     dec     hex filename
 166761    3568  217020  387349   5e915 ./u-boot

Is this small enough to do without a config option? I certainly hope so,
because ifdefing every one of these tiny code changes would look ugly.

Tested on NVIDIA Tegra Seaboard:

*  Legacy uImage containing a kernel (and appended DT), of type
IH_TYPE_KERNEL.

*  Legacy uImage containing a kernel (and appended DT), of type
IH_TYPE_KERNEL_NOLOAD.

*  Legacy uImage containing a kernel, of type IH_TYPE_KERNEL_NOLOAD,
plus a separate plain FDT (no uImage wrapping).

*  Legacy uImage containing a kernel, of type IH_TYPE_KERNEL_NOLOAD,
plus a uImage-wrapped initrd, plus a separate plain FDT (no uImage
wrapping).

* FIT image containing both kernel of type IH_TYPE_KERNEL_NOLOAD
and FDT.

* FIT image containing just a kernel of type IH_TYPE_KERNEL_NOLOAD,
plus a separate plain FDT (no FIT/legacy uImage wrapping).

 common/cmd_bootm.c    |   10 +++++++++-
 common/image.c        |    1 +
 include/image.h       |    1 +
 tools/default_image.c |    3 ++-
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d301332..1d62c99 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -271,7 +271,13 @@ static int bootm_start(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]
 		return 1;
 	}
 
+	if (images.os.type == IH_TYPE_KERNEL_NOLOAD) {
+		images.os.load = images.os.image_start;
+		images.ep += images.os.load;
+	}
+
 	if (((images.os.type == IH_TYPE_KERNEL) ||
+	     (images.os.type == IH_TYPE_KERNEL_NOLOAD) ||
 	     (images.os.type == IH_TYPE_MULTI)) &&
 	    (images.os.os == IH_OS_LINUX)) {
 		/* find ramdisk */
@@ -795,7 +801,8 @@ static int fit_check_kernel(const void *fit, int os_noffset, int verify)
 	}
 
 	show_boot_progress(106);
-	if (!fit_image_check_type(fit, os_noffset, IH_TYPE_KERNEL)) {
+	if (!fit_image_check_type(fit, os_noffset, IH_TYPE_KERNEL) &&
+	    !fit_image_check_type(fit, os_noffset, IH_TYPE_KERNEL_NOLOAD)) {
 		puts("Not a kernel image\n");
 		show_boot_progress(-106);
 		return 0;
@@ -873,6 +880,7 @@ static void *boot_get_kernel(cmd_tbl_t *cmdtp, int flag, int argc,
 		/* get os_data and os_len */
 		switch (image_get_type(hdr)) {
 		case IH_TYPE_KERNEL:
+		case IH_TYPE_KERNEL_NOLOAD:
 			*os_data = image_get_data(hdr);
 			*os_len = image_get_data_size(hdr);
 			break;
diff --git a/common/image.c b/common/image.c
index 555d9d9..aacae5a 100644
--- a/common/image.c
+++ b/common/image.c
@@ -136,6 +136,7 @@ static const table_entry_t uimage_type[] = {
 	{	IH_TYPE_FIRMWARE,   "firmware",	  "Firmware",		},
 	{	IH_TYPE_FLATDT,     "flat_dt",    "Flat Device Tree",	},
 	{	IH_TYPE_KERNEL,	    "kernel",	  "Kernel Image",	},
+	{	IH_TYPE_KERNEL_NOLOAD, "kernel_noload",  "Kernel Image (no loading done)", },
 	{	IH_TYPE_KWBIMAGE,   "kwbimage",   "Kirkwood Boot Image",},
 	{	IH_TYPE_IMXIMAGE,   "imximage",   "Freescale i.MX Boot Image",},
 	{	IH_TYPE_INVALID,    NULL,	  "Invalid Image",	},
diff --git a/include/image.h b/include/image.h
index c56a18d..f01eb92 100644
--- a/include/image.h
+++ b/include/image.h
@@ -162,6 +162,7 @@
 #define IH_TYPE_UBLIMAGE	11	/* Davinci UBL Image		*/
 #define IH_TYPE_OMAPIMAGE	12	/* TI OMAP Config Header Image	*/
 #define IH_TYPE_AISIMAGE	13	/* TI Davinci AIS Image		*/
+#define IH_TYPE_KERNEL_NOLOAD	14	/* OS Kernel Image, can run from any load address */
 
 /*
  * Compression Types
diff --git a/tools/default_image.c b/tools/default_image.c
index 6ea3b46..e9d0729 100644
--- a/tools/default_image.c
+++ b/tools/default_image.c
@@ -35,7 +35,8 @@ static image_header_t header;
 
 static int image_check_image_types(uint8_t type)
 {
-	if ((type > IH_TYPE_INVALID) && (type < IH_TYPE_FLATDT))
+	if (((type > IH_TYPE_INVALID) && (type < IH_TYPE_FLATDT)) ||
+	    (type == IH_TYPE_KERNEL_NOLOAD))
 		return EXIT_SUCCESS;
 	else
 		return EXIT_FAILURE;
-- 
1.7.0.4

[U-Boot] [PATCH 2/2] image: Don't detect XIP images as overlapping.

[Stephen Warren]

bootm_load_os() detects when it writes the decompressed image over
the top of the compressed image. If this happens, the original image
is corrupted. When the original image is a multi-component legacy image,
or a (potentially multi-component) FIT image, this implies that other
components may be corrupted. In turn, this means that booting is unlikely
to be successful.

However, in the case of no image compresssion coupled with an image with
load address equal to where the image is already located (e.g. an XIP
kernel, or IH_TYPE_KERNEL_ANYLOAD), there has been no copy and hence no
corruption, no matter whether it's a single-component legacy image, a
multi-component legacy image, or a FIT image. In this case, disable the
overlap detection, and allow boot to continue.

Without this change, when booting a single-component legacy image that
contains an IH_TYPE_KERNEL_ANYLOAD, bootm_load_os() would have returned
BOOTM_ERR_OVERLAP, but the caller ignores this, and boot continues and
succeeds. Now, the false error is no longer even returned.

Without this change, when booting a FIT image that contains an
IH_TYPE_KERNEL_ANYLOAD, bootm_load_os() would have returned
BOOTM_ERR_OVERLAP, which would then cause the caller to reset the board.
Now, the false error is no longer returned, and boot succeeds.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 common/cmd_bootm.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index 1d62c99..34a5d33 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -319,6 +319,7 @@ static int bootm_load_os(image_info_t os, ulong *load_end, int boot_progress)
 	ulong image_start = os.image_start;
 	ulong image_len = os.image_len;
 	uint unc_len = CONFIG_SYS_BOOTM_LEN;
+	int no_overlap = 0;
 #if defined(CONFIG_LZMA) || defined(CONFIG_LZO)
 	int ret;
 #endif /* defined(CONFIG_LZMA) || defined(CONFIG_LZO) */
@@ -329,6 +330,7 @@ static int bootm_load_os(image_info_t os, ulong *load_end, int boot_progress)
 	case IH_COMP_NONE:
 		if (load == blob_start || load == image_start) {
 			printf("   XIP %s ... ", type_name);
+			no_overlap = 1;
 		} else {
 			printf("   Loading %s ... ", type_name);
 			memmove_wd((void *)load, (void *)image_start,
@@ -423,7 +425,7 @@ static int bootm_load_os(image_info_t os, ulong *load_end, int boot_progress)
 	if (boot_progress)
 		show_boot_progress(7);
 
-	if ((load < blob_end) && (*load_end > blob_start)) {
+	if (!no_overlap && (load < blob_end) && (*load_end > blob_start)) {
 		debug("images.os.start = 0x%lX, images.os.end = 0x%lx\n",
 			blob_start, blob_end);
 		debug("images.os.load = 0x%lx, load_end = 0x%lx\n", load,
-- 
1.7.0.4

[U-Boot] [PATCH 1/2] image: Implement IH_TYPE_KERNEL_NOLOAD

[Stephen Warren]

Stephen Warren wrote at Thursday, November 10, 2011 1:18 PM:
> The legacy uImage format includes an absolute load and entry-point
> address. When bootm operates on a kernel uImage in memory that isn't
> loaded at the address in the image's load address, U-Boot will copy
> the image to its address in the header.
> 
> Some kernel images can actually be loaded and used at any arbitrary
> address. An example is an ARM Linux kernel zImage file. To represent
> this capability, IH_TYPE_KERNEL_NOLOAD is implemented, which operates
> just like IH_TYPE_KERNEL, except that the load address header is
> ignored, and U-Boot does not copy the image to its load address, but
> rather uses it in-place.
> 
> This is useful when sharing a single (uImage-wrapped) zImage across
> multiple boards with different memory layouts; in this case, a specific
> load address need not be picked when creating the uImage, but instead
> is selected by the board-specific U-Boot environment used to load and
> boot that image.
> 
> v2: Rename from IH_TYPE_KERNEL_ANYLOAD to IH_TYPE_KERNEL_NOLOAD.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Wolfgang,

Does this version of the patch look OK; can it be applied?

Thanks.

-- 
nvpublic

[U-Boot] [PATCH 1/2] image: Implement IH_TYPE_KERNEL_NOLOAD

[Wolfgang Denk]

Dear Stephen Warren,

In message <74CDBE0F657A3D45AFBB94109FB122FF174FDAFF85@HQMAIL01.nvidia.com> you wrote:
>
> Does this version of the patch look OK; can it be applied?

Probably yes.  I just need a ton of time (or some helpful hand pushing
things through the staging repo).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"He only drinks when he gets depressed." "Why does he get depressed?"
"Sometimes it's because he hasn't had a drink."
                                     - Terry Pratchett, _Men at Arms_

[U-Boot] [PATCH 1/2] image: Implement IH_TYPE_KERNEL_NOLOAD

[Stefan Roese]

On Thursday 10 November 2011 21:17:53 Stephen Warren wrote:
> The legacy uImage format includes an absolute load and entry-point
> address. When bootm operates on a kernel uImage in memory that isn't
> loaded at the address in the image's load address, U-Boot will copy
> the image to its address in the header.
> 
> Some kernel images can actually be loaded and used at any arbitrary
> address. An example is an ARM Linux kernel zImage file. To represent
> this capability, IH_TYPE_KERNEL_NOLOAD is implemented, which operates
> just like IH_TYPE_KERNEL, except that the load address header is
> ignored, and U-Boot does not copy the image to its load address, but
> rather uses it in-place.
> 
> This is useful when sharing a single (uImage-wrapped) zImage across
> multiple boards with different memory layouts; in this case, a specific
> load address need not be picked when creating the uImage, but instead
> is selected by the board-specific U-Boot environment used to load and
> boot that image.
> 
> v2: Rename from IH_TYPE_KERNEL_ANYLOAD to IH_TYPE_KERNEL_NOLOAD.

Applied to u-boot-staging/sr at denx.de. Thanks.
 
Best regards,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de

[U-Boot] [PATCH 2/2] image: Don't detect XIP images as overlapping.

[Stefan Roese]

On Thursday 10 November 2011 21:17:54 Stephen Warren wrote:
> bootm_load_os() detects when it writes the decompressed image over
> the top of the compressed image. If this happens, the original image
> is corrupted. When the original image is a multi-component legacy image,
> or a (potentially multi-component) FIT image, this implies that other
> components may be corrupted. In turn, this means that booting is unlikely
> to be successful.
> 
> However, in the case of no image compresssion coupled with an image with
> load address equal to where the image is already located (e.g. an XIP
> kernel, or IH_TYPE_KERNEL_ANYLOAD), there has been no copy and hence no
> corruption, no matter whether it's a single-component legacy image, a
> multi-component legacy image, or a FIT image. In this case, disable the
> overlap detection, and allow boot to continue.
> 
> Without this change, when booting a single-component legacy image that
> contains an IH_TYPE_KERNEL_ANYLOAD, bootm_load_os() would have returned
> BOOTM_ERR_OVERLAP, but the caller ignores this, and boot continues and
> succeeds. Now, the false error is no longer even returned.
> 
> Without this change, when booting a FIT image that contains an
> IH_TYPE_KERNEL_ANYLOAD, bootm_load_os() would have returned
> BOOTM_ERR_OVERLAP, which would then cause the caller to reset the board.
> Now, the false error is no longer returned, and boot succeeds.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Applied (after fixing a small merge conflict) to u-boot-staging/sr at denx.de. 
Thanks.

Best regards,
Stefan

--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-0 Fax: (+49)-8142-66989-80 Email: office at denx.de