[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Stephen Warren]

image_get_ram_disk() and image_get_kernel() perform operations in a
consistent order. Modify image_get_fdt() to do things the same way.
This allows a later change to insert some image header manipulations
into these three functions in a consistent fashion.

v2: New patch

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 common/image.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/common/image.c b/common/image.c
index 555d9d9..b773505 100644
--- a/common/image.c
+++ b/common/image.c
@@ -1131,14 +1131,19 @@ static const image_header_t *image_get_fdt(ulong fdt_addr)
 {
 	const image_header_t *fdt_hdr = (const image_header_t *)fdt_addr;
 
-	image_print_contents(fdt_hdr);
+	if (!image_check_magic(fdt_hdr)) {
+		fdt_error("fdt header bad magic number\n");
+		return NULL;
+	}
 
-	puts("   Verifying Checksum ... ");
 	if (!image_check_hcrc(fdt_hdr)) {
 		fdt_error("fdt header checksum invalid");
 		return NULL;
 	}
 
+	image_print_contents(fdt_hdr);
+
+	puts("   Verifying Checksum ... ");
 	if (!image_check_dcrc(fdt_hdr)) {
 		fdt_error("fdt checksum invalid");
 		return NULL;
-- 
1.7.0.4

[U-Boot] [PATCH v2 2/3] image: Support FDTs already loaded at their load address

[Stephen Warren]

boot_get_fdt() expects a uImage-wrapped FDT to be loaded to a staging
location, and then memmove()s it to the load address specified in the
header. This change enhances boot_get_fdt() to detect when the image has
already been loaded to the correct address, and skip this memmove(). The
detection algorithm was written to match the equivalent for the kernel;
see bootm_load_os()'s IH_COMP_NONE case.

v2: New patch

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

diff --git a/common/image.c b/common/image.c
index b773505..7ce5d33 100644
--- a/common/image.c
+++ b/common/image.c
@@ -1371,7 +1371,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[],
 	const image_header_t *fdt_hdr;
 	ulong		fdt_addr;
 	char		*fdt_blob = NULL;
-	ulong		image_start, image_end;
+	ulong		image_start, image_data, image_end;
 	ulong		load_start, load_end;
 #if defined(CONFIG_FIT)
 	void		*fit_hdr;
@@ -1479,21 +1479,28 @@ int boot_get_fdt(int flag, int argc, char * const argv[],
 			 * make sure we don't overwrite initial image
 			 */
 			image_start = (ulong)fdt_hdr;
+			image_data = (ulong)image_get_data(fdt_hdr);
 			image_end = image_get_image_end(fdt_hdr);
 
 			load_start = image_get_load(fdt_hdr);
 			load_end = load_start + image_get_data_size(fdt_hdr);
 
+			if (load_start == image_start ||
+			    load_start == image_data) {
+				fdt_blob = (char *)image_data;
+				break;
+			}
+
 			if ((load_start < image_end) && (load_end > image_start)) {
 				fdt_error("fdt overwritten");
 				goto error;
 			}
 
 			debug("   Loading FDT from 0x%08lx to 0x%08lx\n",
-					image_get_data(fdt_hdr), load_start);
+					image_data, load_start);
 
 			memmove((void *)load_start,
-					(void *)image_get_data(fdt_hdr),
+					(void *)image_data,
 					image_get_data_size(fdt_hdr));
 
 			fdt_blob = (char *)load_start;
-- 
1.7.0.4

[U-Boot] [PATCH v2 2/3] image: Support FDTs already loaded at their load address

[Wolfgang Denk]

Dear Stephen Warren,

In message <1320164902-24190-2-git-send-email-swarren@nvidia.com> you wrote:
> boot_get_fdt() expects a uImage-wrapped FDT to be loaded to a staging
> location, and then memmove()s it to the load address specified in the
> header. This change enhances boot_get_fdt() to detect when the image has
> already been loaded to the correct address, and skip this memmove(). The
> detection algorithm was written to match the equivalent for the kernel;
> see bootm_load_os()'s IH_COMP_NONE case.
> 
> v2: New patch
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  common/image.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)

Applied, thanks.

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
Death, when unnecessary, is a tragic thing.
	-- Flint, "Requiem for Methuselah", stardate 5843.7

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

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

Some payloads can actually be loaded and used at any arbitrary
address. An example is an ARM Linux kernel zImage file. This
is useful when sharing a single zImage across multiple boards
with different memory layouts, or U-Boot builds with different
${load_addr} since sharing a single absolute load address may
not be possible.

With this config option enabled, an image header may contain a
load address of -1/0xffffffff. This indicates the image can
operate at any load address, and U-Boot will avoid automtically
copying it anywhere. In this case, the entry-point field is
specified relative to the start of the image payload.

disabled:

   text	   data	    bss	    dec	    hex	filename
 166727	   3568	 217024	 387319	  5e8f7	./u-boot

enabled:

   text	   data	    bss	    dec	    hex	filename
 166819	   3568	 217020	 387407	  5e94f	./u-boot

v2: image_get_fdt(): Call image_fixup_load_entry() before
image_print_contents(). Rebased on ToT. Now fully tested on kernel,
initrd, and FDT uImages; previous version was only tested with kernel.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 README             |   23 +++++++++++++++++++++++
 common/cmd_bootm.c |    4 ++++
 common/image.c     |   27 +++++++++++++++++++++++++++
 include/image.h    |    4 ++++
 4 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/README b/README
index c05c40a..8e2728c 100644
--- a/README
+++ b/README
@@ -3263,6 +3263,29 @@ Low Level (hardware related) configuration options:
 		be used if available. These functions may be faster under some
 		conditions but may increase the binary size.
 
+Image-related options:
+---------------------------------------------------
+
+- CONFIG_SYS_RELOCATABLE_IMAGES
+
+		The legacy uImage format includes an absolute load and entry-
+		point address. When presented with a uImage in memory that
+		isn't loaded at the address in the image's load address,
+		U-Boot will relocate the image to its address in the header.
+
+		Some payloads can actually be loaded and used at any arbitrary
+		address. An example is an ARM Linux kernel zImage file. This
+		is useful when sharing a single zImage across multiple boards
+		with different memory layouts, or U-Boot builds with different
+		${load_addr} since sharing a single absolute load address may
+		not be possible.
+
+		With this config option enabled, an image header may contain a
+		load address of -1/0xffffffff. This indicates the image can
+		operate at any load address, and U-Boot will avoid automtically
+		copying it anywhere. In this case, the entry-point field is
+		specified relative to the start of the image payload.
+
 Building the Software:
 ======================
 
diff --git a/common/cmd_bootm.c b/common/cmd_bootm.c
index d301332..9b3bb9f 100644
--- a/common/cmd_bootm.c
+++ b/common/cmd_bootm.c
@@ -737,6 +737,10 @@ static image_header_t *image_get_kernel(ulong img_addr, int verify)
 		return NULL;
 	}
 
+#ifdef CONFIG_SYS_RELOCATABLE_IMAGES
+	image_fixup_load_entry(hdr);
+#endif
+
 	show_boot_progress(3);
 	image_print_contents(hdr);
 
diff --git a/common/image.c b/common/image.c
index 7ce5d33..c12cfc2 100644
--- a/common/image.c
+++ b/common/image.c
@@ -344,6 +344,25 @@ void image_print_contents(const void *ptr)
 	}
 }
 
+#ifdef CONFIG_SYS_RELOCATABLE_IMAGES
+void image_fixup_load_entry(image_header_t *hdr)
+{
+	ulong load;
+	ulong hsize;
+	ulong ep;
+
+	load = image_get_load(hdr);
+	if (load != -1)
+		return;
+
+	load = (ulong)hdr;
+	hsize = image_get_header_size();
+	ep = load + hsize + image_get_ep(hdr);
+
+	image_set_load(hdr, load);
+	image_set_ep(hdr, ep);
+}
+#endif
 
 #ifndef USE_HOSTCC
 /**
@@ -381,6 +400,10 @@ static const image_header_t *image_get_ramdisk(ulong rd_addr, uint8_t arch,
 		return NULL;
 	}
 
+#ifdef CONFIG_SYS_RELOCATABLE_IMAGES
+	image_fixup_load_entry((image_header_t *)rd_hdr);
+#endif
+
 	show_boot_progress(10);
 	image_print_contents(rd_hdr);
 
@@ -1141,6 +1164,10 @@ static const image_header_t *image_get_fdt(ulong fdt_addr)
 		return NULL;
 	}
 
+#ifdef CONFIG_SYS_RELOCATABLE_IMAGES
+	image_fixup_load_entry((image_header_t *)fdt_hdr);
+#endif
+
 	image_print_contents(fdt_hdr);
 
 	puts("   Verifying Checksum ... ");
diff --git a/include/image.h b/include/image.h
index c56a18d..b2c1117 100644
--- a/include/image.h
+++ b/include/image.h
@@ -334,6 +334,10 @@ int genimg_get_format(void *img_addr);
 int genimg_has_config(bootm_headers_t *images);
 ulong genimg_get_image(ulong img_addr);
 
+#ifdef CONFIG_SYS_RELOCATABLE_IMAGES
+void image_fixup_load_entry(image_header_t *hdr);
+#endif
+
 int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
 		uint8_t arch, ulong *rd_start, ulong *rd_end);
 
-- 
1.7.0.4

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Stephen Warren,

In message <1320164902-24190-3-git-send-email-swarren@nvidia.com> you wrote:
> The legacy uImage format includes an absolute load and entry-
> point address. When presented with a uImage in memory that
> isn't loaded at the address in the image's load address,
> U-Boot will relocate the image to its address in the header.
> 
> Some payloads can actually be loaded and used at any arbitrary
> address. An example is an ARM Linux kernel zImage file. This
> is useful when sharing a single zImage across multiple boards
> with different memory layouts, or U-Boot builds with different
> ${load_addr} since sharing a single absolute load address may
> not be possible.
> 
> With this config option enabled, an image header may contain a
> load address of -1/0xffffffff. This indicates the image can
> operate at any load address, and U-Boot will avoid automtically
> copying it anywhere. In this case, the entry-point field is
> specified relative to the start of the image payload.

Please don't invent a new solution.  This has been discussed before,
and the agreement was to introduce a new image format where the load
and entry point addresses are not absolute, but interpreted as offsets
relative to the respectice start of system RAM address.

Your own IH_TYPE_*_REL patches are queued and will be merged soon.

I do not see the need for yet another implementation for the very same
thing, so NAK.

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
How can you tell when sour cream goes bad?

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

[Resending in an attempt to avoid base64 encoding]

On 11/05/2011 04:20 PM, Wolfgang Denk wrote:
> Dear Stephen Warren,
> 
> In message <1320164902-24190-3-git-send-email-swarren@nvidia.com> you wrote:
>> The legacy uImage format includes an absolute load and entry-
>> point address. When presented with a uImage in memory that
>> isn't loaded at the address in the image's load address,
>> U-Boot will relocate the image to its address in the header.
>>
>> Some payloads can actually be loaded and used at any arbitrary
>> address. An example is an ARM Linux kernel zImage file. This
>> is useful when sharing a single zImage across multiple boards
>> with different memory layouts, or U-Boot builds with different
>> ${load_addr} since sharing a single absolute load address may
>> not be possible.
>>
>> With this config option enabled, an image header may contain a
>> load address of -1/0xffffffff. This indicates the image can
>> operate at any load address, and U-Boot will avoid automtically
>> copying it anywhere. In this case, the entry-point field is
>> specified relative to the start of the image payload.
> 
> Please don't invent a new solution.  This has been discussed before,
> and the agreement was to introduce a new image format where the load
> and entry point addresses are not absolute, but interpreted as offsets
> relative to the respectice start of system RAM address.
> 
> Your own IH_TYPE_*_REL patches are queued and will be merged soon.

Oh. I kept pushing and pushing on these and kept meeting resistance. I
had absolutely no idea at all that there was agreement over those
patches; the reviews just stopped happening after you refused to look at
them unless I provided U-Boot size information with every possible
combination of ifdef locations present/removed.

Anyway, I have withdrawn my support for those patches; please don't
apply them. In my opinion, this new solution is far superior because:

a) There's no need to revise mkimage to support this new scheme. Hence,
it can be rolled out with just target-size changes, not host-side tool
changes (well, a host-side script change is needed, but that's probably
far easier than rolling out new mkimage binaries)

b) The implementation of this new scheme is far simpler, and less
invasive to the U-Boot code-base, and hence probably far more maintainable.

c) I've validated that the new scheme handles kernel, initrd, and FDT. I
never got around to testing a separate FDT image with the old patches

> I do not see the need for yet another implementation for the very same
> thing, so NAK.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

On 11/07/2011 09:56 AM, Stephen Warren wrote:
> [Resending in an attempt to avoid base64 encoding]
> 
> On 11/05/2011 04:20 PM, Wolfgang Denk wrote:
>> Dear Stephen Warren,
>>
>> In message <1320164902-24190-3-git-send-email-swarren@nvidia.com> you wrote:
>>> The legacy uImage format includes an absolute load and entry-
>>> point address. When presented with a uImage in memory that
>>> isn't loaded at the address in the image's load address,
>>> U-Boot will relocate the image to its address in the header.
>>>
>>> Some payloads can actually be loaded and used at any arbitrary
>>> address. An example is an ARM Linux kernel zImage file. This
>>> is useful when sharing a single zImage across multiple boards
>>> with different memory layouts, or U-Boot builds with different
>>> ${load_addr} since sharing a single absolute load address may
>>> not be possible.
>>>
>>> With this config option enabled, an image header may contain a
>>> load address of -1/0xffffffff. This indicates the image can
>>> operate at any load address, and U-Boot will avoid automtically
>>> copying it anywhere. In this case, the entry-point field is
>>> specified relative to the start of the image payload.
>>
>> Please don't invent a new solution.  This has been discussed before,
>> and the agreement was to introduce a new image format where the load
>> and entry point addresses are not absolute, but interpreted as offsets
>> relative to the respectice start of system RAM address.
>>
>> Your own IH_TYPE_*_REL patches are queued and will be merged soon.
> 
> Oh. I kept pushing and pushing on these and kept meeting resistance. I
> had absolutely no idea at all that there was agreement over those
> patches; the reviews just stopped happening after you refused to look at
> them unless I provided U-Boot size information with every possible
> combination of ifdef locations present/removed.
> 
> Anyway, I have withdrawn my support for those patches; please don't
> apply them. In my opinion, this new solution is far superior because:
> 
> a) There's no need to revise mkimage to support this new scheme. Hence,
> it can be rolled out with just target-size changes, not host-side tool
> changes (well, a host-side script change is needed, but that's probably
> far easier than rolling out new mkimage binaries)
> 
> b) The implementation of this new scheme is far simpler, and less
> invasive to the U-Boot code-base, and hence probably far more maintainable.
> 
> c) I've validated that the new scheme handles kernel, initrd, and FDT. I
> never got around to testing a separate FDT image with the old patches

Sorry, and I forgot:

d) This new solution is much more flexible. With IH_TYPE_*_REL, you have
to pick some SDRAM-relative address for the uImage load address that's
valid across all SoCs the image will be used on. This is easy enough for
Tegra20 and Tegra30, but I have no idea what the memory layout is for
U-Boot on OMAP, MSM, Exynos, ... I foresee potential difficulty here.
With the new scheme, all you say is "this image works /anywhere/; don't
copy it." Given the way Linux zImage works, I know
this works fine on all those SoCs, and even if it didn't, the U-Boot
scripts for those SoCs could arrange for the uImage to be loaded to a
SoC-specific address that the zImage /would/ work at.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Simon Glass]

Hi Stephen,

On Mon, Nov 7, 2011 at 9:09 AM, Stephen Warren <swarren@nvidia.com> wrote:
> On 11/07/2011 09:56 AM, Stephen Warren wrote:
>> [Resending in an attempt to avoid base64 encoding]
>>
>> On 11/05/2011 04:20 PM, Wolfgang Denk wrote:
>>> Dear Stephen Warren,
>>>
>>> In message <1320164902-24190-3-git-send-email-swarren@nvidia.com> you wrote:
>>>> The legacy uImage format includes an absolute load and entry-
>>>> point address. When presented with a uImage in memory that
>>>> isn't loaded at the address in the image's load address,
>>>> U-Boot will relocate the image to its address in the header.
>>>>
>>>> Some payloads can actually be loaded and used at any arbitrary
>>>> address. An example is an ARM Linux kernel zImage file. This
>>>> is useful when sharing a single zImage across multiple boards
>>>> with different memory layouts, or U-Boot builds with different
>>>> ${load_addr} since sharing a single absolute load address may
>>>> not be possible.
>>>>
>>>> With this config option enabled, an image header may contain a
>>>> load address of -1/0xffffffff. This indicates the image can
>>>> operate at any load address, and U-Boot will avoid automtically
>>>> copying it anywhere. In this case, the entry-point field is
>>>> specified relative to the start of the image payload.
>>>
>>> Please don't invent a new solution. ?This has been discussed before,
>>> and the agreement was to introduce a new image format where the load
>>> and entry point addresses are not absolute, but interpreted as offsets
>>> relative to the respectice start of system RAM address.
>>>
>>> Your own IH_TYPE_*_REL patches are queued and will be merged soon.
>>
>> Oh. I kept pushing and pushing on these and kept meeting resistance. I
>> had absolutely no idea at all that there was agreement over those
>> patches; the reviews just stopped happening after you refused to look at
>> them unless I provided U-Boot size information with every possible
>> combination of ifdef locations present/removed.
>>
>> Anyway, I have withdrawn my support for those patches; please don't
>> apply them. In my opinion, this new solution is far superior because:
>>
>> a) There's no need to revise mkimage to support this new scheme. Hence,
>> it can be rolled out with just target-size changes, not host-side tool
>> changes (well, a host-side script change is needed, but that's probably
>> far easier than rolling out new mkimage binaries)
>>
>> b) The implementation of this new scheme is far simpler, and less
>> invasive to the U-Boot code-base, and hence probably far more maintainable.
>>
>> c) I've validated that the new scheme handles kernel, initrd, and FDT. I
>> never got around to testing a separate FDT image with the old patches
>
> Sorry, and I forgot:
>
> d) This new solution is much more flexible. With IH_TYPE_*_REL, you have
> to pick some SDRAM-relative address for the uImage load address that's
> valid across all SoCs the image will be used on. This is easy enough for
> Tegra20 and Tegra30, but I have no idea what the memory layout is for
> U-Boot on OMAP, MSM, Exynos, ... I foresee potential difficulty here.
> With the new scheme, all you say is "this image works /anywhere/; don't
> copy it." Given the way Linux zImage works, I know
> this works fine on all those SoCs, and even if it didn't, the U-Boot
> scripts for those SoCs could arrange for the uImage to be loaded to a
> SoC-specific address that the zImage /would/ work at.

Fair enough so far as it goes.

But doesn't that mean that we are stuck with zImage and cannot have
U-Boot do the decompression?

Regards,
Simon

>
> --
> nvpublic
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ1vb9DB=UkrS0tG47zrYubc0SvG5vK0whUvN3b7_5uU5Q@mail.gmail.com> you wrote:
> 
> > copy it." Given the way Linux zImage works, I know
> > this works fine on all those SoCs, and even if it didn't, the U-Boot
> > scripts for those SoCs could arrange for the uImage to be loaded to a
> > SoC-specific address that the zImage /would/ work at.
> 
> Fair enough so far as it goes.

But why should U-Boot need to be configured with image-specific
information, when we have well defined ways to store exactly that
needed information in the image itself?  That would be a full step
backward.

> But doesn't that mean that we are stuck with zImage and cannot have
> U-Boot do the decompression?

Indeed - this is just one of the disadvatages and reasons why I reject
that new code.

If I find some time I will eventually re-review the IH_TYPE_*_REL
patches, and most probably apply them.

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
Real programmers can write assembly code in any language.   :-)
                      - Larry Wall in  <8571@jpl-devvax.JPL.NASA.GOV>

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

Simon Glass wrote at Monday, November 07, 2011 12:47 PM:
> On Mon, Nov 7, 2011 at 9:09 AM, Stephen Warren <swarren@nvidia.com> wrote:
> > On 11/07/2011 09:56 AM, Stephen Warren wrote:
> >> [Resending in an attempt to avoid base64 encoding]
> >>
> >> On 11/05/2011 04:20 PM, Wolfgang Denk wrote:
> >>> Dear Stephen Warren,
> >>>
> >>> In message <1320164902-24190-3-git-send-email-swarren@nvidia.com> you wrote:
> >>>> The legacy uImage format includes an absolute load and entry-
> >>>> point address. When presented with a uImage in memory that
> >>>> isn't loaded at the address in the image's load address,
> >>>> U-Boot will relocate the image to its address in the header.
> >>>>
> >>>> Some payloads can actually be loaded and used at any arbitrary
> >>>> address. An example is an ARM Linux kernel zImage file. This
> >>>> is useful when sharing a single zImage across multiple boards
> >>>> with different memory layouts, or U-Boot builds with different
> >>>> ${load_addr} since sharing a single absolute load address may
> >>>> not be possible.
> >>>>
> >>>> With this config option enabled, an image header may contain a
> >>>> load address of -1/0xffffffff. This indicates the image can
> >>>> operate at any load address, and U-Boot will avoid automtically
> >>>> copying it anywhere. In this case, the entry-point field is
> >>>> specified relative to the start of the image payload.
> >>>
> >>> Please don't invent a new solution. ?This has been discussed before,
> >>> and the agreement was to introduce a new image format where the load
> >>> and entry point addresses are not absolute, but interpreted as offsets
> >>> relative to the respectice start of system RAM address.
> >>>
> >>> Your own IH_TYPE_*_REL patches are queued and will be merged soon.
> >>
> >> Oh. I kept pushing and pushing on these and kept meeting resistance. I
> >> had absolutely no idea at all that there was agreement over those
> >> patches; the reviews just stopped happening after you refused to look at
> >> them unless I provided U-Boot size information with every possible
> >> combination of ifdef locations present/removed.
> >>
> >> Anyway, I have withdrawn my support for those patches; please don't
> >> apply them. In my opinion, this new solution is far superior because:
> >>
> >> a) There's no need to revise mkimage to support this new scheme. Hence,
> >> it can be rolled out with just target-size changes, not host-side tool
> >> changes (well, a host-side script change is needed, but that's probably
> >> far easier than rolling out new mkimage binaries)
> >>
> >> b) The implementation of this new scheme is far simpler, and less
> >> invasive to the U-Boot code-base, and hence probably far more maintainable.
> >>
> >> c) I've validated that the new scheme handles kernel, initrd, and FDT. I
> >> never got around to testing a separate FDT image with the old patches
> >
> > Sorry, and I forgot:
> >
> > d) This new solution is much more flexible. With IH_TYPE_*_REL, you have
> > to pick some SDRAM-relative address for the uImage load address that's
> > valid across all SoCs the image will be used on. This is easy enough for
> > Tegra20 and Tegra30, but I have no idea what the memory layout is for
> > U-Boot on OMAP, MSM, Exynos, ... I foresee potential difficulty here.
> > With the new scheme, all you say is "this image works /anywhere/; don't
> > copy it." Given the way Linux zImage works, I know
> > this works fine on all those SoCs, and even if it didn't, the U-Boot
> > scripts for those SoCs could arrange for the uImage to be loaded to a
> > SoC-specific address that the zImage /would/ work at.
> 
> Fair enough so far as it goes.
> 
> But doesn't that mean that we are stuck with zImage and cannot have
> U-Boot do the decompression?

"Stuck with" isn't really a good description.

zImage is a way of booting ARM Linux. There may be others(?), but zImage
is certainly a valid and popular mechanism. I don't see any reason to
push against using it; the only real alternative to my patches is a
straight-up bootz command rather than anything else, and that also uses
zImage.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Stephen Warren,

In message <74CDBE0F657A3D45AFBB94109FB122FF173F9A51AC@HQMAIL01.nvidia.com> you wrote:
>
> "Stuck with" isn't really a good description.

It is, IMO.

> zImage is a way of booting ARM Linux. There may be others(?), but zImage
> is certainly a valid and popular mechanism. I don't see any reason to

Well, and uImage is the way of booting any OS on any architecture in
U-Boot.  It's certainly a valid and popular mechanism.

I think a discussion on such a base is not exactly constructive.


What exactly is wrong with the use of relative (offset) addresses?

And what made you change your mind?


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
"At least they're __________EXPERIENCED incompetents"

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

On 11/07/2011 03:11 PM, Wolfgang Denk wrote:
> Dear Stephen Warren,
> 
> In message <74CDBE0F657A3D45AFBB94109FB122FF173F9A51AC@HQMAIL01.nvidia.com> you wrote:
>>
>> "Stuck with" isn't really a good description.
> 
> It is, IMO.
> 
>> zImage is a way of booting ARM Linux. There may be others(?), but zImage
>> is certainly a valid and popular mechanism. I don't see any reason to
> 
> Well, and uImage is the way of booting any OS on any architecture in
> U-Boot.  It's certainly a valid and popular mechanism.

I agree (well, /a/ way, not /the/ way).

I think the difference here is that I get the impression that people
within the U-Boot community would like to do away with zImage in general
and replace it with uImage, which simply isn't plausible, whereas I'm
perfectly happy for uImage to continue to exist, sometimes as a
container for zImage, potentially a container for something else
entirely, simply depending on what people want to put in there.

> I think a discussion on such a base is not exactly constructive.

But yes, use of zImage in the first place is tangential. People
definitely need/want to use zImage, and I think they should be allowed
to, and I'm just trying to eliminate some pain points when embedding an
ARM zImage within a uImage.

> What exactly is wrong with the use of relative (offset) addresses?
> 
> And what made you change your mind?

I mentioned this before.

The fundamental problem with uImage having an absolute load address is
that there may be no single absolute address that is usable as SDRAM
across all ARM SoCs which may be supported by a single ARM Linux kernel.
This is the basic problem I tried to solve with both my patch sets.

The reason that placing an SDRAM-relative address in the uImage header
doesn't work is fundamentally the same reason, just driven by SW rather
than HW memory maps.

People build U-Boot to load/run at all kinds of different addresses;
even within NVIDIA we're currently using 0x00e08000 for mainline U-Boot,
but would rather use 0x00108000 to fit better with our flashing tools. I
have no idea what random addresses U-Boot is typically built for on
OMAP, MSM, Exynos, or any other SoC. And this is just U-Boot; what about
all the default (or end-user-custom) scripts that U-Boot executes; how
do I know what ${loadaddr} is on every single U-boot installation, for
each of kernel, initrd, FDT?

Given this, I'm no longer totally convinced that it's possible to pick
some location in SDRAM (even specified relative rather absolute) where I
can guarantee the kernel can reside.

Note that this location is something a distro vendor would have to pick
when creating their distro kernel uImage, but end-users would expect
that uImage to work irrespective of how they've hacked their U-Boot
scripts - maybe they've changed their scripts to load the initrd or FDT
to the place location the distro vendor assumed was free for the kernel.
Maybe it isn't reasonable for users to expect this, but avoiding the
need to pick such a location will reduce distro's support costs.

Instead, with a "-1" load address meaning "don't copy the kernel" as in
my most recent patches, all we care about is that the U-Boot scripts
load the kernel somewhere that doesn't overlap with where the initrd or
FDT were loaded (or where U-Boot's code is). The decision point for this
address is whoever writes/hacks the U-Boot scripts for the board. this
is much more decentralized; the distro vendor doesn't have to make this
decision, but rather it'd deferred to whatever the end-user or board
maintainer picks in their U-Boot scripts. Much more flexible, much less
risk of conflict, and any problems should be visible to the person who
caused the problem, not the distro vendor.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Stephen Warren,

In message <4EB85BF3.8030306@nvidia.com> you wrote:
>
> I think the difference here is that I get the impression that people
> within the U-Boot community would like to do away with zImage in general
> and replace it with uImage, which simply isn't plausible, whereas I'm
> perfectly happy for uImage to continue to exist, sometimes as a
> container for zImage, potentially a container for something else
> entirely, simply depending on what people want to put in there.

This is you r interpretation. One could also argue that there are
people out there who know only zImage format and don't bother to ask
why other people might want to use different formats.

And it's not just that they don't care - actually they actively block
any such different approaches.


> The fundamental problem with uImage having an absolute load address is
> that there may be no single absolute address that is usable as SDRAM
> across all ARM SoCs which may be supported by a single ARM Linux kernel.
> This is the basic problem I tried to solve with both my patch sets.

But this is actually a non-issue.  See the 
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/115774/focus=115881

If you wrap a zImage, there is a _guarantee_ that such a single
working address exists, but it is not an absolute address, it is
relative to the start of system RAM.  If we chose an offset of 32 KiB
this will work on all systems.

> The reason that placing an SDRAM-relative address in the uImage header
> doesn't work is fundamentally the same reason, just driven by SW rather
> than HW memory maps.

I'm missing an explanation why you think a "SDRAM-relative address in
the uImage header doesn't work".   Please elucidate.


> People build U-Boot to load/run at all kinds of different addresses;
> even within NVIDIA we're currently using 0x00e08000 for mainline U-Boot,
> but would rather use 0x00108000 to fit better with our flashing tools. I

You mean you don't put U-Boot at the end of physical RAM?  Well, I
guess you must have your reasons for deviating from what is considered
standard in mainline.

> have no idea what random addresses U-Boot is typically built for on
> OMAP, MSM, Exynos, or any other SoC. And this is just U-Boot; what about
> all the default (or end-user-custom) scripts that U-Boot executes; how
> do I know what ${loadaddr} is on every single U-boot installation, for
> each of kernel, initrd, FDT?

I can understand that you don't have such knowledge.  But I don't see
how this is in any way related with the topic we're discussing here?

> Given this, I'm no longer totally convinced that it's possible to pick
> some location in SDRAM (even specified relative rather absolute) where I
> can guarantee the kernel can reside.

It appears ARM is the only architecture to ever have such a problem.
For everybody else this has been working without any problems for
over a decade.

> Note that this location is something a distro vendor would have to pick
> when creating their distro kernel uImage, but end-users would expect
> that uImage to work irrespective of how they've hacked their U-Boot

So what?  See above - we've been doing this for many, many years
already.  This has never been a problem anywhere.  Execpt on ARM.

> scripts - maybe they've changed their scripts to load the initrd or FDT
> to the place location the distro vendor assumed was free for the kernel.
> Maybe it isn't reasonable for users to expect this, but avoiding the
> need to pick such a location will reduce distro's support costs.

We are discussing to introduce a new feature here.  No matter what the
outcome is, the then new interface between U-Boot and Linux should be
properly documented.  As far as I understand (see above) we can even
have some kind of common standard (at least for all the cases that are
willing to accept the limitations of the zImage approach; and even for
those preferring to wrap raw images there would be a semi-standard on
all except of 3 pretty much irrelevant processors).  A distro vendor
would be well advised to just follow that standard, then.  And users
would be able know that there is auch a standard.

Umm... what exactly was your problem?

> Instead, with a "-1" load address meaning "don't copy the kernel" as in
> my most recent patches, all we care about is that the U-Boot scripts
> load the kernel somewhere that doesn't overlap with where the initrd or
> FDT were loaded (or where U-Boot's code is). The decision point for this
> address is whoever writes/hacks the U-Boot scripts for the board. this
> is much more decentralized; the distro vendor doesn't have to make this
> decision, but rather it'd deferred to whatever the end-user or board

I don't understand you.  Above you complain that people might "changed
their scripts" in ways incompatible with the given uImages (and note
that a simple "iminfo" command will actually display the image
properties), and here you cite it as an advantage that they can
"write[s]/hack[s] the U-Boot scripts".

If somebody does that, he needs an understanding of the memory map of
the system, of the images (kernel, initrd, DTB) they are loading, and
(in the case of zImage) about the exact behaviour of the kernel
wrapper when unpacking / relocating the kernel code.

Do you really claim that people with that understanding are not
capable of dealing with relative uImages?  If yes, why would that be
the case?

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
Given a choice between two theories, take the one which is funnier.

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

On 11/07/2011 04:08 PM, Wolfgang Denk wrote:
> In message <4EB85BF3.8030306@nvidia.com> you [Stephen Warren] wrote:
...
>> The fundamental problem with uImage having an absolute load address is
>> that there may be no single absolute address that is usable as SDRAM
>> across all ARM SoCs which may be supported by a single ARM Linux kernel.
>> This is the basic problem I tried to solve with both my patch sets.
> 
> But this is actually a non-issue.  See the 
> http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/115774/focus=115881
>
> If you wrap a zImage, there is a _guarantee_ that such a single
> working address exists, but it is not an absolute address, it is
> relative to the start of system RAM.  If we chose an offset of 32 KiB
> this will work on all systems.

In the thread linked from that link you gave above, there are specific
examples where the value isn't 32K-from-start-of-SDRAM.

Even if that weren't true, putting the zImage there is actually the
absolute worst place it could be.

The uncompressed image needs to end up at 32K-from-start-of-SDRAM (or
whatever SoC-specific value the kernel defines). If U-Boot puts the
zImage at that same location, the first thing the U-Boot decompressor
must do is copy the compressed image somewhere else in SDRAM, so that
when the decompressed kernel is written to 32K (or wherever), it doesn't
overwrite the compressed image it's decompressing.

>> The reason that placing an SDRAM-relative address in the uImage header
>> doesn't work is fundamentally the same reason, just driven by SW rather
>> than HW memory maps.
> 
> I'm missing an explanation why you think a "SDRAM-relative address in
> the uImage header doesn't work".   Please elucidate.

The paragaphs immediately following the one you quoted were that
explanation.

>> People build U-Boot to load/run at all kinds of different addresses;
>> even within NVIDIA we're currently using 0x00e08000 for mainline U-Boot,
>> but would rather use 0x00108000 to fit better with our flashing tools. I
> 
> You mean you don't put U-Boot at the end of physical RAM?  Well, I
> guess you must have your reasons for deviating from what is considered
> standard in mainline.

I keep forgetting that U-Boot typically relocates itself to high memory,
so the location it runs from initially isn't always relevant.

Still, this is an optional feature, so I don't think it's good practice
to rely on it absolutely.

As background, this feature is enabled for Tegra20 in the mainline
U-Boot repository, but not in our internal Tegra30 repositories for
example. I believe we eventually intend to enable this option for
Tegra30 too, but there's some bug that causes the relocation to fail at
the moment.

>> have no idea what random addresses U-Boot is typically built for on
>> OMAP, MSM, Exynos, or any other SoC. And this is just U-Boot; what about
>> all the default (or end-user-custom) scripts that U-Boot executes; how
>> do I know what ${loadaddr} is on every single U-boot installation, for
>> each of kernel, initrd, FDT?
> 
> I can understand that you don't have such knowledge.  But I don't see
> how this is in any way related with the topic we're discussing here?

It's the central point of the discussion!

The issue is: When a distro vendor creates a uImage, can they pick an
address (be it absolute, relative, or whatever) where the bootm command
can copy the zImage to, and be 100% guaranteed that address won't
conflict with U-Boot code, wherever the U-Boot environment loaded the
initrd, or wherever the U-Boot environment loaded the FDT, or ...?

Given that anyone can tweak the U-Boot environment to ext/fat/...load
the initrd and FDT to any address whatsoever, I believe the answer to
this question is no.

And given the answer is no: We need a solution where U-Boot can be told
to just leave the zImage wherever the U-Boot environment happened to
load it, and use it in place. Presumably whoever constructed the
environment has the most detailed knowledge of the HW's and U-Boot
environment's expected memory layout for that particular board.

(I honestly feel like I've repeated myself on this point many times. I'm
not sure how to restate it any differently.)

>> Given this, I'm no longer totally convinced that it's possible to pick
>> some location in SDRAM (even specified relative rather absolute) where I
>> can guarantee the kernel can reside.
> 
> It appears ARM is the only architecture to ever have such a problem.
> For everybody else this has been working without any problems for
> over a decade.
> 
>> Note that this location is something a distro vendor would have to pick
>> when creating their distro kernel uImage, but end-users would expect
>> that uImage to work irrespective of how they've hacked their U-Boot
> 
> So what?  See above - we've been doing this for many, many years
> already.  This has never been a problem anywhere.  Execpt on ARM.

It's quite possible that there's more standardization on other
architectures. I'm not familiar enough with e.g. PPC to know why there
aren't issues there.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Stephen Warren,

In message <4EB87122.3050602@nvidia.com> you wrote:
>
> The uncompressed image needs to end up at 32K-from-start-of-SDRAM (or
> whatever SoC-specific value the kernel defines). If U-Boot puts the
> zImage at that same location, the first thing the U-Boot decompressor
> must do is copy the compressed image somewhere else in SDRAM, so that
> when the decompressed kernel is written to 32K (or wherever), it doesn't
> overwrite the compressed image it's decompressing.

Good.

You confirm my understanding that only the zImages are relocatable,
while the raw images are not.

> > You mean you don't put U-Boot at the end of physical RAM?  Well, I
> > guess you must have your reasons for deviating from what is considered
> > standard in mainline.
> 
> I keep forgetting that U-Boot typically relocates itself to high memory,
> so the location it runs from initially isn't always relevant.
> 
> Still, this is an optional feature, so I don't think it's good practice
> to rely on it absolutely.

No, this is not an optinal feature. That's how it is supposed to
work, and how it actually works on all ARM systems in mainline
U-Boot.

> As background, this feature is enabled for Tegra20 in the mainline
> U-Boot repository, but not in our internal Tegra30 repositories for
> example. I believe we eventually intend to enable this option for
> Tegra30 too, but there's some bug that causes the relocation to fail at
> the moment.

I cannot and will not comment on out-of-tree code.

> It's the central point of the discussion!
> 
> The issue is: When a distro vendor creates a uImage, can they pick an
> address (be it absolute, relative, or whatever) where the bootm command
> can copy the zImage to, and be 100% guaranteed that address won't
> conflict with U-Boot code, wherever the U-Boot environment loaded the
> initrd, or wherever the U-Boot environment loaded the FDT, or ...?

The chaos results from two systems working against each other: U-Boot
is capable of loading linux kernel images in place and start them, but
instead of letting it do what it can, you load a zImage, which
includes it's own wrapper, copy code, gzip code and all that stuff.

Agreed, that may be useful on simple boot loaders. but in U-Boot it is
not only not useful, it creates all the problems you are complaining
about.

If you just wrap a raw kernel image instead into a uImage (like we for
example do on Power), then you have everythign you want: you can load
the uImage at semi-random addresses in RAM, or you can even directly
boot ir from NOR flash.


> (I honestly feel like I've repeated myself on this point many times. I'm
> not sure how to restate it any differently.)

Repeating the stuff does not make it more convincing to me.

> > So what?  See above - we've been doing this for many, many years
> > already.  This has never been a problem anywhere.  Execpt on ARM.
> 
> It's quite possible that there's more standardization on other
> architectures. I'm not familiar enough with e.g. PPC to know why there
> aren't issues there.

On PPC we simple do not use the image wrapper when building uImages.

It's only ARM that insists it must always only zImages, and everthing
else is inherently bad.  Patches to wrap raw kernel images instead
have been submitted a number of times before (I think starting with
2.4.17 or something like that) but have always been rejected.

Drop the wrapper in uImages, and the problem goes away.

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
I am an atheist, thank God!

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Stephen Warren,

In message <74CDBE0F657A3D45AFBB94109FB122FF173F9A5035@HQMAIL01.nvidia.com> you wrote:
>
> > Your own IH_TYPE_*_REL patches are queued and will be merged soon.
> 
> Oh. I kept pushing and pushing on these and kept meeting resistance. I

There was no resistance ever.  There were just the normal review
comments.

> had absolutely no idea at all that there was agreement over those patches;
> the reviews just stopped happening after you refused to look at them

If there are no further complaints that usually menas the stuff is
sitting in the queue waiting to be processed.  Sorry, but my bandwidth
_is_ limited.

> Anyway, I have withdrawn my support for those patches; please don't apply
> them. In my opinion, this new solution is far superior because:

Argh... So we are back at square one.

The problem with this new approach is that Linux kernel images are NOT
freely relocatable.  They do have a fix entry point, even if this is
not an absolute address, but a relative one.  The natural way to
handle this is exactly that:  add support for images with relative
)offset based) load and entry point addresses.

Your new approahc is indeed simpler - actually it is too simplistic,
as you cannot record load address / entry point information any more.
It may work when using the kernel wrapper - but what if you don't want
to do that?

> > I do not see the need for yet another implementation for the very same
> > thing, so NAK.

The NAK remains.  The new code will not go in.

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
A star captain's most solemn oath is that he will give his life, even
his entire crew, rather than violate the Prime Directive.
	-- Kirk, "The Omega Glory", stardate unknown

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Marek Vasut]

> Dear Stephen Warren,
> 
> In message <74CDBE0F657A3D45AFBB94109FB122FF173F9A5035@HQMAIL01.nvidia.com> 
you wrote:
> > > Your own IH_TYPE_*_REL patches are queued and will be merged soon.
> > 
> > Oh. I kept pushing and pushing on these and kept meeting resistance. I
> 
> There was no resistance ever.  There were just the normal review
> comments.
> 
> > had absolutely no idea at all that there was agreement over those
> > patches; the reviews just stopped happening after you refused to look at
> > them
> 
> If there are no further complaints that usually menas the stuff is
> sitting in the queue waiting to be processed.  Sorry, but my bandwidth
> _is_ limited.
> 
> > Anyway, I have withdrawn my support for those patches; please don't apply
> 
> > them. In my opinion, this new solution is far superior because:
> Argh... So we are back at square one.
> 
> The problem with this new approach is that Linux kernel images are NOT
> freely relocatable.  They do have a fix entry point, even if this is
> not an absolute address, but a relative one.  The natural way to
> handle this is exactly that:  add support for images with relative
> )offset based) load and entry point addresses.

You have that runtime patching stuff in linux-arm-kernel now, there should be no 
problem with that anymore actually. So basically I understood there was an 
agreement to make special uImage/fitImage which ... oh doh, here is where I'm 
getting lost. Is it that the kernel will still be copied to address, but a 
relative one to where uImage is loaded -- and the entrypoint will be relative to 
that same address?

> 
> Your new approahc is indeed simpler - actually it is too simplistic,
> as you cannot record load address / entry point information any more.
> It may work when using the kernel wrapper - but what if you don't want
> to do that?
> 
> > > I do not see the need for yet another implementation for the very same
> > > thing, so NAK.
> 
> The NAK remains.  The new code will not go in.
> 
> Best regards,
> 
> Wolfgang Denk

Cheers

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

On 11/07/2011 02:04 PM, Marek Vasut wrote:
...
>> The problem with this new approach is that Linux kernel images are NOT
>> freely relocatable.  They do have a fix entry point, even if this is
>> not an absolute address, but a relative one.  The natural way to
>> handle this is exactly that:  add support for images with relative
>> )offset based) load and entry point addresses.
> 
> You have that runtime patching stuff in linux-arm-kernel now, there should be no 
> problem with that anymore actually. So basically I understood there was an 
> agreement to make special uImage/fitImage which ... oh doh, here is where I'm 
> getting lost. Is it that the kernel will still be copied to address, but a 
> relative one to where uImage is loaded -- and the entrypoint will be relative to 
> that same address?

U-Boot scripts load uImages to some script-defined address.

(At least for kernel images) when running the bootm command, U-Boot will
then copy the kernel image from whatever place the script loaded it to
whatever value the "load address" uImage header field contains.

With my first set of patches, I created IH_TYPE_KERNEL_REL (as a pair to
IH_TYPE_KERNEL) where the load address in the header is not an absolute
address, but rather is interpreted as an offset from "the start of
SDRAM", whatever that is for a particular board. The idea was that while
there could not be a single absolute load address that was valid for all
ARM SoCs, perhaps there could be a single offset from SDRAM that was
valid for all ARM SoCs. With this scheme, U-Boot's bootm command would
still perform the same copy I mentioned above. This applied equally to
"legacy" uImages and FIT images.

With the new set of patches I posted, I didn't add any new uImage
formats, but instead defined a single load address value (0xffffffff) as
meaning "no load address specified", or "load address irrelevant". In
this case, when bootm processes the kernel image, it re-writes the load
address of the image to be equal to wherever the script actually ended
up loading the image. Hence, the kernel image is already in the desired
location, and the copy of the kernel is avoided.

In my opinion, the new scheme is simpler, more correct, more flexible,
more efficient (fewer copies of the kernel data)..., for the reasons I
mentioned a couple emails back.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Marek Vasut]

> On 11/07/2011 02:04 PM, Marek Vasut wrote:
> ...
> 
> >> The problem with this new approach is that Linux kernel images are NOT
> >> freely relocatable.  They do have a fix entry point, even if this is
> >> not an absolute address, but a relative one.  The natural way to
> >> handle this is exactly that:  add support for images with relative
> >> )offset based) load and entry point addresses.
> > 
> > You have that runtime patching stuff in linux-arm-kernel now, there
> > should be no problem with that anymore actually. So basically I
> > understood there was an agreement to make special uImage/fitImage which
> > ... oh doh, here is where I'm getting lost. Is it that the kernel will
> > still be copied to address, but a relative one to where uImage is loaded
> > -- and the entrypoint will be relative to that same address?
> 
> U-Boot scripts load uImages to some script-defined address.
> 
> (At least for kernel images) when running the bootm command, U-Boot will
> then copy the kernel image from whatever place the script loaded it to
> whatever value the "load address" uImage header field contains.
> 
> With my first set of patches, I created IH_TYPE_KERNEL_REL (as a pair to
> IH_TYPE_KERNEL) where the load address in the header is not an absolute
> address, but rather is interpreted as an offset from "the start of
> SDRAM", whatever that is for a particular board. The idea was that while
> there could not be a single absolute load address that was valid for all
> ARM SoCs, perhaps there could be a single offset from SDRAM that was
> valid for all ARM SoCs. With this scheme, U-Boot's bootm command would
> still perform the same copy I mentioned above. This applied equally to
> "legacy" uImages and FIT images.
> 
> With the new set of patches I posted, I didn't add any new uImage
> formats, but instead defined a single load address value (0xffffffff) as
> meaning "no load address specified", or "load address irrelevant". In
> this case, when bootm processes the kernel image, it re-writes the load
> address of the image to be equal to wherever the script actually ended
> up loading the image. Hence, the kernel image is already in the desired
> location, and the copy of the kernel is avoided.

But the kernel ends at offset where uImage was loaded to + few bytes 
(sizeof(uImage header)), right? 
> In my opinion, the new scheme is simpler, more correct, more flexible,
> more efficient (fewer copies of the kernel data)..., for the reasons I
> mentioned a couple emails back.

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

On 11/07/2011 02:59 PM, Marek Vasut wrote:
>> On 11/07/2011 02:04 PM, Marek Vasut wrote:
>> ...
>>
>>>> The problem with this new approach is that Linux kernel images are NOT
>>>> freely relocatable.  They do have a fix entry point, even if this is
>>>> not an absolute address, but a relative one.  The natural way to
>>>> handle this is exactly that:  add support for images with relative
>>>> )offset based) load and entry point addresses.
>>>
>>> You have that runtime patching stuff in linux-arm-kernel now, there
>>> should be no problem with that anymore actually. So basically I
>>> understood there was an agreement to make special uImage/fitImage which
>>> ... oh doh, here is where I'm getting lost. Is it that the kernel will
>>> still be copied to address, but a relative one to where uImage is loaded
>>> -- and the entrypoint will be relative to that same address?
>>
>> U-Boot scripts load uImages to some script-defined address.
>>
>> (At least for kernel images) when running the bootm command, U-Boot will
>> then copy the kernel image from whatever place the script loaded it to
>> whatever value the "load address" uImage header field contains.
>>
>> With my first set of patches, I created IH_TYPE_KERNEL_REL (as a pair to
>> IH_TYPE_KERNEL) where the load address in the header is not an absolute
>> address, but rather is interpreted as an offset from "the start of
>> SDRAM", whatever that is for a particular board. The idea was that while
>> there could not be a single absolute load address that was valid for all
>> ARM SoCs, perhaps there could be a single offset from SDRAM that was
>> valid for all ARM SoCs. With this scheme, U-Boot's bootm command would
>> still perform the same copy I mentioned above. This applied equally to
>> "legacy" uImages and FIT images.
>>
>> With the new set of patches I posted, I didn't add any new uImage
>> formats, but instead defined a single load address value (0xffffffff) as
>> meaning "no load address specified", or "load address irrelevant". In
>> this case, when bootm processes the kernel image, it re-writes the load
>> address of the image to be equal to wherever the script actually ended
>> up loading the image. Hence, the kernel image is already in the desired
>> location, and the copy of the kernel is avoided.
> 
> But the kernel ends at offset where uImage was loaded to + few bytes 
> (sizeof(uImage header)), right? 

I assume you meant "starts" not "ends" right?

Yes, the payload data is actual_load_addr + sizeof(uimage header).

Looking at the implementation of image_fixup_load_entry() in my patches,
I admit this is only correctly accounted for in the "ep" case, not the
"load" case.

That said, the bootm command (see file cmd_bootm.c, function
bootm_load_os(), case IH_COMP_NONE) accepts the image as already being
correctly placed if either the uImage header or the image itself is
already located at location in the uImage's headers' load address field.
I could see an argument that this is in fact a bug.

>> In my opinion, the new scheme is simpler, more correct, more flexible,
>> more efficient (fewer copies of the kernel data)..., for the reasons I
>> mentioned a couple emails back.

-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information.  Any unauthorized review, use, disclosure or distribution
is prohibited.  If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Marek Vasut,

In message <201111072204.41980.marek.vasut@gmail.com> you wrote:
>
> You have that runtime patching stuff in linux-arm-kernel now, there should be no 
> problem with that anymore actually. So basically I understood there was an 
> agreement to make special uImage/fitImage which ... oh doh, here is where I'm 
> getting lost. Is it that the kernel will still be copied to address, but a 
> relative one to where uImage is loaded -- and the entrypoint will be relative to 
> that same address?

Relative to the start of the system RAM.

See
http://thread.gmane.org/gmane.linux.ports.arm.kernel/117512/focus=119281

There are to interesting pieces of information nicely summarized:

1) zImages are are relocatable. They should be loaded and started at
   offsets between 32 KiB and 128 MiB in system RAM.

2) Raw images (without the preloader) have to be started at a fixed
   address, virt_to_phys(PAGE_OFFSET + TEXT_OFFSET), which usually is
   at an offset of 32 KiB in system RAM (with very few exzceptions).

Both sitations can be handled perfectly find with offset addresses in
the images.

When building the images from the kernel Makefiles, we can also make
sure that such architecture specific addresses are correctly set.

This allows to boot such images without additional configuration or
adjustment of load and start addresses in the boot loader.


With Stephen's new approach, we could only use the zImage approach,
and we have to add additional configuration information to the boot
loader.


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
"Free markets select for winning solutions."        - Eric S. Raymond

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

On 11/07/2011 03:27 PM, Wolfgang Denk wrote:
> Dear Marek Vasut,
> 
> In message <201111072204.41980.marek.vasut@gmail.com> you wrote:
>>
>> You have that runtime patching stuff in linux-arm-kernel now, there should be no 
>> problem with that anymore actually. So basically I understood there was an 
>> agreement to make special uImage/fitImage which ... oh doh, here is where I'm 
>> getting lost. Is it that the kernel will still be copied to address, but a 
>> relative one to where uImage is loaded -- and the entrypoint will be relative to 
>> that same address?
> 
> Relative to the start of the system RAM.
> 
> See
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/117512/focus=119281
> 
> There are to interesting pieces of information nicely summarized:
> 
> 1) zImages are are relocatable. They should be loaded and started at
>    offsets between 32 KiB and 128 MiB in system RAM.
> 
> 2) Raw images (without the preloader) have to be started at a fixed
>    address, virt_to_phys(PAGE_OFFSET + TEXT_OFFSET), which usually is
>    at an offset of 32 KiB in system RAM (with very few exzceptions).
> 
> Both sitations can be handled perfectly find with offset addresses in
> the images.
> 
> When building the images from the kernel Makefiles, we can also make
> sure that such architecture specific addresses are correctly set.
> 
> This allows to boot such images without additional configuration or
> adjustment of load and start addresses in the boot loader.
> 
> 
> With Stephen's new approach, we could only use the zImage approach,

You can continue to use raw images; they just may not benefit from the
feature that allows them to be loaded at arbitrary addresses.

But, given your description of them, that isn't an issue, since they
need to be loaded at the specific absolute address they were built for
anyway. And if the description isn't true or becomes untrue later, and
raw images can be loaded at arbitrary locations, the "-1 load address"
technique will end up working just fine for raw images too.

> and we have to add additional configuration information to the boot
> loader.

Sorry, I'm unclear what "additional configuration information" needs to
be added to the boot-loader, and which of cases (1) and (2) that
statement applies to.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Stephen Warren,

In message <4EB85EA6.3000106@nvidia.com> you wrote:
>
> > and we have to add additional configuration information to the boot
> > loader.
> 
> Sorry, I'm unclear what "additional configuration information" needs to
> be added to the boot-loader, and which of cases (1) and (2) that
> statement applies to.

You must define the load address and the entry point address somewhere
in the U-Boot environment or "U-Boot scripts", as you tend to call it.

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
"The greatest warriors are the ones who fight for peace."
- Holly Near

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

On 11/07/2011 04:10 PM, Wolfgang Denk wrote:
> Dear Stephen Warren,
> 
> In message <4EB85EA6.3000106@nvidia.com> you wrote:
>>
>>> and we have to add additional configuration information to the boot
>>> loader.
>>
>> Sorry, I'm unclear what "additional configuration information" needs to
>> be added to the boot-loader, and which of cases (1) and (2) that
>> statement applies to.
> 
> You must define the load address and the entry point address somewhere
> in the U-Boot environment or "U-Boot scripts", as you tend to call it.

The commands in the U-Boot environment need to define some location to
fat/ext/...load the image to either way.

The U-Boot environment never has to define the final address where the
kernel is copied to by bootm. With unmodified ToT U-Boot that absolute
address comes directly from the image's load address field. With my
first patches, that absolute address is calculated from the image's load
address field. With my latest patches, that address is calculated as
wherever the U-Boot environment happened to load the kernel (and hence
there is no copy); this case doesn't require new configuration
information, it just re-uses the existing ${loadaddr} value.

The entry point also always comes from the image header; with similar
absolute/sdram-relative/image-relative derivations depending on patch-set.

I don't see any requirement for new configuration information here.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Nicolas Pitre]

On Mon, 7 Nov 2011, Wolfgang Denk wrote:

> Dear Marek Vasut,
> 
> In message <201111072204.41980.marek.vasut@gmail.com> you wrote:
> >
> > You have that runtime patching stuff in linux-arm-kernel now, there should be no 
> > problem with that anymore actually. So basically I understood there was an 
> > agreement to make special uImage/fitImage which ... oh doh, here is where I'm 
> > getting lost. Is it that the kernel will still be copied to address, but a 
> > relative one to where uImage is loaded -- and the entrypoint will be relative to 
> > that same address?
> 
> Relative to the start of the system RAM.
> 
> See
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/117512/focus=119281
> 
> There are to interesting pieces of information nicely summarized:
> 
> 1) zImages are are relocatable. They should be loaded and started at
>    offsets between 32 KiB and 128 MiB in system RAM.
> 
> 2) Raw images (without the preloader) have to be started at a fixed
>    address, virt_to_phys(PAGE_OFFSET + TEXT_OFFSET), which usually is
>    at an offset of 32 KiB in system RAM (with very few exzceptions).
> 
> Both sitations can be handled perfectly find with offset addresses in
> the images.

No.  Please, we're trying to remove _all_ hardcoded addresses from the 
kernel boot process, absolute or relative.  This is why zImage can be 
loaded at practically any address.  If uImage insists on having a 
relative offset encoded into it, this is slightly better than the 
absolute address but not by much.  Having the ability to use the _same_ 
uImage file and load it at _different_ addresses as specified by an 
argument to the load command is highly desirable.  Pure zImage can do it 
already, so only u-Boot is imposing restrictions here.

> When building the images from the kernel Makefiles, we can also make
> sure that such architecture specific addresses are correctly set.

We don't want any hardcoded architecture specific address anymore.  
This is being removed from the kernel as we speak.  If I cannot use a 
totally generic way to not specify a load address (using -1 for example) 
with mkimage soon, I'll be forced to remove the uImage makefile target 
from Linux as it will simply be broken otherwise.

> This allows to boot such images without additional configuration or
> adjustment of load and start addresses in the boot loader.

This allows to boot such image on only one specific architecture if 
uImage must contain architecture specific addresses, be that absolute or 
relative.  Granted, a relative offset is less problematic than an 
absolute address, but it is a problem nevertheless.  Such architecture 
specific information must live with the boot loader (ideally as a 
script) and not embodied into an image that could otherwise be totally 
generic.

> With Stephen's new approach, we could only use the zImage approach,
> and we have to add additional configuration information to the boot
> loader.

Absolutely!  That is where that configuration information must be.  The 
bootloader is not generic since it must initialize a very specific piece 
of hardware.  It therefore makes sense to associate the per architecture 
details such as the best kernel load address with the bootloader, not in 
the image itself.

OTOH, the kernel must become as generic as possible.  Using DT plays a 
big role in this.  But not having a load address/offset encoded in the 
image is also part of this. Think of what mess a Linux distribution for 
ARM would otherwise have to carry when it must know that configuration 
information for each device it intends to support.  Having a single 
kernel binary for as many ARM devices as possible is what we're aiming 
for, and currently u-Boot is one of the obstacles.


Nicolas

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Nicolas Pitre,

In message <alpine.LFD.2.02.1111071736280.3307@xanadu.home> you wrote:
>
> > 1) zImages are are relocatable. They should be loaded and started at
> >    offsets between 32 KiB and 128 MiB in system RAM.
> > 
> > 2) Raw images (without the preloader) have to be started at a fixed
> >    address, virt_to_phys(PAGE_OFFSET + TEXT_OFFSET), which usually is
> >    at an offset of 32 KiB in system RAM (with very few exzceptions).
> > 
> > Both sitations can be handled perfectly find with offset addresses in
> > the images.
> 
> No.  Please, we're trying to remove _all_ hardcoded addresses from the 
> kernel boot process, absolute or relative.  ...

I understand you are referring here to zImages only. Correct?

Or will raw images (without the preloader) be fully relocatable, too?

>                                        ...  This is why zImage can be 
> loaded at practically any address.  If uImage insists on having a 
> relative offset encoded into it, this is slightly better than the 
> absolute address but not by much.  Having the ability to use the _same_ 
> uImage file and load it at _different_ addresses as specified by an 
> argument to the load command is highly desirable...

Why is it so important to load it at specific (different) addresses
when it can be started from any address?

Maybe this is a key point.  I simply fail to understand this.


> We don't want any hardcoded architecture specific address anymore.  
> This is being removed from the kernel as we speak.  If I cannot use a 

Also for raw images?

> totally generic way to not specify a load address (using -1 for example) 
> with mkimage soon, I'll be forced to remove the uImage makefile target 
> from Linux as it will simply be broken otherwise.

Interesting argument.  Because you are fully flexible and can use any
address you cannot accept a make target that wraps your fully
relocatable image to load it on a specific address.  Now that makes
sense to me.

Or do I smell attempted extortion?


> This allows to boot such image on only one specific architecture if 
> uImage must contain architecture specific addresses, be that absolute or 
> relative.  Granted, a relative offset is less problematic than an 
> absolute address, but it is a problem nevertheless.  Such architecture 

Please explain _why_ you consider it a problem.  Describe use cases
where it doesn't work.

> specific information must live with the boot loader (ideally as a 
> script) and not embodied into an image that could otherwise be totally 
> generic.

Must it?  I don't see the need.  I don't even see the benefit.

> > With Stephen's new approach, we could only use the zImage approach,
> > and we have to add additional configuration information to the boot
> > loader.
> 
> Absolutely!  That is where that configuration information must be.  The 
> bootloader is not generic since it must initialize a very specific piece 
> of hardware.  It therefore makes sense to associate the per architecture 
> details such as the best kernel load address with the bootloader, not in 
> the image itself.

Hm... if that is true, then it means we will have fully relocatable
raw images?

> OTOH, the kernel must become as generic as possible.  Using DT plays a 
> big role in this.  But not having a load address/offset encoded in the 
> image is also part of this. Think of what mess a Linux distribution for 
> ARM would otherwise have to carry when it must know that configuration 
> information for each device it intends to support.  Having a single 
> kernel binary for as many ARM devices as possible is what we're aiming 
> for, and currently u-Boot is one of the obstacles.

Please explain which exact problems you see if Linux on ARM would by
default wrap the zImage into a relative uImage with load address =
entry point address at 32 KiB offset from start of system RAM?


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
Mistakes are often the stepping stones to utter failure.

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

On 11/07/2011 04:25 PM, Wolfgang Denk wrote:
> Dear Nicolas Pitre,
> 
> In message <alpine.LFD.2.02.1111071736280.3307@xanadu.home> you wrote:
>>
>>> 1) zImages are are relocatable. They should be loaded and started at
>>>    offsets between 32 KiB and 128 MiB in system RAM.
>>>
>>> 2) Raw images (without the preloader) have to be started at a fixed
>>>    address, virt_to_phys(PAGE_OFFSET + TEXT_OFFSET), which usually is
>>>    at an offset of 32 KiB in system RAM (with very few exzceptions).
>>>
>>> Both sitations can be handled perfectly find with offset addresses in
>>> the images.
>>
>> No.  Please, we're trying to remove _all_ hardcoded addresses from the 
>> kernel boot process, absolute or relative.  ...
> 
> I understand you are referring here to zImages only. Correct?
> 
> Or will raw images (without the preloader) be fully relocatable, too?
> 
>>                                        ...  This is why zImage can be 
>> loaded at practically any address.  If uImage insists on having a 
>> relative offset encoded into it, this is slightly better than the 
>> absolute address but not by much.  Having the ability to use the _same_ 
>> uImage file and load it at _different_ addresses as specified by an 
>> argument to the load command is highly desirable...
> 
> Why is it so important to load it at specific (different) addresses
> when it can be started from any address?
> 
> Maybe this is a key point.  I simply fail to understand this.

Yes, that's probably the key point.

The central issue is that the memory map on different SoCs is different.
Equally, the U-Boot environment on different SoCs is different. This
means that the uImage /will/ be loaded at different addresses on
different SoCs, and equally nobody can necessarily definea single
address (absolute /or/ relative) where it could then be copied to
without trashing initrd/FDT/...

The only place that has full knowledge of the board's memory layout is
the U-Boot environment for that board, and hence I assert that the
U-Boot environment should define where to load the kernel (and initrd
and FDT), and if U-Boot must copy them, where to copy them to. In
particular, the creator of the uImage can't possibly pick these values
and expect them to work everywhere.

(as an aside, why do CC's keep getting dropped from all these emails?)

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Stephen Warren,

In message <4EB87375.1040100@nvidia.com> you wrote:
>
> The only place that has full knowledge of the board's memory layout is
> the U-Boot environment for that board, and hence I assert that the
> U-Boot environment should define where to load the kernel (and initrd
> and FDT), and if U-Boot must copy them, where to copy them to. In
> particular, the creator of the uImage can't possibly pick these values
> and expect them to work everywhere.

But as you said yourself, the (raw) kernel is not relocatable.  It
gets loaded and started at pre-defined (at image build time)
addresses.  Only the kernel wrapper adds the complexity you are
complaining about.  Drop it, then.

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
The optimum committee has no members.
                                                   - Norman Augustine

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Nicolas Pitre]

On Tue, 8 Nov 2011, Wolfgang Denk wrote:

> Dear Stephen Warren,
> 
> In message <4EB87375.1040100@nvidia.com> you wrote:
> >
> > The only place that has full knowledge of the board's memory layout is
> > the U-Boot environment for that board, and hence I assert that the
> > U-Boot environment should define where to load the kernel (and initrd
> > and FDT), and if U-Boot must copy them, where to copy them to. In
> > particular, the creator of the uImage can't possibly pick these values
> > and expect them to work everywhere.
> 
> But as you said yourself, the (raw) kernel is not relocatable.  It
> gets loaded and started at pre-defined (at image build time)
> addresses.  Only the kernel wrapper adds the complexity you are
> complaining about.  Drop it, then.

Many of us insist on preserving that complexity.  From our point of 
view, it is u-Boot which is too complex and should drop its uImage 
complexity.

But instead of asking you to drop u-Boot's complexity, I'm only asking 
for u-Boot to let both methods to coexist and work.

Given that the zImage "complexity" is not going away because we insist 
on keeping it, either we find a middle ground such as Stephen's patches, 
or we advocate for a different bootloader on ARM.

Do you never happend to compromize once in a while?


Nicolas

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Nicolas Pitre,

In message <alpine.LFD.2.02.1111071942150.3307@xanadu.home> you wrote:
> 
> > But as you said yourself, the (raw) kernel is not relocatable.  It
> > gets loaded and started at pre-defined (at image build time)
> > addresses.  Only the kernel wrapper adds the complexity you are
> > complaining about.  Drop it, then.
> 
> Many of us insist on preserving that complexity.  From our point of 
> view, it is u-Boot which is too complex and should drop its uImage 
> complexity.

Insisting without giving explanations about the reasons is not exactly
a constructive form of a discussion.  Actually it is no discussion at
all.

> But instead of asking you to drop u-Boot's complexity, I'm only asking 
> for u-Boot to let both methods to coexist and work.

Ok, we can make a deal on such a base.  See the suggestion inmy
previous message.

> Given that the zImage "complexity" is not going away because we insist 
> on keeping it, either we find a middle ground such as Stephen's patches, 
> or we advocate for a different bootloader on ARM.
> 
> Do you never happend to compromize once in a while?

In my understanding, compromize is not a one-sided business.

It is actually easy to convince me, but it needs facts and arguments,
not mere insisting on preconceived statements or plain extortion.

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
The shortest unit of time in the multiverse is the News York  Second,
defined  as  the  period  of  time between the traffic lights turning
green and the cab behind you honking.
                                - Terry Pratchett, _Lords and Ladies_

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Marek Vasut]

> Dear Nicolas Pitre,
> 
> In message <alpine.LFD.2.02.1111071942150.3307@xanadu.home> you wrote:
> > > But as you said yourself, the (raw) kernel is not relocatable.  It
> > > gets loaded and started at pre-defined (at image build time)
> > > addresses.  Only the kernel wrapper adds the complexity you are
> > > complaining about.  Drop it, then.
> > 
> > Many of us insist on preserving that complexity.  From our point of
> > view, it is u-Boot which is too complex and should drop its uImage
> > complexity.
> 
> Insisting without giving explanations about the reasons is not exactly
> a constructive form of a discussion.  Actually it is no discussion at
> all.

Ok, so guys ... let me ask a stupid question:

Will it be a problem to extend bootm (if not already done) to load zImages 
directly, with -z option for example ? Won't that satisty both parties -- 
Wolfgang because the "better" solution will still be there (uImage) -- and Nico 
so he can load his "worse" solution (zImage).

And please don't flame me about this "worse" and "better" stuff, you get the 
idea.

M

> 
> > But instead of asking you to drop u-Boot's complexity, I'm only asking
> > for u-Boot to let both methods to coexist and work.
> 
> Ok, we can make a deal on such a base.  See the suggestion inmy
> previous message.
> 
> > Given that the zImage "complexity" is not going away because we insist
> > on keeping it, either we find a middle ground such as Stephen's patches,
> > or we advocate for a different bootloader on ARM.
> > 
> > Do you never happend to compromize once in a while?
> 
> In my understanding, compromize is not a one-sided business.
> 
> It is actually easy to convince me, but it needs facts and arguments,
> not mere insisting on preconceived statements or plain extortion.
> 
> Best regards,
> 
> Wolfgang Denk

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Marek Vasut,

In message <201111081235.05464.marek.vasut@gmail.com> you wrote:
>
> Ok, so guys ... let me ask a stupid question:

Not a stupid question at all.

> Will it be a problem to extend bootm (if not already done) to load zImages 
> directly, with -z option for example ? Won't that satisty both parties -- 

bootm is for uImage format.  I see no sense in "extending" it.

I already suggested to add a new command ("bootz" ?) that could be
used to boot zImage files.

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
Where people stand is not as important as which way they face.
        - Terry Pratchett & Stephen Briggs, _The Discworld Companion_

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Marek Vasut]

> Dear Marek Vasut,
> 
> In message <201111081235.05464.marek.vasut@gmail.com> you wrote:
> > Ok, so guys ... let me ask a stupid question:
> Not a stupid question at all.
> 
> > Will it be a problem to extend bootm (if not already done) to load
> > zImages directly, with -z option for example ? Won't that satisty both
> > parties --
> 
> bootm is for uImage format.  I see no sense in "extending" it.
> 
> I already suggested to add a new command ("bootz" ?) that could be
> used to boot zImage files.

I saw that just now. I'd vote for the "bootz" solution. That way, everyone will 
be happy and this flame would be over.

M

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

(resending due to MIME encoding last time; sorry)

On 11/08/2011 04:50 AM, Wolfgang Denk wrote:
> Dear Marek Vasut,
> 
> In message <201111081235.05464.marek.vasut@gmail.com> you wrote:
>>
>> Ok, so guys ... let me ask a stupid question:
> 
> Not a stupid question at all.
> 
>> Will it be a problem to extend bootm (if not already done) to load zImages 
>> directly, with -z option for example ? Won't that satisty both parties -- 

I originally thought about a bootz command or extending bootm to support
zImages. It looked like coding that up would be significantly more
complex and invasive than extending uImages to support relative or
unspecified load addresses, and hence I naively imagined that such
patches were far more likely to be accepted. It's evident that I was
very wrong in that assessment.

> bootm is for uImage format.  I see no sense in "extending" it.

bootm already supports two completely different formats; legacy uImage
and FIT images. To me, it seems logical to simply add support for a
third image format for the kernel at least. Do you completely disagree
with this? Well, bootm would need to recognize raw (non-uImage-wrapped)
initrd and FDT blobs too, since currently bootm expects everything to be
uImage-wrapped.

> I already suggested to add a new command ("bootz" ?) that could be
> used to boot zImage files.

One potential advantage of extending bootz to recognize zImage directly
would be the re-use of the overall bootm flow and arch functions such as
arch/arm/lib/bootm.c:do_bootm_linux(). I /think/ that creating a new
separate bootz command would require duplicating a lot of code and might
make re-using do_bootm_linux() more complex, although again I'd need to
look at the code in more detail to say for sure.

Are you willing to entertain extending bootm to recognize a third image
format if this makes the patches less invasive, and/or leads to more
maintainable code?

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Stephen Warren,

In message <74CDBE0F657A3D45AFBB94109FB122FF173F9A5424@HQMAIL01.nvidia.com> you wrote:
>
> > bootm is for uImage format.  I see no sense in "extending" it.
> 
> bootm already supports two completely different formats; legacy uImage
> and FIT images. To me, it seems logical to simply add support for a third
> image format for the kernel at least. Do you completely disagree with
> this? Well, bootm would need to recognize raw (non-uImage-wrapped) initrd
> and FDT blobs too, since currently bootm expects everything to be
> uImage-wrapped.

Right, once you start this way you will quickly have a mess.

Yes, bootm supports both uImage and FIT format images, which are
considered "U-Boot native" image formats.  For other formats we use
different commands - there is "bootelf" for ELF files, or there is
bootvx() to boot VxWorks images.

Given the different set of requirements for zImage it makes more sense
to me to provide a separate command for it.  This will also allow for
less #ifdef's for the case you do not want to enable "bootz" support
in the configuration.

> One potential advantage of extending bootz to recognize zImage directly
> would be the re-use of the overall bootm flow and arch functions such as
> arch/arm/lib/bootm.c:do_bootm_linux(). I /think/ that creating a new

I guess this is a typo above, and you mean "extending bootm" ?  Well,
imagine how many #ifdef's would be needed to make this "bootz" support
configurable.

> separate bootz command would require duplicating a lot of code and might
> make re-using do_bootm_linux() more complex, although again I'd need to
> look at the code in more detail to say for sure.

Eventually common parts may be factored out.

> Are you willing to entertain extending bootm to recognize a third image
> format if this makes the patches less invasive, and/or leads to more
> maintainable code?

I have to admit that I don't like the idea, but I will not argue over
hard facts.  But please keep in mind that bootz support shall be a
configuration option, that can be selected or omittet at build time.
My feeling is that this would require quite a number of new #ifdef's
if you try to add it into the existing code.

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
It usually takes more than three weeks to prepare  a  good  impromptu
speech.                                                  - Mark Twain

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Stephen,

In message <20111108194433.7C9A013BE0C5@gemini.denx.de> I wrote:
> 
> > Are you willing to entertain extending bootm to recognize a third image
> > format if this makes the patches less invasive, and/or leads to more
> > maintainable code?
> 
> I have to admit that I don't like the idea, but I will not argue over
> hard facts.  But please keep in mind that bootz support shall be a
> configuration option, that can be selected or omittet at build time.
> My feeling is that this would require quite a number of new #ifdef's
> if you try to add it into the existing code.

Thinking about this again I wonder if this is really what is needed.
Why do we need a third image format?

What would happen if we just create a new image type IH_TYPE_ZIMAGE?

Then you can use plain "bootm", and probably most of the existing
code. A few tests may need to be extended by some "|| (type ==
IH_TYPE_ZIMAGE)", and in the end you can just skip the parts that deal
with loading and starting.

For the sake of consistency, such images should be built with
IH_COMP_NONE, then.


What do you think?

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
"There are three principal ways to lose money: wine, women,  and  en-
gineers.  While  the first two are more pleasant, the third is by far
the more certain."                      -- Baron Rothschild, ca. 1800

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

On 11/08/2011 02:17 PM, Wolfgang Denk wrote:
> Dear Stephen,
> 
> In message <20111108194433.7C9A013BE0C5@gemini.denx.de> I wrote:
>>
>>> Are you willing to entertain extending bootm to recognize a third image
>>> format if this makes the patches less invasive, and/or leads to more
>>> maintainable code?
>>
>> I have to admit that I don't like the idea, but I will not argue over
>> hard facts.  But please keep in mind that bootz support shall be a
>> configuration option, that can be selected or omittet at build time.
>> My feeling is that this would require quite a number of new #ifdef's
>> if you try to add it into the existing code.
> 
> Thinking about this again I wonder if this is really what is needed.
> Why do we need a third image format?
>
> What would happen if we just create a new image type IH_TYPE_ZIMAGE?

That would cover the kernel uImage case. We'd also need a new image type
for "use in place" FDTs, since that also gets relocated to the image
load address before use. I think initrds don't for some reason.

The advantage of using a -1 load address was that it avoided a
proliferation of new IH_TYPE_* values, and an associated requirement for
everyone to update their mkimage tool.

Another advantage of directly recognizing non-uImage-wrapper zImage,
initrd, and FDT (in either bootm or bootz), is that distros wouldn't
have to run mkimage ever; they could just dump the kernel in e.g.
/boot/zImage (plus say /boot/initrd.img and /boot/fdt.bin), and expect
U-Boot to pick it up and use it without any further processing. This
would isolate more of the distro kernel install scripts from U-Boot, and
keep them generic.

...
> For the sake of consistency, such images should be built with
> IH_COMP_NONE, then.

Certainly.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Stephen Warren,

In message <4EB9ACDF.90702@nvidia.com> you wrote:
>
> > What would happen if we just create a new image type IH_TYPE_ZIMAGE?
> 
> That would cover the kernel uImage case. We'd also need a new image type
> for "use in place" FDTs, since that also gets relocated to the image
> load address before use. I think initrds don't for some reason.

Did you check how PPC handles this?  We usually load the DTB as a
separate file (the plain DTB, not any image) and just pass it's
address to the bootm command.  U-Boot then knows how to handle the DTB
on a specific board.

Why would an image type for DTBs be needed?

> Another advantage of directly recognizing non-uImage-wrapper zImage,
> initrd, and FDT (in either bootm or bootz), is that distros wouldn't
> have to run mkimage ever; they could just dump the kernel in e.g.
> /boot/zImage (plus say /boot/initrd.img and /boot/fdt.bin), and expect
> U-Boot to pick it up and use it without any further processing. This
> would isolate more of the distro kernel install scripts from U-Boot, and
> keep them generic.

I have no real preference for one implementation versus the other. My
gut feeling is that bootm + IH_TYPE_ZIMAGE is much more straightfor-
ward, leaner and easier to implement. But it will add the 64 byte
uImage header which might be a nuisance in some cases. On the other
hand, bootz as to implement all the existing code again for handling
ramdisk/initrd and DTB, basicly identically. And you miss features
you might find useful in some cases - like that uImages have a name,
a timestamp, or a checksum, so you can identify and verify what has
been loaded.

I would go for IH_TYPE_ZIMAGE, but I'm not going to implement it.

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
Men will always be men -- no matter where they are.
	-- Harry Mudd, "Mudd's Women", stardate 1329.8

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Nicolas Pitre]

On Tue, 8 Nov 2011, Wolfgang Denk wrote:

> Dear Nicolas Pitre,
> 
> In message <alpine.LFD.2.02.1111071736280.3307@xanadu.home> you wrote:
> >
> > > 1) zImages are are relocatable. They should be loaded and started at
> > >    offsets between 32 KiB and 128 MiB in system RAM.
> > > 
> > > 2) Raw images (without the preloader) have to be started at a fixed
> > >    address, virt_to_phys(PAGE_OFFSET + TEXT_OFFSET), which usually is
> > >    at an offset of 32 KiB in system RAM (with very few exzceptions).
> > > 
> > > Both sitations can be handled perfectly find with offset addresses in
> > > the images.
> > 
> > No.  Please, we're trying to remove _all_ hardcoded addresses from the 
> > kernel boot process, absolute or relative.  ...
> 
> I understand you are referring here to zImages only. Correct?

Correct.  Anything else is not relocatable.

> Or will raw images (without the preloader) be fully relocatable, too?

No.

> >                                        ...  This is why zImage can be 
> > loaded at practically any address.  If uImage insists on having a 
> > relative offset encoded into it, this is slightly better than the 
> > absolute address but not by much.  Having the ability to use the _same_ 
> > uImage file and load it at _different_ addresses as specified by an 
> > argument to the load command is highly desirable...
> 
> Why is it so important to load it at specific (different) addresses
> when it can be started from any address?

The kernel code can be started from any address.  We want the _code_ to 
be that way.

However a particular board may or may not load the kernel at any 
address.  This is a machine specific restriction, not a kernel 
restriction.

> Maybe this is a key point.  I simply fail to understand this.

Let me repeat again.  We want one single kernel image binary that ARM 
distributions can use for all their target machines.  It is therefore 
necessary that uImage be free of any hardcoded load address (absolute or 
relative).  If a particular board require a particular load address for 
the kernel, this must be encoded in its own u-Boot environment and not 
in the distributed uImage.  Failing that, uImage simply cannot be used 
as a distribution format for the kernel because any address/offset 
enforced by the uImage format is going to be incompatible with the needs 
of a particular machine somewhere.

> > We don't want any hardcoded architecture specific address anymore.  
> > This is being removed from the kernel as we speak.  If I cannot use a 
> 
> Also for raw images?

No.  The requirements on raw images are unchanged.  you can use them if 
you wish, but generic ARM distributions can't use that if they want to 
target more than one SOC.  Therefore raw images are not interesting by 
the use case at hand.

> > totally generic way to not specify a load address (using -1 for example) 
> > with mkimage soon, I'll be forced to remove the uImage makefile target 
> > from Linux as it will simply be broken otherwise.
> 
> Interesting argument.  Because you are fully flexible and can use any
> address you cannot accept a make target that wraps your fully
> relocatable image to load it on a specific address.  Now that makes
> sense to me.
> 
> Or do I smell attempted extortion?

What do you not understand in the fact that such a specific address 
makes the resulting image not universal?  I'm telling you that I need to 
produce a kernel image that doesn't carry with it any machine specific 
load address so that same image can be installed unmodified on any 
machine.

Instead, you insist on making that image less useful by attaching to it 
some restrictions on its load address at build time rather than applying 
those restrictions only at load time and only on machines where that 
matters.

For the last time, we don't want any address encoded in the kernel 
image for the simple fact that we don't know at build time what machine 
the kernel will be used on, and therefore what address to use.  This is 
why zImage was made totally relocatable and totally position 
independent so it can figure out at run time what address to use.  
Figuring out the address to use at "make uImage" time only works for a 
kernel that will boot on a single specific machine.

> > This allows to boot such image on only one specific architecture if 
> > uImage must contain architecture specific addresses, be that absolute or 
> > relative.  Granted, a relative offset is less problematic than an 
> > absolute address, but it is a problem nevertheless.  Such architecture 
> 
> Please explain _why_ you consider it a problem.  Describe use cases
> where it doesn't work.

That has been explained a few times already.

> > specific information must live with the boot loader (ideally as a 
> > script) and not embodied into an image that could otherwise be totally 
> > generic.
> 
> Must it?  I don't see the need.  I don't even see the benefit.

Why do I care having this conversation with you then?  Please tell me.

> > > With Stephen's new approach, we could only use the zImage approach,
> > > and we have to add additional configuration information to the boot
> > > loader.
> > 
> > Absolutely!  That is where that configuration information must be.  The 
> > bootloader is not generic since it must initialize a very specific piece 
> > of hardware.  It therefore makes sense to associate the per architecture 
> > details such as the best kernel load address with the bootloader, not in 
> > the image itself.
> 
> Hm... if that is true, then it means we will have fully relocatable
> raw images?

It is already the case, more or less.  The raw image _must_ be loaded at 
TEXT_OFFSET from start of RAM, regardless of where that RAM is.  So if 
all you care about is raw image kernels, then having a relative a load 
address for uImage makes perfect sense.

The role of the zImage code is to figure out at run time about its 
position in RAM, deduce where the RAM starts, relocate itself if it is 
in the way of the decompressed kernel, and decompress the actual kernel 
in its final location (RAM start + TEXT_OFFSET) without having to 
relocate the bigger decompressed data.  Unless it is in the way of the 
decompressed kernel, this code can be executed in place and doesn't need 
to be relocated to any particular address.

> > OTOH, the kernel must become as generic as possible.  Using DT plays a 
> > big role in this.  But not having a load address/offset encoded in the 
> > image is also part of this. Think of what mess a Linux distribution for 
> > ARM would otherwise have to carry when it must know that configuration 
> > information for each device it intends to support.  Having a single 
> > kernel binary for as many ARM devices as possible is what we're aiming 
> > for, and currently u-Boot is one of the obstacles.
> 
> Please explain which exact problems you see if Linux on ARM would by
> default wrap the zImage into a relative uImage with load address =
> entry point address at 32 KiB offset from start of system RAM?

It will always have to relocate itself away from there as this is 
typically where the decompressed kernel must be placed.

Knowing that, most people are already attempting to load their uImage 
elsewhere, say at an offset of 16Mb FROM START of RAM.  Or maybe they 
have a ramdisk at that address so they load the kernel at an offset of 
32MB.  Or any such restriction that is particular to a specific 
machine/setup.

But u-Boot will then ignore the wish of the user and relocate the image 
content to 32KB after start of RAM, causing one copy.  Now zImage knows 
it must not overwrite itself with decompressed data, so it will move 
itself away i.e. a second copy.  Bot those copies are totally useless 
and could be avoided if only u-Boot would stop ignoring the user's wish 
to load and run zImage from the address indicated by the provided 
argument to the load command.

This is a rigid restriction that a well intended bootloader would not 
impose all the time if the user wishes to do otherwise.  You might not 
be concerned by use cases where we want images to be executed right 
where they've been loaded irrespective of what that might be, but that 
doesn't make that any less of a valid use case.  And Stephen's patches 
are providing just that.


Nicolas

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Simon Glass]

Hi Nicolas,

On Mon, Nov 7, 2011 at 4:35 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Tue, 8 Nov 2011, Wolfgang Denk wrote:
>
>> Dear Nicolas Pitre,
>>
>> In message <alpine.LFD.2.02.1111071736280.3307@xanadu.home> you wrote:
>> >
>> > > 1) zImages are are relocatable. They should be loaded and started at
>> > > ? ?offsets between 32 KiB and 128 MiB in system RAM.
>> > >
>> > > 2) Raw images (without the preloader) have to be started at a fixed
>> > > ? ?address, virt_to_phys(PAGE_OFFSET + TEXT_OFFSET), which usually is
>> > > ? ?at an offset of 32 KiB in system RAM (with very few exzceptions).
>> > >
>> > > Both sitations can be handled perfectly find with offset addresses in
>> > > the images.
>> >
>> > No. ?Please, we're trying to remove _all_ hardcoded addresses from the
>> > kernel boot process, absolute or relative. ?...
>>
>> I understand you are referring here to zImages only. Correct?
>
> Correct. ?Anything else is not relocatable.
>
>> Or will raw images (without the preloader) be fully relocatable, too?
>
> No.
>
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?... ?This is why zImage can be
>> > loaded at practically any address. ?If uImage insists on having a
>> > relative offset encoded into it, this is slightly better than the
>> > absolute address but not by much. ?Having the ability to use the _same_
>> > uImage file and load it at _different_ addresses as specified by an
>> > argument to the load command is highly desirable...
>>
>> Why is it so important to load it at specific (different) addresses
>> when it can be started from any address?
>
> The kernel code can be started from any address. ?We want the _code_ to
> be that way.
>
> However a particular board may or may not load the kernel at any
> address. ?This is a machine specific restriction, not a kernel
> restriction.
>
>> Maybe this is a key point. ?I simply fail to understand this.
>
> Let me repeat again. ?We want one single kernel image binary that ARM
> distributions can use for all their target machines. ?It is therefore
> necessary that uImage be free of any hardcoded load address (absolute or
> relative). ?If a particular board require a particular load address for
> the kernel, this must be encoded in its own u-Boot environment and not
> in the distributed uImage. ?Failing that, uImage simply cannot be used
> as a distribution format for the kernel because any address/offset
> enforced by the uImage format is going to be incompatible with the needs
> of a particular machine somewhere.
>
>> > We don't want any hardcoded architecture specific address anymore.
>> > This is being removed from the kernel as we speak. ?If I cannot use a
>>
>> Also for raw images?
>
> No. ?The requirements on raw images are unchanged. ?you can use them if
> you wish, but generic ARM distributions can't use that if they want to
> target more than one SOC. ?Therefore raw images are not interesting by
> the use case at hand.

I will leave you to your discussion, but want to pick up on this point.

Can I assume that we have (or can have) a 'make uImage' target or
similar in the kernel which can pack together:

- a compressed kernel (not zImage, I mean something that U-Boot can
decompress), with a rel_offset of 32KB
- a DTB
- a ramdisk

and that with Stephen's patch (committed to U-Boot) today, we can, in
U-Boot, with a script, load this uImage to somewhere and have U-Boot
decompress the kernel and set the bits out nicely in RAM, no matter
where that RAM is? The kernel will start at 32KB, and the other bits
will be somewhere above that. Then U-Boot can enter the kernel at 32KB
and all will be well, yes?

Because it seems to me that this approach would work just as well as
the zImage approach (it is perhaps more 'pure' from a boot loader
point of view), and that the code in the kernel zImage header that
figures out where SDRAM is and decompresses the kernel to 32KB could
just as well be in U-Boot.

Then both groups get what they want.

Regards,
Simon

>
>> > totally generic way to not specify a load address (using -1 for example)
>> > with mkimage soon, I'll be forced to remove the uImage makefile target
>> > from Linux as it will simply be broken otherwise.
>>
>> Interesting argument. ?Because you are fully flexible and can use any
>> address you cannot accept a make target that wraps your fully
>> relocatable image to load it on a specific address. ?Now that makes
>> sense to me.
>>
>> Or do I smell attempted extortion?
>
> What do you not understand in the fact that such a specific address
> makes the resulting image not universal? ?I'm telling you that I need to
> produce a kernel image that doesn't carry with it any machine specific
> load address so that same image can be installed unmodified on any
> machine.
>
> Instead, you insist on making that image less useful by attaching to it
> some restrictions on its load address at build time rather than applying
> those restrictions only at load time and only on machines where that
> matters.
>
> For the last time, we don't want any address encoded in the kernel
> image for the simple fact that we don't know at build time what machine
> the kernel will be used on, and therefore what address to use. ?This is
> why zImage was made totally relocatable and totally position
> independent so it can figure out at run time what address to use.
> Figuring out the address to use at "make uImage" time only works for a
> kernel that will boot on a single specific machine.
>
>> > This allows to boot such image on only one specific architecture if
>> > uImage must contain architecture specific addresses, be that absolute or
>> > relative. ?Granted, a relative offset is less problematic than an
>> > absolute address, but it is a problem nevertheless. ?Such architecture
>>
>> Please explain _why_ you consider it a problem. ?Describe use cases
>> where it doesn't work.
>
> That has been explained a few times already.
>
>> > specific information must live with the boot loader (ideally as a
>> > script) and not embodied into an image that could otherwise be totally
>> > generic.
>>
>> Must it? ?I don't see the need. ?I don't even see the benefit.
>
> Why do I care having this conversation with you then? ?Please tell me.
>
>> > > With Stephen's new approach, we could only use the zImage approach,
>> > > and we have to add additional configuration information to the boot
>> > > loader.
>> >
>> > Absolutely! ?That is where that configuration information must be. ?The
>> > bootloader is not generic since it must initialize a very specific piece
>> > of hardware. ?It therefore makes sense to associate the per architecture
>> > details such as the best kernel load address with the bootloader, not in
>> > the image itself.
>>
>> Hm... if that is true, then it means we will have fully relocatable
>> raw images?
>
> It is already the case, more or less. ?The raw image _must_ be loaded at
> TEXT_OFFSET from start of RAM, regardless of where that RAM is. ?So if
> all you care about is raw image kernels, then having a relative a load
> address for uImage makes perfect sense.
>
> The role of the zImage code is to figure out at run time about its
> position in RAM, deduce where the RAM starts, relocate itself if it is
> in the way of the decompressed kernel, and decompress the actual kernel
> in its final location (RAM start + TEXT_OFFSET) without having to
> relocate the bigger decompressed data. ?Unless it is in the way of the
> decompressed kernel, this code can be executed in place and doesn't need
> to be relocated to any particular address.
>
>> > OTOH, the kernel must become as generic as possible. ?Using DT plays a
>> > big role in this. ?But not having a load address/offset encoded in the
>> > image is also part of this. Think of what mess a Linux distribution for
>> > ARM would otherwise have to carry when it must know that configuration
>> > information for each device it intends to support. ?Having a single
>> > kernel binary for as many ARM devices as possible is what we're aiming
>> > for, and currently u-Boot is one of the obstacles.
>>
>> Please explain which exact problems you see if Linux on ARM would by
>> default wrap the zImage into a relative uImage with load address =
>> entry point address at 32 KiB offset from start of system RAM?
>
> It will always have to relocate itself away from there as this is
> typically where the decompressed kernel must be placed.
>
> Knowing that, most people are already attempting to load their uImage
> elsewhere, say at an offset of 16Mb FROM START of RAM. ?Or maybe they
> have a ramdisk at that address so they load the kernel at an offset of
> 32MB. ?Or any such restriction that is particular to a specific
> machine/setup.
>
> But u-Boot will then ignore the wish of the user and relocate the image
> content to 32KB after start of RAM, causing one copy. ?Now zImage knows
> it must not overwrite itself with decompressed data, so it will move
> itself away i.e. a second copy. ?Bot those copies are totally useless
> and could be avoided if only u-Boot would stop ignoring the user's wish
> to load and run zImage from the address indicated by the provided
> argument to the load command.
>
> This is a rigid restriction that a well intended bootloader would not
> impose all the time if the user wishes to do otherwise. ?You might not
> be concerned by use cases where we want images to be executed right
> where they've been loaded irrespective of what that might be, but that
> doesn't make that any less of a valid use case. ?And Stephen's patches
> are providing just that.
>
>
> Nicolas
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Nicolas Pitre]

On Mon, 7 Nov 2011, Simon Glass wrote:

> On Mon, Nov 7, 2011 at 4:35 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> 
> > On Tue, 8 Nov 2011, Wolfgang Denk wrote:
> >
> >> Dear Nicolas Pitre,
> >>
> >> > We don't want any hardcoded architecture specific address anymore.
> >> > This is being removed from the kernel as we speak. ?If I cannot use a
> >>
> >> Also for raw images?
> >
> > No. ?The requirements on raw images are unchanged. ?you can use them if
> > you wish, but generic ARM distributions can't use that if they want to
> > target more than one SOC. ?Therefore raw images are not interesting by
> > the use case at hand.
> 
> I will leave you to your discussion, but want to pick up on this point.
> 
> Can I assume that we have (or can have) a 'make uImage' target or
> similar in the kernel which can pack together:
> 
> - a compressed kernel (not zImage, I mean something that U-Boot can
> decompress), with a rel_offset of 32KB

Yes.

> - a DTB

No.

> - a ramdisk

No.  However you can provide a set of files which can be included in an 
initramfs image linked directly into the kernel.  But distributors never 
use that facility as they prefer a customized ramdisk created during 
kernel installation.

> and that with Stephen's patch (committed to U-Boot) today, we can, in
> U-Boot, with a script, load this uImage to somewhere and have U-Boot
> decompress the kernel and set the bits out nicely in RAM, no matter
> where that RAM is? The kernel will start at 32KB, and the other bits
> will be somewhere above that. Then U-Boot can enter the kernel at 32KB
> and all will be well, yes?
> 
> Because it seems to me that this approach would work just as well as
> the zImage approach (it is perhaps more 'pure' from a boot loader
> point of view), and that the code in the kernel zImage header that
> figures out where SDRAM is and decompresses the kernel to 32KB could
> just as well be in U-Boot.

Firstly, there is not just u-Boot out there.  And since the layout 
optimization for Linux when decompressing is by definition Linux 
specific, this better live in zImage than be duplicated in every 
bootloaders.

Secondly, we don't want to pack a DTB with the kernel.  For the same 
reasons as for the load address, we want a distributable kernel binary 
which contains no assumption about any specific board or machine.  The 
kernel should be generic and be provided a machine specific DTB by the 
boot loader.

> Then both groups get what they want.

No.  For both groups to get what they want, Stephen's latest patches 
should be merged.  All they do is to allow for -1 as a load address in 
uImage to mean "never relocate but just use whatever address where 
uImage has been loaded."  This cannot be simpler than that.


Nicolas

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Simon Glass]

Hi Nicolas,

On Mon, Nov 7, 2011 at 7:51 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Mon, 7 Nov 2011, Simon Glass wrote:
>
>> On Mon, Nov 7, 2011 at 4:35 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
>>
>> > On Tue, 8 Nov 2011, Wolfgang Denk wrote:
>> >
>> >> Dear Nicolas Pitre,
>> >>
>> >> > We don't want any hardcoded architecture specific address anymore.
>> >> > This is being removed from the kernel as we speak. ?If I cannot use a
>> >>
>> >> Also for raw images?
>> >
>> > No. ?The requirements on raw images are unchanged. ?you can use them if
>> > you wish, but generic ARM distributions can't use that if they want to
>> > target more than one SOC. ?Therefore raw images are not interesting by
>> > the use case at hand.
>>
>> I will leave you to your discussion, but want to pick up on this point.
>>
>> Can I assume that we have (or can have) a 'make uImage' target or
>> similar in the kernel which can pack together:
>>
>> - a compressed kernel (not zImage, I mean something that U-Boot can
>> decompress), with a rel_offset of 32KB
>
> Yes.
>
>> - a DTB
>
> No.
>
>> - a ramdisk
>
> No. ?However you can provide a set of files which can be included in an
> initramfs image linked directly into the kernel. ?But distributors never
> use that facility as they prefer a customized ramdisk created during
> kernel installation.
>
>> and that with Stephen's patch (committed to U-Boot) today, we can, in
>> U-Boot, with a script, load this uImage to somewhere and have U-Boot
>> decompress the kernel and set the bits out nicely in RAM, no matter
>> where that RAM is? The kernel will start at 32KB, and the other bits
>> will be somewhere above that. Then U-Boot can enter the kernel at 32KB
>> and all will be well, yes?
>>
>> Because it seems to me that this approach would work just as well as
>> the zImage approach (it is perhaps more 'pure' from a boot loader
>> point of view), and that the code in the kernel zImage header that
>> figures out where SDRAM is and decompresses the kernel to 32KB could
>> just as well be in U-Boot.
>
> Firstly, there is not just u-Boot out there. ?And since the layout
> optimization for Linux when decompressing is by definition Linux
> specific, this better live in zImage than be duplicated in every
> bootloaders.

Actually I was talking about the case where U-Boot does the
decompression. You have said it is supported above. I don't suggest
that it be the only option, only that it be an option. Perhaps only
U-Boot will use it, but that is fine. U-Boot is a popular boot loader.

If the boot loader puts the pieces in the right place, decompressed
and ready to go, we could presumably avoid this code in the kernel:

?1096 ?4860 27129 arch/arm/boot/compressed/head.S
   13    53   304 arch/arm/boot/compressed/big-endian.S
   50   153  1267 arch/arm/boot/compressed/decompress.c
 1096  4860 27129 arch/arm/boot/compressed/head.S
   47   214  1238 arch/arm/boot/compressed/head-sa1100.S
  139   650  3537 arch/arm/boot/compressed/head-shark.S
  150   619  3564 arch/arm/boot/compressed/head-sharpsl.S
   53   263  1685 arch/arm/boot/compressed/head-shmobile.S
   41   179   992 arch/arm/boot/compressed/head-xscale.S
  134   571  2868 arch/arm/boot/compressed/ll_char_wr.S
  124   324  3011 arch/arm/boot/compressed/Makefile
  208   589  3812 arch/arm/boot/compressed/misc.c
  260   604  5289 arch/arm/boot/compressed/ofw-shark.c
    6    10   145 arch/arm/boot/compressed/piggy.gzip.S
    6    10   145 arch/arm/boot/compressed/piggy.lzma.S
    6    10   144 arch/arm/boot/compressed/piggy.lzo.S
   70   226  1481 arch/arm/boot/compressed/vmlinux.lds.in
 2403  9335 56611 total

>
> Secondly, we don't want to pack a DTB with the kernel. ?For the same
> reasons as for the load address, we want a distributable kernel binary
> which contains no assumption about any specific board or machine. ?The
> kernel should be generic and be provided a machine specific DTB by the
> boot loader.

Yes, although one way for the boot loader to do that is to use mkimage
to create a FIT image with the kernel and the DTB file. Can the kernel
better facilitate that?

>
>> Then both groups get what they want.
>
> No. ?For both groups to get what they want, Stephen's latest patches
> should be merged. ?All they do is to allow for -1 as a load address in
> uImage to mean "never relocate but just use whatever address where
> uImage has been loaded." ?This cannot be simpler than that.

Quite agree with the latest patch - I tested it.

That gives the kernel what it wants. How can we give U-Boot what it
wants, which is apparently the ability to decompress the kernel itself
and arrange everything in memory at the right place? Wolfgang
complains that patches to do this have been repeatedly rejected in the
kernel. If this is the FIT image, how about adding a 'fitImage' make
target?

These are the targets I can see in the kernel:

Architecture specific targets (arm):
* zImage        - Compressed kernel image (arch/arm/boot/zImage)
  Image         - Uncompressed kernel image (arch/arm/boot/Image)
* xipImage      - XIP kernel image, if configured (arch/arm/boot/xipImage)
  uImage        - U-Boot wrapped zImage
  bootpImage    - Combined zImage and initial RAM disk
                  (supply initrd image via make variable INITRD=<path>)
  dtbs          - Build device tree blobs for enabled boards
  install       - Install uncompressed kernel
  zinstall      - Install compressed kernel
                  Install using (your) ~/bin/installkernel or
                  (distribution) /sbin/installkernel or
                  install to $(INSTALL_PATH) and run lilo

There doesn't seem to be a FIT image, but yet this is the 'modern'
image format in U-Boot. It seems odd that uImage says 'U-Boot wrapped
zImage' which suggests that it cannot support U-Boot decompressing the
image.

There seems to be a discussion about it here:

http://permalink.gmane.org/gmane.linux.kbuild.devel/4345

Regards,
Simon

>
>
> Nicolas
>

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ2aRP5Hn-3jREa=OfGs0K7Ny9b2mWP3pwPBRW5svL3Aew@mail.gmail.com> you wrote:
> 
> > Firstly, there is not just u-Boot out there.  And since the layout
> > optimization for Linux when decompressing is by definition Linux
> > specific, this better live in zImage than be duplicated in every
> > bootloaders.
> 
> Actually I was talking about the case where U-Boot does the
> decompression. You have said it is supported above. I don't suggest
> that it be the only option, only that it be an option. Perhaps only
> U-Boot will use it, but that is fine. U-Boot is a popular boot loader.


Thanks a lot for bringing up these arguments.

> If the boot loader puts the pieces in the right place, decompressed
> and ready to go, we could presumably avoid this code in the kernel:
> 
>  1096  4860 27129 arch/arm/boot/compressed/head.S
>    13    53   304 arch/arm/boot/compressed/big-endian.S
>    50   153  1267 arch/arm/boot/compressed/decompress.c
>  1096  4860 27129 arch/arm/boot/compressed/head.S
>    47   214  1238 arch/arm/boot/compressed/head-sa1100.S
>   139   650  3537 arch/arm/boot/compressed/head-shark.S
>   150   619  3564 arch/arm/boot/compressed/head-sharpsl.S
>    53   263  1685 arch/arm/boot/compressed/head-shmobile.S
>    41   179   992 arch/arm/boot/compressed/head-xscale.S
>   134   571  2868 arch/arm/boot/compressed/ll_char_wr.S
>   124   324  3011 arch/arm/boot/compressed/Makefile
>   208   589  3812 arch/arm/boot/compressed/misc.c
>   260   604  5289 arch/arm/boot/compressed/ofw-shark.c
>     6    10   145 arch/arm/boot/compressed/piggy.gzip.S
>     6    10   145 arch/arm/boot/compressed/piggy.lzma.S
>     6    10   144 arch/arm/boot/compressed/piggy.lzo.S
>    70   226  1481 arch/arm/boot/compressed/vmlinux.lds.in
>  2403  9335 56611 total

I think you can even add the actual (de-) compressor routines.

> That gives the kernel what it wants. How can we give U-Boot what it
> wants, which is apparently the ability to decompress the kernel itself
> and arrange everything in memory at the right place? Wolfgang
> complains that patches to do this have been repeatedly rejected in the
> kernel. If this is the FIT image, how about adding a 'fitImage' make
> target?

It would not only be FIT images.  Why not support old uImage format in
a "proper" way?  In most cases people do not need the features
provided by FIT images, and they prefer the simplicity of uImages.


Thanks again.

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
Crash programs fail because they are based on the theory  that,  with
nine women pregnant, you can get a baby a month.  - Wernher von Braun

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Loïc Minier]

On Mon, Nov 07, 2011, Simon Glass wrote:
>                                      How can we give U-Boot what it
> wants, which is apparently the ability to decompress the kernel itself
> and arrange everything in memory at the right place? Wolfgang
> complains that patches to do this have been repeatedly rejected in the
> kernel. If this is the FIT image, how about adding a 'fitImage' make
> target?

 Looking at this thread from the point of view of distributions (e.g.
 Debian/Fedora/Ubuntu etc.), I see some integration disadvantages in the
 FIT image approach and to uImage in general.  The format itself is nice
 and handy, but in an ideal world a distribution would provide a
 SoC-agnostic image, and even a bootloader-agnostic image.


 Currently, what Debian and Ubuntu do (didn't check Fedora) is:
 * ship zImage files (vmlinuz-*) in kernel .deb packages
 * when kernels or initrd related files are installed, generate:
   - uImage from the right vmlinuz file with the exact load value
   - an initrd.gz with initramfs-tools, then an uInitrd from it
   - a boot script for that board, then a boot.scr from it which is an
     U-Boot script in uImage format

 This is painful in at least these ways:

 a) images are not board-agnostic or even bootloader agnostic (I'd love
    if a single SD card image would boot on multiple OMAP3 boards, or on
    OMAP3 and OMAP4, or on i.MX51, i.MX53, OMAP3 and OMAP4, whether the
    firmware uses U-Boot or is just a boot ROM, or UEFI or whatever)

 b) distributors keep track of load addresses for uImage/uInitrd/boot.scr
    as to generate correct uImages

 c) this wastes space for multiple copies of the boot files (uImage +
    uInitrd + boot.scr + vmlinuz + initrd.gz)

 I can see how the proposal to use relative load addresses or to set -1
 as load address in uImage help with some of the above, but for sure c)
 will remain a hard issue with FIT images as some distributors will keep
 generating an initrd when various packages get installed (e.g. for LVM
 or mdadm support, or encrypted root etc.).

 There are advantages to the uImage format such as usage in flash, or
 choice of compression type (whatever you stuff into it), and these
 remain valid in other use cases, but it seems sensible to support
 vmlinuz directly from U-Boot to support a more generic distro image
 boot protocol.


 Personally, I also like the separation of layers so that vmlinuz is
 kept opaque; for instance vmlinuz can embed an initrd or a DTB without
 the bootloader having to care about them, or it could be compressed
 with LZO or bzip2 even if the bootloader in firmware doesn't support
 this or that compression type.  I think distros would prefer not using
 these features and having a DTB-aware bootloader though.

-- 
Lo?c Minier

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Jason]

Nicolas,

On Mon, Nov 07, 2011 at 10:51:33PM -0500, Nicolas Pitre wrote:
> On Mon, 7 Nov 2011, Simon Glass wrote:
> 
> > On Mon, Nov 7, 2011 at 4:35 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> > 
> > > On Tue, 8 Nov 2011, Wolfgang Denk wrote:
> > >
> > >> Dear Nicolas Pitre,
> > >>
> > >> > We don't want any hardcoded architecture specific address anymore.
> > >> > This is being removed from the kernel as we speak. ?If I cannot use a
> > >>
> > >> Also for raw images?
> > >
> > > No. ?The requirements on raw images are unchanged. ?you can use them if
> > > you wish, but generic ARM distributions can't use that if they want to
> > > target more than one SOC. ?Therefore raw images are not interesting by
> > > the use case at hand.
> > 
> > I will leave you to your discussion, but want to pick up on this point.
> > 
> > Can I assume that we have (or can have) a 'make uImage' target or
> > similar in the kernel which can pack together:
> > 
> > - a compressed kernel (not zImage, I mean something that U-Boot can
> > decompress), with a rel_offset of 32KB
> 
> Yes.
> 
> > - a DTB
> 
> No.
> 
> > - a ramdisk
> 
> No.  However you can provide a set of files which can be included in an 
> initramfs image linked directly into the kernel.  But distributors never 
> use that facility as they prefer a customized ramdisk created during 
> kernel installation.

exactly, see my comments below.

> > and that with Stephen's patch (committed to U-Boot) today, we can, in
> > U-Boot, with a script, load this uImage to somewhere and have U-Boot
> > decompress the kernel and set the bits out nicely in RAM, no matter
> > where that RAM is? The kernel will start at 32KB, and the other bits
> > will be somewhere above that. Then U-Boot can enter the kernel at 32KB
> > and all will be well, yes?
> > 
> > Because it seems to me that this approach would work just as well as
> > the zImage approach (it is perhaps more 'pure' from a boot loader
> > point of view), and that the code in the kernel zImage header that
> > figures out where SDRAM is and decompresses the kernel to 32KB could
> > just as well be in U-Boot.
> 
> Firstly, there is not just u-Boot out there.  And since the layout 
> optimization for Linux when decompressing is by definition Linux 
> specific, this better live in zImage than be duplicated in every 
> bootloaders.
> 
> Secondly, we don't want to pack a DTB with the kernel.  For the same 
> reasons as for the load address, we want a distributable kernel binary 
> which contains no assumption about any specific board or machine.  The 
> kernel should be generic and be provided a machine specific DTB by the 
> boot loader.
> 
> > Then both groups get what they want.
> 
> No.  For both groups to get what they want, Stephen's latest patches 
> should be merged.  All they do is to allow for -1 as a load address in 
> uImage to mean "never relocate but just use whatever address where 
> uImage has been loaded."  This cannot be simpler than that.

It sounds like you are intending for distributions to provide uImages.
Why can't they provide a generic zImage, and a post-install hook runs
mkimage to add the board specific uImage header?  Similar to update-grub
on x86{_64}.  This seems _more_ flexible to me, and fitting with
standard conventions distributions are accustomed to.

eg, after a kernel upgrade on a debian system, since I had 'apt-get
install u-boot' there is a post-kernel install hook that would run a
distro provided 'update-uboot' script.  This script would read a board
specific config file (or, the u-boot environment), add the u-boot header
to the kernel, and possibly modify the u-boot environment to now load
uImage-3.1-34.  The file shipped with the kernel image package was a
generic zImage which will now work on the board.

Or, maybe I'm missing something...  I always thought 'make uImage' was
a convenience for developers.

hth,

Jason.

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Nicolas Pitre]

On Tue, 8 Nov 2011, Jason wrote:

> It sounds like you are intending for distributions to provide uImages.
> Why can't they provide a generic zImage, and a post-install hook runs
> mkimage to add the board specific uImage header?  Similar to update-grub
> on x86{_64}.  This seems _more_ flexible to me, and fitting with
> standard conventions distributions are accustomed to.

That means also distributing, and more importantly maintaining, a list 
of boards and their particular requirements.  Or, alternately, digging 
into the u-Boot environment to figure out that information which is 
fragile and certainly more complex than not having to do this operation 
at all.

Standard conventions for distributions currently mean to simply drop the 
kernel in a known location and let the bootloader load it without any 
special wrapping.  Given that they already handle grub config files, 
having the same config format used on ARM would certainly make things 
even more "conventional".

Maybe we could port grub2 to be executed on top of u-Boot as a second 
stage bootloader?


Nicolas

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Linus Walleij]

On Tue, Nov 8, 2011 at 2:10 AM, Simon Glass <sjg@chromium.org> wrote:

> Can I assume that we have (or can have) a 'make uImage' target or
> similar in the kernel which can pack together:
>
> - a compressed kernel (not zImage, I mean something that U-Boot can
> decompress), with a rel_offset of 32KB

As explained by Nico, having the boot loader decompress the
kernel is *bad*.

> - a DTB
> - a ramdisk

When testing various systems I already usually use a uImage with
zImage+initramfs+DTB. This is frowned upon in some places but
it works like this:

I attach the initramfs from a cpio file rather than from giving a file
path list. This is rather popular in some embedded systems.

I configure it in like this with the config script
(you can also use menuconfig or interactive config...):

scripts/config --file .config \
        --enable BLK_DEV_INITRD \
        --set-str INITRAMFS_SOURCE rootfs.cpio \
        --enable INITRAMFS_COMPRESSION_GZIP \
        --enable RD_GZIP

Since this was so convenient I made a patch to attach a DTB
the same way which was floated on devicetree-discuss:
http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg07256.html

Nico didn't like that:
http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg06828.html

Alas, I'm still using that patch though. For systems where I cannot
influence the boot loader it's easiest, since I just add another
config snippet when I want to run them with DT:

scripts/config --file .config \
        --enable USE_OF \
        --enable ARM_APPENDED_DTB \
        --enable ARM_ATAG_DTB_COMPAT \
        --enable PROC_DEVICETREE \
        --set-str ARM_APPENDED_DTB_FILE my-device-tree.dtb

Yours,
Linus Walleij

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Linus Walleij,

In message <CACRpkdY7fov87fuSS+KfbDtyGhR2NtbTXzVEEDrBUawXeHPmUw@mail.gmail.com> you wrote:
> 
> > Can I assume that we have (or can have) a 'make uImage' target or
> > similar in the kernel which can pack together:
> >
> > - a compressed kernel (not zImage, I mean something that U-Boot can
> > decompress), with a rel_offset of 32KB
> 
> As explained by Nico, having the boot loader decompress the
> kernel is *bad*.

This is your point of view, but others (including me) think different.

> I configure it in like this with the config script
> (you can also use menuconfig or interactive config...):
> 
> scripts/config --file .config \
>         --enable BLK_DEV_INITRD \
>         --set-str INITRAMFS_SOURCE rootfs.cpio \
>         --enable INITRAMFS_COMPRESSION_GZIP \
>         --enable RD_GZIP
> 
> Since this was so convenient I made a patch to attach a DTB

Convenient?  Again, this is a matter of taste and aquired habits.

With U-Boot none of this is needed.

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
Power is danger.
	-- The Centurion, "Balance of Terror", stardate 1709.2

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Linus Walleij]

On Sat, Dec 10, 2011 at 11:53 PM, Wolfgang Denk <wd@denx.de> wrote:
> [Me]
>> As explained by Nico, having the boot loader decompress the
>> kernel is *bad*.
>
> This is your point of view, but others (including me) think different.

Yes, as C. B. Roylance Kent stated in 1893:
Those are my principles, but if you don't like them, they can be changed.

Thanks,
Linus Walleij

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

On 12/10/2011 03:42 PM, Linus Walleij wrote:
...
> Since this was so convenient I made a patch to attach a DTB
> the same way which was floated on devicetree-discuss:
> http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg07256.html
> 
> Nico didn't like that:
> http://www.mail-archive.com/devicetree-discuss at lists.ozlabs.org/msg06828.html
> 
> Alas, I'm still using that patch though. For systems where I cannot
> influence the boot loader it's easiest, since I just add another
> config snippet when I want to run them with DT:
> 
> scripts/config --file .config \
>         --enable USE_OF \
>         --enable ARM_APPENDED_DTB \
>         --enable ARM_ATAG_DTB_COMPAT \
>         --enable PROC_DEVICETREE \
>         --set-str ARM_APPENDED_DTB_FILE my-device-tree.dtb

Jut as an FYI, there's also:

git://git.secretlab.ca/git/linux-2.6 devicetree/test
fbae2702ae21ad8d1de5f135647c30b250782060 "Build a uImage with dtb
already appended".

I tend to use this all the time for device-tree.

I find this significantly easier than using U-Boot's
pass-an-FDT-to-the-kernel feature, because right now half the time I'm
passing an FDT and half not, and making U-Boot's script support either
option automatically isn't something I really want to get into when I
can simply "make uImage" or "make uImage-dtb.tegra-${board}" to select
which option I want when building the kernel.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Nicolas,

may I suggest that you please try to relax for a moment, and try to
look at things from a completely unprejudiced point of view?  We will
come back to your arguments later, promised.


In message <alpine.LFD.2.02.1111071840080.3307@xanadu.home> you wrote:
>
> > I understand you are referring here to zImages only. Correct?
> 
> Correct.  Anything else is not relocatable.
> 
> > Or will raw images (without the preloader) be fully relocatable, too?
> 
> No.

OK. So the situation on ARM is basicly not different from what we
have on other architectures.

The uImage format has been designed to handle this situation: we
normally wrap the raw image (eventually after compressing it) into an
uImage, and insert the load and entry point addresses that are well
known at kernel build time.  uImages are not directly executable.

zImages on ARM include a preloader, that wraps the the raw image
(eventually after compressing it) and packs it together with some
code that internally accounts for the fixed load and entry point
addresses of the kernel which implements the needed relocation and
uncompression routines to form a new executable, position independent
format.

In both cases the _kernel_ image is not position independent. It must
be loaded to a specific address and started at a specific entry point.
The exact information where these are is known at built time, and
somehow encoded in the images (here in the image header, there in the
preloader code).


Is this summary correct so far?


> > Why is it so important to load it at specific (different) addresses
> > when it can be started from any address?
> 
> The kernel code can be started from any address.  We want the _code_ to 
> be that way.

This is not quite correct.  The _kernel_ code has a fixed address.
It is the preloader code which can be started from any address.


> However a particular board may or may not load the kernel at any 
> address.  This is a machine specific restriction, not a kernel 
> restriction.

Correct.  Stephen Warren also writes in <4EB87122.3050602@nvidia.com>:
"Presumably whoever constructed the environment has the most detailed
knowledge of the HW's and U-Boot environment's expected memory layout
for that particular board."   And in <4EB87375.1040100@nvidia.com>:
"The only place that has full knowledge of the board's memory layout
is the U-Boot environment for that board, and hence I assert that the
U-Boot environment should define where to load the kernel (and initrd
and FDT), and if U-Boot must copy them, where to copy them to. In
particular, the creator of the uImage can't possibly pick these
values and expect them to work everywhere."

I mostly agree with this.  That's the reason why U-Boot offers to
handle this based on a better understanding of the low level hardware
details.  This seems to me to be an easier way that to handle this in
the Linux kernel environment (in the form of the preloader).


> > Maybe this is a key point.  I simply fail to understand this.
> 
> Let me repeat again.  We want one single kernel image binary that ARM 
> distributions can use for all their target machines.  It is therefore 
> necessary that uImage be free of any hardcoded load address (absolute or 
> relative).  If a particular board require a particular load address for 
> the kernel, this must be encoded in its own u-Boot environment and not 
> in the distributed uImage.  Failing that, uImage simply cannot be used 
> as a distribution format for the kernel because any address/offset 
> enforced by the uImage format is going to be incompatible with the needs 
> of a particular machine somewhere.

uImage is _intented_ to carry a raw OS image and provide the boot
loader the information where to load and start it.

You do not want to do that, so you should not attempt to assault
uImages to make it fit your completely different purposes.  If you
want to use relocatable zImages which have all the needed information
encoded internally then just do so.  But do not insist that uImage
does things it was not designed for.

> No.  The requirements on raw images are unchanged.  you can use them if 
> you wish, but generic ARM distributions can't use that if they want to 
> target more than one SOC.  Therefore raw images are not interesting by 
> the use case at hand.

They are not interesting for you.  But that does not mean that they
are not interesting in general.  Simon Glass brings up this point,
too, in <CAPnjgZ1WajMLznSDNDLBys-C3Um2rVvZjOLk3BfSO2M=FRW2Dg@mail.gmail.com>:

| Can I assume that we have (or can have) a 'make uImage' target or
| similar in the kernel which can pack together:
| 
| - a compressed kernel (not zImage, I mean something that U-Boot can
| decompress), with a rel_offset of 32KB
| - a DTB
| - a ramdisk
| 
| and that with Stephen's patch (committed to U-Boot) today, we can, in
| U-Boot, with a script, load this uImage to somewhere and have U-Boot
| decompress the kernel and set the bits out nicely in RAM, no matter
| where that RAM is? The kernel will start at 32KB, and the other bits
| will be somewhere above that. Then U-Boot can enter the kernel at 32KB
| and all will be well, yes?

Why are attempts blocked to provide such support for people who prefer
this approach?  A "one size fits all" approach like "we have always
been using zImages, so it must be a perfect solution for everybody" is
eventually _not_ the optimal solution for all.


> What do you not understand in the fact that such a specific address 
> makes the resulting image not universal?  I'm telling you that I need to 
> produce a kernel image that doesn't carry with it any machine specific 
> load address so that same image can be installed unmodified on any 
> machine.

And what I'm telling you is that you could probably have that for free
if you dropped the preloader.

> Instead, you insist on making that image less useful by attaching to it 
> some restrictions on its load address at build time rather than applying 
> those restrictions only at load time and only on machines where that 
> matters.
> 
> For the last time, we don't want any address encoded in the kernel 
> image for the simple fact that we don't know at build time what machine 
> the kernel will be used on, and therefore what address to use.  This is 

But instead of letting the boot loader who has the detailled knowledge
about which memory is actually present on the target system you build
a non-trivial preloader and insist that the ultimate way is to wrape
the kernel with it.


> why zImage was made totally relocatable and totally position 
> independent so it can figure out at run time what address to use.  
> Figuring out the address to use at "make uImage" time only works for a 
> kernel that will boot on a single specific machine.

There is no actual reason this has to be that way.  If you want then
risk a view over the rim of your plate and look how it's working for
other architectures.  It's not really new problems we're dealing with
here.  Kernel images that run on a number of different processors and
boards are not new, and device tree support has been available for
years, too.  It's not that there is no preexisting experience with
such issues.

On Power Architecture systems, we use uImage format and/or FIT image
format to wrap raw kernel images (without preloader).  If somebody
wants, he can attach / include the DT blob to that image, and
eventually also a ramdisk/initrd image.  This is not mandatory, it is
all optimal.  And it just works.



> > Please explain _why_ you consider it a problem.  Describe use cases
> > where it doesn't work.
> 
> That has been explained a few times already.
> 
> > > specific information must live with the boot loader (ideally as a 
> > > script) and not embodied into an image that could otherwise be totally 
> > > generic.
> > 
> > Must it?  I don't see the need.  I don't even see the benefit.
> 
> Why do I care having this conversation with you then?  Please tell me.

I can ask you the same.  I ask you for a real use case example, and
all you reply is "That has been explained a few times already."
Without firther reference.

At least I was trying to understand you, and listening to your
arguments.  I cannot say that I feel you did so, too.


> It is already the case, more or less.  The raw image _must_ be loaded at 
> TEXT_OFFSET from start of RAM, regardless of where that RAM is.  So if 
> all you care about is raw image kernels, then having a relative a load 
> address for uImage makes perfect sense.

Thanks for this clear statement.  So I will apply the patches to add
such support to U-Boot.

> The role of the zImage code is to figure out at run time about its 
> position in RAM, deduce where the RAM starts, relocate itself if it is 
> in the way of the decompressed kernel, and decompress the actual kernel 
> in its final location (RAM start + TEXT_OFFSET) without having to 
> relocate the bigger decompressed data.  Unless it is in the way of the 
> decompressed kernel, this code can be executed in place and doesn't need 
> to be relocated to any particular address.

In U-Boot, this is actually not needed, as U-Boot offers to do exactly
that. As an additional benefit, you don't have to download and store
the wrapper code, which results in smaller and thus even faster
booting images.

> But u-Boot will then ignore the wish of the user and relocate the image 
> content to 32KB after start of RAM, causing one copy.  Now zImage knows 
> it must not overwrite itself with decompressed data, so it will move 
> itself away i.e. a second copy.  Bot those copies are totally useless 
> and could be avoided if only u-Boot would stop ignoring the user's wish 
> to load and run zImage from the address indicated by the provided 
> argument to the load command.

...or they could be avoided if you did not use the zImage wrapper in
the first place.

> This is a rigid restriction that a well intended bootloader would not 
> impose all the time if the user wishes to do otherwise.  You might not 
> be concerned by use cases where we want images to be executed right 
> where they've been loaded irrespective of what that might be, but that 
> doesn't make that any less of a valid use case.  And Stephen's patches 
> are providing just that.

No, they aren't.

I already NAKed these patches, and this discussion has made it clear
to me that this was a correct decision.  What you want is not uImages.


I suggest the following:

- I will apply Stephen's previous patches to support relative images
  as they are useful for people who want to use "proper" uImages
  (containing a raw kernel image as payload) on different boards /
  architectures.

- If you want to boot zImages (with the kernel wrapper included and
  thus fully relocatable), then please feel free and submit patches to
  add support for booting zImage format in U-Boot.  Then you get what
  you want, and you can use zImages directly, without the 64 byte
  uImage header that is only hindering for your usage mode.

- It would be appreciated if we could get support for real uImages
  (wrapping a raw kernel image) into Linux, then.

This way those who want to use zImages can do so, and those who prefer
a different approach can have their ways, too.

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Nicolas Pitre]

On Tue, 8 Nov 2011, Wolfgang Denk wrote:

> In message <alpine.LFD.2.02.1111071840080.3307@xanadu.home> you wrote:
> >
> > > I understand you are referring here to zImages only. Correct?
> > 
> > Correct.  Anything else is not relocatable.
> > 
> > > Or will raw images (without the preloader) be fully relocatable, too?
> > 
> > No.
> 
> OK. So the situation on ARM is basicly not different from what we
> have on other architectures.
> 
> The uImage format has been designed to handle this situation: we
> normally wrap the raw image (eventually after compressing it) into an
> uImage, and insert the load and entry point addresses that are well
> known at kernel build time.  uImages are not directly executable.
> 
> zImages on ARM include a preloader, that wraps the the raw image
> (eventually after compressing it) and packs it together with some
> code that internally accounts for the fixed load and entry point
> addresses of the kernel which implements the needed relocation and
> uncompression routines to form a new executable, position independent
> format.
> 
> In both cases the _kernel_ image is not position independent. It must
> be loaded to a specific address and started at a specific entry point.
> The exact information where these are is known at built time, and
> somehow encoded in the images (here in the image header, there in the
> preloader code).
> 
> 
> Is this summary correct so far?

No.  Your statement that says "The exact information where these are is 
known at built time, and somehow encoded in the images (here in the 
image header, there in the preloader code)" is false.  None of that 
information is known at build time nor encoded in the preloader as you 
call it.

> > > Why is it so important to load it at specific (different) addresses
> > > when it can be started from any address?
> > 
> > The kernel code can be started from any address.  We want the _code_ to 
> > be that way.
> 
> This is not quite correct.  The _kernel_ code has a fixed address.
> It is the preloader code which can be started from any address.

The preloader is part of the kernel, whether you like it or not, and it 
is not going away any time soon.  That would help the conversation if 
you accepted it as such.

> > However a particular board may or may not load the kernel at any 
> > address.  This is a machine specific restriction, not a kernel 
> > restriction.
> 
> Correct.  Stephen Warren also writes in <4EB87122.3050602@nvidia.com>:
> "Presumably whoever constructed the environment has the most detailed
> knowledge of the HW's and U-Boot environment's expected memory layout
> for that particular board."   And in <4EB87375.1040100@nvidia.com>:
> "The only place that has full knowledge of the board's memory layout
> is the U-Boot environment for that board, and hence I assert that the
> U-Boot environment should define where to load the kernel (and initrd
> and FDT), and if U-Boot must copy them, where to copy them to. In
> particular, the creator of the uImage can't possibly pick these
> values and expect them to work everywhere."
> 
> I mostly agree with this.  That's the reason why U-Boot offers to
> handle this based on a better understanding of the low level hardware
> details.  This seems to me to be an easier way that to handle this in
> the Linux kernel environment (in the form of the preloader).

You are mixing up issues.  The bootloader has knowledge of where is the 
best place to load kernel images on a per machine basis.  This is why 
the zImage code was made to accept and execute from any address in order 
to give the bootloader any possible flexibility in that regard.
However it is the kernel's responsibility to deal with its own 
environment and change it at will when we see fit, and therefore this is 
best if the bootloader doesn't take any responsibility here.

> > > Maybe this is a key point.  I simply fail to understand this.
> > 
> > Let me repeat again.  We want one single kernel image binary that ARM 
> > distributions can use for all their target machines.  It is therefore 
> > necessary that uImage be free of any hardcoded load address (absolute or 
> > relative).  If a particular board require a particular load address for 
> > the kernel, this must be encoded in its own u-Boot environment and not 
> > in the distributed uImage.  Failing that, uImage simply cannot be used 
> > as a distribution format for the kernel because any address/offset 
> > enforced by the uImage format is going to be incompatible with the needs 
> > of a particular machine somewhere.
> 
> uImage is _intented_ to carry a raw OS image and provide the boot
> loader the information where to load and start it.
> 
> You do not want to do that, so you should not attempt to assault
> uImages to make it fit your completely different purposes.  If you
> want to use relocatable zImages which have all the needed information
> encoded internally then just do so.  But do not insist that uImage
> does things it was not designed for.

OK, fine.  I don't really care about uImage if it cannot accommodate our 
needs.  I thought that the -1 extension was a pretty agile and 
non-obstrusive way to make u-Boot more flexible and versatile, and not 
only for Linux loading.  Failing that, can we have support for loading 
zImage directly then?  There are precedents in the upstream u-Boot for 
doing that on other architectures already.

> > No.  The requirements on raw images are unchanged.  you can use them if 
> > you wish, but generic ARM distributions can't use that if they want to 
> > target more than one SOC.  Therefore raw images are not interesting by 
> > the use case at hand.
> 
> They are not interesting for you.  But that does not mean that they
> are not interesting in general.  Simon Glass brings up this point,
> too, in <CAPnjgZ1WajMLznSDNDLBys-C3Um2rVvZjOLk3BfSO2M=FRW2Dg@mail.gmail.com>:
> 
> | Can I assume that we have (or can have) a 'make uImage' target or
> | similar in the kernel which can pack together:
> | 
> | - a compressed kernel (not zImage, I mean something that U-Boot can
> | decompress), with a rel_offset of 32KB
> | - a DTB
> | - a ramdisk
> | 
> | and that with Stephen's patch (committed to U-Boot) today, we can, in
> | U-Boot, with a script, load this uImage to somewhere and have U-Boot
> | decompress the kernel and set the bits out nicely in RAM, no matter
> | where that RAM is? The kernel will start at 32KB, and the other bits
> | will be somewhere above that. Then U-Boot can enter the kernel at 32KB
> | and all will be well, yes?
> 
> Why are attempts blocked to provide such support for people who prefer
> this approach?  A "one size fits all" approach like "we have always
> been using zImages, so it must be a perfect solution for everybody" is
> eventually _not_ the optimal solution for all.

I never intended to prevent raw images from being used with uImage.  
This is however not the use case I'm looking after.  That doesn't mean 
that because I'm interested in a different usage model than yours that 
I'm going to 
actively prevent raw Image from being used.  I see obvious limitations 
with it, but if that is what you want then by all means just use that.  
We cannot say as much from the u-Boot side for the use case I'm 
interested in though.

> > What do you not understand in the fact that such a specific address 
> > makes the resulting image not universal?  I'm telling you that I need to 
> > produce a kernel image that doesn't carry with it any machine specific 
> > load address so that same image can be installed unmodified on any 
> > machine.
> 
> And what I'm telling you is that you could probably have that for free
> if you dropped the preloader.

But that is not what I want.

> > Instead, you insist on making that image less useful by attaching to it 
> > some restrictions on its load address at build time rather than applying 
> > those restrictions only at load time and only on machines where that 
> > matters.
> > 
> > For the last time, we don't want any address encoded in the kernel 
> > image for the simple fact that we don't know at build time what machine 
> > the kernel will be used on, and therefore what address to use.  This is 
> 
> But instead of letting the boot loader who has the detailled knowledge
> about which memory is actually present on the target system you build
> a non-trivial preloader and insist that the ultimate way is to wrape
> the kernel with it.

Yes, because the kernel still requires to take total control on its 
execution environment, and we are now doing that at run time instead of 
compile time, which does include techniques like self modifying code 
i.e. runtime patching of the kernel binary, etc.

> > why zImage was made totally relocatable and totally position 
> > independent so it can figure out at run time what address to use.  
> > Figuring out the address to use at "make uImage" time only works for a 
> > kernel that will boot on a single specific machine.
> 
> There is no actual reason this has to be that way.  If you want then
> risk a view over the rim of your plate and look how it's working for
> other architectures.  It's not really new problems we're dealing with
> here.  Kernel images that run on a number of different processors and
> boards are not new, and device tree support has been available for
> years, too.  It's not that there is no preexisting experience with
> such issues.
> 
> On Power Architecture systems, we use uImage format and/or FIT image
> format to wrap raw kernel images (without preloader).  If somebody
> wants, he can attach / include the DT blob to that image, and
> eventually also a ramdisk/initrd image.  This is not mandatory, it is
> all optimal.  And it just works.

I'm coming to the conclusion that your world view is different from mine 
then.  Maybe ARM used to be similar to PPC in the past, but ARM is 
moving on and things are changing.  The requirements are changing.  
u-Boot is trying to hold us in the past and making itself more and more 
an obstacle.  The only thing you could do is to merge the provided 
patches which doesn't affect in any way your own view, but would allow 
for u-Boot not to be an obstacle anymore and everybody would be happy.  
Given those patches are 
rather simple and non intrusive, I'm failing to understand why you are 
putting so much resistance in the name of some philosophical agenda or I 
don't know what other reasons.

> > It is already the case, more or less.  The raw image _must_ be loaded at 
> > TEXT_OFFSET from start of RAM, regardless of where that RAM is.  So if 
> > all you care about is raw image kernels, then having a relative a load 
> > address for uImage makes perfect sense.
> 
> Thanks for this clear statement.  So I will apply the patches to add
> such support to U-Boot.

Good.  I've given you confirmation that you can use the kernel in the 
way you want to use it.  I consider it a valid although suboptimal use 
case, but I'm not denying you that possibility.

> > This is a rigid restriction that a well intended bootloader would not 
> > impose all the time if the user wishes to do otherwise.  You might not 
> > be concerned by use cases where we want images to be executed right 
> > where they've been loaded irrespective of what that might be, but that 
> > doesn't make that any less of a valid use case.  And Stephen's patches 
> > are providing just that.
> 
> No, they aren't.
> 
> I already NAKed these patches, and this discussion has made it clear
> to me that this was a correct decision.  What you want is not uImages.

You are therefore denying me the ability to use the kernel according to 
the use case I care about.  Maybe I should reconsider my willingness to 
let you use raw kernel image then?  Because if you are not willing to 
collaborate for the case I care about, why should I care about yours?


Nicolas

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Marek Vasut]

> On Tue, 8 Nov 2011, Wolfgang Denk wrote:
> > In message <alpine.LFD.2.02.1111071840080.3307@xanadu.home> you wrote:
> > > > I understand you are referring here to zImages only. Correct?
> > > 
> > > Correct.  Anything else is not relocatable.
> > > 
> > > > Or will raw images (without the preloader) be fully relocatable, too?
> > > 
> > > No.
> > 
> > OK. So the situation on ARM is basicly not different from what we
> > have on other architectures.

[...]

> > I already NAKed these patches, and this discussion has made it clear
> > to me that this was a correct decision.  What you want is not uImages.
> 
> You are therefore denying me the ability to use the kernel according to
> the use case I care about.  Maybe I should reconsider my willingness to
> let you use raw kernel image then?  Because if you are not willing to
> collaborate for the case I care about, why should I care about yours?
> 
> 
> Nicolas

Nico, will it be OK if we/you implemented "bootz" command to boot zImages? Will 
that be satifying for you? I understand Wolfgang isn't oposed to such solution.

M

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Nicolas Pitre,

In message <alpine.LFD.2.02.1111080847540.3307@xanadu.home> you wrote:
> 
> > In both cases the _kernel_ image is not position independent. It must
> > be loaded to a specific address and started at a specific entry point.
> > The exact information where these are is known at built time, and
> > somehow encoded in the images (here in the image header, there in the
> > preloader code).
> > 
> > Is this summary correct so far?
> 
> No.  Your statement that says "The exact information where these are is 
> known at built time, and somehow encoded in the images (here in the 
> image header, there in the preloader code)" is false.  None of that 
> information is known at build time nor encoded in the preloader as you 
> call it.

Hm...  You wrote in <alpine.LFD.2.02.1111071840080.3307@xanadu.home>:

| The role of the zImage code is to figure out at run time about its 
| position in RAM, deduce where the RAM starts, relocate itself if it is 
| in the way of the decompressed kernel, and decompress the actual kernel 
| in its final location (RAM start + TEXT_OFFSET) without having to 
| relocate the bigger decompressed data. ...

To me this seems as if the "final location (RAM start + TEXT_OFFSET)"
is known at build time - ok, only in form of a _relative_ (to the
start of system RAM) address, but nevertheless.

> > This is not quite correct.  The _kernel_ code has a fixed address.
> > It is the preloader code which can be started from any address.
> 
> The preloader is part of the kernel, whether you like it or not, and it 
> is not going away any time soon.  That would help the conversation if 
> you accepted it as such.

The preloader is a part of the Linux kernel source tree, but wether I
use it with the Linux kernel or not is my own choice.  Or is it
mandatory to use it ?


Why the heck are you just so opinionated and not willing to even
remotely consider that other people have other ideas or needs?

It seems you don't even notice that I don't ever argument against
using zImages - my opionion is if it's useful for some and doesn;t
hurt others so let them use it.

However, your statements have been repeatedly on the edge of being
insulting.


> OK, fine.  I don't really care about uImage if it cannot accommodate our 
> needs.  I thought that the -1 extension was a pretty agile and 
> non-obstrusive way to make u-Boot more flexible and versatile, and not 
> only for Linux loading.  Failing that, can we have support for loading 
---------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> zImage directly then?  There are precedents in the upstream u-Boot for 
--^^^^^^^^^^^^^^^^^^^^^
> doing that on other architectures already.

Let's remember your question here.  Actually I answered it already in
my previous message, explicitly and clear.  I don't understand why you
intentionally ignore it.  See below.

> I never intended to prevent raw images from being used with uImage.  
> This is however not the use case I'm looking after.  That doesn't mean 
> that because I'm interested in a different usage model than yours that 
> I'm going to 
> actively prevent raw Image from being used.  I see obvious limitations 
> with it, but if that is what you want then by all means just use that.  
> We cannot say as much from the u-Boot side for the use case I'm 
> interested in though.

Good.  So there should be room for different implementations that
serve different needs or interests.


> > And what I'm telling you is that you could probably have that for free
> > if you dropped the preloader.
> 
> But that is not what I want.

OK.  It's your decision.


> > I already NAKed these patches, and this discussion has made it clear
> > to me that this was a correct decision.  What you want is not uImages.
> 
> You are therefore denying me the ability to use the kernel according to 
> the use case I care about.  Maybe I should reconsider my willingness to 
> let you use raw kernel image then?  Because if you are not willing to 
> collaborate for the case I care about, why should I care about yours?


May I ask why exactly you cut off here, and NOT comment at all about
the following part I wrote:

| I suggest the following:
| 
| - I will apply Stephen's previous patches to support relative images
|   as they are useful for people who want to use "proper" uImages
|   (containing a raw kernel image as payload) on different boards /
|   architectures.
| 
| - If you want to boot zImages (with the kernel wrapper included and
|   thus fully relocatable), then please feel free and submit patches to
|   add support for booting zImage format in U-Boot.  Then you get what
|   you want, and you can use zImages directly, without the 64 byte
|   uImage header that is only hindering for your usage mode.
| 
| - It would be appreciated if we could get support for real uImages
|   (wrapping a raw kernel image) into Linux, then.
| 
| This way those who want to use zImages can do so, and those who prefer
| a different approach can have their ways, too.


Here I offered you exactly what you are asking for above.

But when I offer it, you just cut it off and continue to complain and
threaden and insult.

You tell me I was not willing to collaborate.

How do you describe your own attitude then?


Anyway.  My proposal is up.


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
In C we had to code our own bugs, in C++ we can inherit them.

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Detlev Zundel]

Hello Wolfgang and Nicolas,

please allow me to barge in at that point.

As I strongly believe that we all want to advance our software in a
technical sense and not spend time in flame wars - I am trying to think
of ways forward from the current state of affairs.

Without evaluating all the arguments individually, let me pick up
Wolfgangs latest suggestion which indeed seems like a pragmatic way
forward:

> | I suggest the following:
> | 
> | - I will apply Stephen's previous patches to support relative images
> |   as they are useful for people who want to use "proper" uImages
> |   (containing a raw kernel image as payload) on different boards /
> |   architectures.
> | 
> | - If you want to boot zImages (with the kernel wrapper included and
> |   thus fully relocatable), then please feel free and submit patches to
> |   add support for booting zImage format in U-Boot.  Then you get what
> |   you want, and you can use zImages directly, without the 64 byte
> |   uImage header that is only hindering for your usage mode.
> | 
> | - It would be appreciated if we could get support for real uImages
> |   (wrapping a raw kernel image) into Linux, then.
> | 
> | This way those who want to use zImages can do so, and those who prefer
> | a different approach can have their ways, too.

My hope is that we can re-start the discussion from this - which
actually looks like a consensus.

For the unlikely but probable case of further disagreement, I propose a
vote on the mailing list as an "emergency tie breaker".  As we
previously did not need to resort to such a measure, we do not have a
formal procedure ready for such a thing.  Thinking about it a little
while, it would probably make sense to have people vote on the issue
that demonstrably care about the software and have invested substantial
effort in it.  A natural choice for such a set people are of course the
custodians.  So I would suggest to instantiate a poll among the
custodians as such a "tie breaker" if needed.

What do you think - is this a way forward?

Thanks
  Detlev

-- 
Perfecting oneself is as much unlearning as it is learning. 
                                        -- Edsger Dijkstra 
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Nicolas Pitre]

On Tue, 8 Nov 2011, Wolfgang Denk wrote:

> Dear Nicolas Pitre,
> 
> In message <alpine.LFD.2.02.1111080847540.3307@xanadu.home> you wrote:
> > 
> > > In both cases the _kernel_ image is not position independent. It must
> > > be loaded to a specific address and started at a specific entry point.
> > > The exact information where these are is known at built time, and
> > > somehow encoded in the images (here in the image header, there in the
> > > preloader code).
> > > 
> > > Is this summary correct so far?
> > 
> > No.  Your statement that says "The exact information where these are is 
> > known at built time, and somehow encoded in the images (here in the 
> > image header, there in the preloader code)" is false.  None of that 
> > information is known at build time nor encoded in the preloader as you 
> > call it.
> 
> Hm...  You wrote in <alpine.LFD.2.02.1111071840080.3307@xanadu.home>:
> 
> | The role of the zImage code is to figure out at run time about its 
> | position in RAM, deduce where the RAM starts, relocate itself if it is 
> | in the way of the decompressed kernel, and decompress the actual kernel 
> | in its final location (RAM start + TEXT_OFFSET) without having to 
> | relocate the bigger decompressed data. ...
> 
> To me this seems as if the "final location (RAM start + TEXT_OFFSET)"
> is known at build time - ok, only in form of a _relative_ (to the
> start of system RAM) address, but nevertheless.

That is correct.  We used to encode RAM start in the kernel as well, but 
that part is now runtime determined, based on the position of the code.

> > > This is not quite correct.  The _kernel_ code has a fixed address.
> > > It is the preloader code which can be started from any address.
> > 
> > The preloader is part of the kernel, whether you like it or not, and it 
> > is not going away any time soon.  That would help the conversation if 
> > you accepted it as such.
> 
> The preloader is a part of the Linux kernel source tree, but wether I
> use it with the Linux kernel or not is my own choice.  Or is it
> mandatory to use it ?

No, it is not.

> Why the heck are you just so opinionated and not willing to even
> remotely consider that other people have other ideas or needs?

Unfortunately I feel the same way towards you.  however I never 
dismissed the fact that other people have different ideas or needs.  
I'm simply trying to convince you that I (and others) have other ideas 
or needs.  But this is going in circles.

> It seems you don't even notice that I don't ever argument against
> using zImages - my opionion is if it's useful for some and doesn;t
> hurt others so let them use it.

I'm very glad to know that.

> > OK, fine.  I don't really care about uImage if it cannot accommodate our 
> > needs.  I thought that the -1 extension was a pretty agile and 
> > non-obstrusive way to make u-Boot more flexible and versatile, and not 
> > only for Linux loading.  Failing that, can we have support for loading 
> ---------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > zImage directly then?  There are precedents in the upstream u-Boot for 
> --^^^^^^^^^^^^^^^^^^^^^
> > doing that on other architectures already.
> 
> Let's remember your question here.  Actually I answered it already in
> my previous message, explicitly and clear.  I don't understand why you
> intentionally ignore it.  See below.

This wasn't intentional.  A serious oversight on my part for sure.  In 
which case I apologize for missing your suggestion.

> > I never intended to prevent raw images from being used with uImage.  
> > This is however not the use case I'm looking after.  That doesn't 
> > mean that because I'm interested in a different usage model than 
> > yours that I'm going to actively prevent raw Image from being used.  
> > I see obvious limitations with it, but if that is what you want then 
> > by all means just use that.  We cannot say as much from the u-Boot 
> > side for the use case I'm interested in though.
> 
> Good.  So there should be room for different implementations that
> serve different needs or interests.

Indeed!

> | I suggest the following:
> | 
> | - I will apply Stephen's previous patches to support relative images
> |   as they are useful for people who want to use "proper" uImages
> |   (containing a raw kernel image as payload) on different boards /
> |   architectures.
> | 
> | - If you want to boot zImages (with the kernel wrapper included and
> |   thus fully relocatable), then please feel free and submit patches to
> |   add support for booting zImage format in U-Boot.  Then you get what
> |   you want, and you can use zImages directly, without the 64 byte
> |   uImage header that is only hindering for your usage mode.

Good Lord!  I definitively missed this.  Per a live conversation with 
Stephen, I was under the impression this would have been a quite 
intrusive change, and my impression is that such things were attempted 
in the past with no success.  I'm definitively in favor of such a 
solution.

> | - It would be appreciated if we could get support for real uImages
> |   (wrapping a raw kernel image) into Linux, then.

That certainly can be done, ideally in an architecture independent way. 
A dedicated kconfig menu for u-Boot options and flavors could be common 
to all architectures, and this could then be maintained outside of the 
ARM directory.  Someone would have to step forward and volunteer for 
that role.


Nicolas

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Stephen Warren]

(Sigh, resending again to avoid rejected MIME encoding)

On 11/07/2011 01:26 PM, Wolfgang Denk wrote:
> Dear Stephen Warren,
> 
> In message <74CDBE0F657A3D45AFBB94109FB122FF173F9A5035@HQMAIL01.nvidia.com> you wrote:
>>
>>> Your own IH_TYPE_*_REL patches are queued and will be merged soon.
>>
>> Oh. I kept pushing and pushing on these and kept meeting resistance. I
> 
> There was no resistance ever.  There were just the normal review
> comments.
> 
>> had absolutely no idea at all that there was agreement over those patches;
>> the reviews just stopped happening after you refused to look at them
> 
> If there are no further complaints that usually menas the stuff is
> sitting in the queue waiting to be processed.  Sorry, but my bandwidth
> _is_ limited.
> 
>> Anyway, I have withdrawn my support for those patches; please don't apply
>> them. In my opinion, this new solution is far superior because:
> 
> Argh... So we are back at square one.
> 
> The problem with this new approach is that Linux kernel images are NOT
> freely relocatable.  They do have a fix entry point, even if this is
> not an absolute address, but a relative one.

That's simply not true ARM Linux zImages /are/ relocatable - as far as
I'm aware, they can run from anywhere in SDRAM. I've certainly tested
loading ARM zImages at a few random locations and it works fine, and
I've been told by senior Linux kernel people that this is intentional.
(Well, there is a restriction that the image must be somewhere within
the first 128M of RAM for PATCH_PHYS_VIRT(?) to operate correctly, but
I'd still count that as been almost arbitrarily relocatable)

> The natural way to
> handle this is exactly that:  add support for images with relative
> )offset based) load and entry point addresses.
> 
> Your new approahc is indeed simpler - actually it is too simplistic,
> as you cannot record load address / entry point information any more.
> It may work when using the kernel wrapper - but what if you don't want
> to do that?

Note that the only thing that made my IH_TYPE_*_REL work was this exact
same runs-at-any-location feature of ARM zImages; there was never
anything in the kernel that allowed it to solely run at "some specific
offset from the base of SDRAM"; that was just the first way I thought of
modifying U-Boot to enable this current feature.

-- 
nvpublic

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Nicolas Pitre]

On Mon, 7 Nov 2011, Stephen Warren wrote:

> (Sigh, resending again to avoid rejected MIME encoding)
> 
> On 11/07/2011 01:26 PM, Wolfgang Denk wrote:
> > Dear Stephen Warren,
> > 
> > In message <74CDBE0F657A3D45AFBB94109FB122FF173F9A5035@HQMAIL01.nvidia.com> you wrote:
> >> Anyway, I have withdrawn my support for those patches; please don't apply
> >> them. In my opinion, this new solution is far superior because:
> > 
> > Argh... So we are back at square one.
> > 
> > The problem with this new approach is that Linux kernel images are NOT
> > freely relocatable.  They do have a fix entry point, even if this is
> > not an absolute address, but a relative one.

So what?  The Linux entry point is always fixed.  Relative to the image 
payload, it must always be 0.

> That's simply not true ARM Linux zImages /are/ relocatable - as far as
> I'm aware, they can run from anywhere in SDRAM. I've certainly tested
> loading ARM zImages at a few random locations and it works fine, and
> I've been told by senior Linux kernel people that this is intentional.

Exact.

> > The natural way to
> > handle this is exactly that:  add support for images with relative
> > )offset based) load and entry point addresses.
> > 
> > Your new approahc is indeed simpler - actually it is too simplistic,
> > as you cannot record load address / entry point information any more.
> > It may work when using the kernel wrapper - but what if you don't want
> > to do that?

Only the kernel zImage is relocatable.  So only the kernel 
zImage should be wrapped into a uImage using the -1 load address flag.  
And this solution allows for zImage to be loaded wherever we want 
_after_ the uImage encapsulation.

So yes, this is a simplistic solution, but it is damn good, and it 
solves the u-Boot restrictions we've been complaining about for at least 
two years now.

Therefore, can this patch be merged now?  I've provided my ACK for it 
already.

Thank you.


Nicolas

[U-Boot] [PATCH v2 3/3] image: Allow images to indicate they're loadable at any address

[Wolfgang Denk]

Dear Nicolas,

In message <alpine.LFD.2.02.1111071633330.3307@xanadu.home> you wrote:
> 
> So yes, this is a simplistic solution, but it is damn good, and it 
> solves the u-Boot restrictions we've been complaining about for at least 
> two years now.

Could you please explain which of these restrictions cannot be solved
by using the IH_TYPE_*_REL approach?

I just explained (see
http://thread.gmane.org/gmane.comp.boot-loaders.u-boot/115774/focus=115881)
why I consider this a better approch than Stephen's new patches.


There are other reasons why we do want to have support for uImage
format.  Just one example: the length information in the image header
comes in very handy when you go for very fast booting systems - here
the U-Boot SPL code can load and start Linux directly, bypassing all
the normale U-Boot loading and initialization.  And you can boot the
very same image normally as well.  With zImage, you would again have
to provide additional information, probably be defining yet another
header format.  OK, such super-fast booting systems are only a tiny
fraction of the systems in the field, but...

> Therefore, can this patch be merged now?  I've provided my ACK for it 
> already.

And I NAKed it already.  Twice, actually.

I am still willing to reconsider this NAK, but I am asking for really
convincing arguments why the IH_TYPE_*_REL approach would be less
useful.

Thanks.

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
Misquotation is, in fact, the pride and privilege of the  learned.  A
widely-read  man  never  quotes  accurately,  for  the rather obvious
reason that he has read too widely.
                - Hesketh Pearson _Common Misquotations_ introduction

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Marek Vasut]

> image_get_ram_disk() and image_get_kernel() perform operations in a
> consistent order. Modify image_get_fdt() to do things the same way.
> This allows a later change to insert some image header manipulations
> into these three functions in a consistent fashion.
> 
> v2: New patch
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>

Hi Stephen,

this patchset is good and all, but can we not also introduce cmd_zload to load 
zImages? Wolfgang, today's ARM hardware will really benefit from that, uImage 
holds us back a lot these days. Other option is to extend cmd_bootm() to load 
zImages.

Cheers

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Simon Glass]

Hi,

On Sat, Nov 5, 2011 at 11:41 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> image_get_ram_disk() and image_get_kernel() perform operations in a
>> consistent order. Modify image_get_fdt() to do things the same way.
>> This allows a later change to insert some image header manipulations
>> into these three functions in a consistent fashion.
>>
>> v2: New patch
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>
> Hi Stephen,
>
> this patchset is good and all, but can we not also introduce cmd_zload to load
> zImages? Wolfgang, today's ARM hardware will really benefit from that, uImage
> holds us back a lot these days. Other option is to extend cmd_bootm() to load
> zImages.
>
> Cheers
>

Just a quick Q. What is the ultimate intent here? Should we be aiming
to have U-Boot copy and decompress the data into RAM ready for Linux?
In theory this should be slightly faster since U-Boot already has the
data in its cache. I think zImage now supports having an FDT inside
but what is the advantage of zImage over a uImage with compressed
portions?

Regards,
Simon

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Marek Vasut]

> Hi,
> 
> On Sat, Nov 5, 2011 at 11:41 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> image_get_ram_disk() and image_get_kernel() perform operations in a
> >> consistent order. Modify image_get_fdt() to do things the same way.
> >> This allows a later change to insert some image header manipulations
> >> into these three functions in a consistent fashion.
> >> 
> >> v2: New patch
> >> 
> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> > 
> > Hi Stephen,
> > 
> > this patchset is good and all, but can we not also introduce cmd_zload to
> > load zImages? Wolfgang, today's ARM hardware will really benefit from
> > that, uImage holds us back a lot these days. Other option is to extend
> > cmd_bootm() to load zImages.
> > 
> > Cheers
> 
> Just a quick Q. What is the ultimate intent here? Should we be aiming
> to have U-Boot copy and decompress the data into RAM ready for Linux?

Nope, not at all. We have a problem with booting linux images which support 
multiple different SoCs (because the RAM might be elsewhere for different SoCs).

> In theory this should be slightly faster since U-Boot already has the
> data in its cache. I think zImage now supports having an FDT inside
> but what is the advantage of zImage over a uImage with compressed
> portions?

That's not the point. We need to load FDT, load zImage, setup the regs and boot 
it from where we load the zImage. The uImage envelope contains fixed address to 
where the kernel image is loaded, which interferes with kernel's runtime 
patching of the kernel base address (zreladdr).

Basically kernel can be loaded to address A1 on one SoC, address A2 on different 
SoC, but in the old times, the kernel had to be linked to a predefined address. 
That's not true anymore. Now if kernel is loaded to address A1, it adjusts 
itself and runs from A1, so for A2 etc.

uImage blocks this because it forces u-boot to copy zImage to fixed address.
> 
> Regards,
> Simon

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Simon Glass]

Hi Marek,

On Sat, Nov 5, 2011 at 12:39 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> Hi,
>>
>> On Sat, Nov 5, 2011 at 11:41 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> >> image_get_ram_disk() and image_get_kernel() perform operations in a
>> >> consistent order. Modify image_get_fdt() to do things the same way.
>> >> This allows a later change to insert some image header manipulations
>> >> into these three functions in a consistent fashion.
>> >>
>> >> v2: New patch
>> >>
>> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> >
>> > Hi Stephen,
>> >
>> > this patchset is good and all, but can we not also introduce cmd_zload to
>> > load zImages? Wolfgang, today's ARM hardware will really benefit from
>> > that, uImage holds us back a lot these days. Other option is to extend
>> > cmd_bootm() to load zImages.
>> >
>> > Cheers
>>
>> Just a quick Q. What is the ultimate intent here? Should we be aiming
>> to have U-Boot copy and decompress the data into RAM ready for Linux?
>
> Nope, not at all. We have a problem with booting linux images which support
> multiple different SoCs (because the RAM might be elsewhere for different SoCs).
>
>> In theory this should be slightly faster since U-Boot already has the
>> data in its cache. I think zImage now supports having an FDT inside
>> but what is the advantage of zImage over a uImage with compressed
>> portions?
>
> That's not the point. We need to load FDT, load zImage, setup the regs and boot
> it from where we load the zImage. The uImage envelope contains fixed address to
> where the kernel image is loaded, which interferes with kernel's runtime
> patching of the kernel base address (zreladdr).
>
> Basically kernel can be loaded to address A1 on one SoC, address A2 on different
> SoC, but in the old times, the kernel had to be linked to a predefined address.
> That's not true anymore. Now if kernel is loaded to address A1, it adjusts
> itself and runs from A1, so for A2 etc.
>
> uImage blocks this because it forces u-boot to copy zImage to fixed address.

Stephen's patch set should fix that by allowing an unspecified load address .

So this means that we are fine if we use a zImage, but what about a
uImage? Are we giving up on that altogether? Stephen's original patch
on this subject seemed to me to solve the problem with uImage (the one
he had all the code size grief with).

Can't we commit both of Stephen's patches? It seems to me that they
solve different problems.

Regards,
Simon

>>
>> Regards,
>> Simon
>

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Marek Vasut]

> Hi Marek,
> 
> On Sat, Nov 5, 2011 at 12:39 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> Hi,
> >> 
> >> On Sat, Nov 5, 2011 at 11:41 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> >> >> image_get_ram_disk() and image_get_kernel() perform operations in a
> >> >> consistent order. Modify image_get_fdt() to do things the same way.
> >> >> This allows a later change to insert some image header manipulations
> >> >> into these three functions in a consistent fashion.
> >> >> 
> >> >> v2: New patch
> >> >> 
> >> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >> > 
> >> > Hi Stephen,
> >> > 
> >> > this patchset is good and all, but can we not also introduce cmd_zload
> >> > to load zImages? Wolfgang, today's ARM hardware will really benefit
> >> > from that, uImage holds us back a lot these days. Other option is to
> >> > extend cmd_bootm() to load zImages.
> >> > 
> >> > Cheers
> >> 
> >> Just a quick Q. What is the ultimate intent here? Should we be aiming
> >> to have U-Boot copy and decompress the data into RAM ready for Linux?
> > 
> > Nope, not at all. We have a problem with booting linux images which
> > support multiple different SoCs (because the RAM might be elsewhere for
> > different SoCs).
> > 
> >> In theory this should be slightly faster since U-Boot already has the
> >> data in its cache. I think zImage now supports having an FDT inside
> >> but what is the advantage of zImage over a uImage with compressed
> >> portions?
> > 
> > That's not the point. We need to load FDT, load zImage, setup the regs
> > and boot it from where we load the zImage. The uImage envelope contains
> > fixed address to where the kernel image is loaded, which interferes with
> > kernel's runtime patching of the kernel base address (zreladdr).
> > 
> > Basically kernel can be loaded to address A1 on one SoC, address A2 on
> > different SoC, but in the old times, the kernel had to be linked to a
> > predefined address. That's not true anymore. Now if kernel is loaded to
> > address A1, it adjusts itself and runs from A1, so for A2 etc.
> > 
> > uImage blocks this because it forces u-boot to copy zImage to fixed
> > address.
> 
> Stephen's patch set should fix that by allowing an unspecified load address
> .
> 
> So this means that we are fine if we use a zImage, but what about a
> uImage? Are we giving up on that altogether? Stephen's original patch
> on this subject seemed to me to solve the problem with uImage (the one
> he had all the code size grief with).

I'd be more open to adding some kind of a flag to uImage, rather than using 
address 0xffffffff. For this is quite fragile and seems a lot like a hack.
> 
> Can't we commit both of Stephen's patches? It seems to me that they
> solve different problems.

Yes, 1/3 and 2/3 look good.
> 
> Regards,
> Simon
> 
> >> Regards,
> >> Simon

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Simon Glass]

Hi Marek,

On Sat, Nov 5, 2011 at 1:41 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> Hi Marek,
>>
>> On Sat, Nov 5, 2011 at 12:39 PM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> >> Hi,
>> >>
>> >> On Sat, Nov 5, 2011 at 11:41 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> >> >> image_get_ram_disk() and image_get_kernel() perform operations in a
>> >> >> consistent order. Modify image_get_fdt() to do things the same way.
>> >> >> This allows a later change to insert some image header manipulations
>> >> >> into these three functions in a consistent fashion.
>> >> >>
>> >> >> v2: New patch
>> >> >>
>> >> >> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> >> >
>> >> > Hi Stephen,
>> >> >
>> >> > this patchset is good and all, but can we not also introduce cmd_zload
>> >> > to load zImages? Wolfgang, today's ARM hardware will really benefit
>> >> > from that, uImage holds us back a lot these days. Other option is to
>> >> > extend cmd_bootm() to load zImages.
>> >> >
>> >> > Cheers
>> >>
>> >> Just a quick Q. What is the ultimate intent here? Should we be aiming
>> >> to have U-Boot copy and decompress the data into RAM ready for Linux?
>> >
>> > Nope, not at all. We have a problem with booting linux images which
>> > support multiple different SoCs (because the RAM might be elsewhere for
>> > different SoCs).
>> >
>> >> In theory this should be slightly faster since U-Boot already has the
>> >> data in its cache. I think zImage now supports having an FDT inside
>> >> but what is the advantage of zImage over a uImage with compressed
>> >> portions?
>> >
>> > That's not the point. We need to load FDT, load zImage, setup the regs
>> > and boot it from where we load the zImage. The uImage envelope contains
>> > fixed address to where the kernel image is loaded, which interferes with
>> > kernel's runtime patching of the kernel base address (zreladdr).
>> >
>> > Basically kernel can be loaded to address A1 on one SoC, address A2 on
>> > different SoC, but in the old times, the kernel had to be linked to a
>> > predefined address. That's not true anymore. Now if kernel is loaded to
>> > address A1, it adjusts itself and runs from A1, so for A2 etc.
>> >
>> > uImage blocks this because it forces u-boot to copy zImage to fixed
>> > address.
>>
>> Stephen's patch set should fix that by allowing an unspecified load address
>> .
>>
>> So this means that we are fine if we use a zImage, but what about a
>> uImage? Are we giving up on that altogether? Stephen's original patch
>> on this subject seemed to me to solve the problem with uImage (the one
>> he had all the code size grief with).
>
> I'd be more open to adding some kind of a flag to uImage, rather than using
> address 0xffffffff. For this is quite fragile and seems a lot like a hack.

Possibly, but at least it makes it very clear that the load address
should not be used.

>>
>> Can't we commit both of Stephen's patches? It seems to me that they
>> solve different problems.
>
> Yes, 1/3 and 2/3 look good.

Actually I meant both series, sorry. The previous series enhanced
uImage to understand an image that can go anywhere, and I think that
is useful also. In both series, a load address of -1 means 'ignore
it'.

Regards,
Simon


>>
>> Regards,
>> Simon
>>
>> >> Regards,
>> >> Simon
>

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Wolfgang Denk]

Dear Marek Vasut,

In message <201111052141.27086.marek.vasut@gmail.com> you wrote:
>
> I'd be more open to adding some kind of a flag to uImage, rather than using 
> address 0xffffffff. For this is quite fragile and seems a lot like a hack.

We don't want to have 0xffffffff.

The agreement was to support relative imagaes.

See
10/18 Stephen Warren     [PATCH v2 REPOST 1/3] [COSMETIC] checkpatch whitespace cleanups
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/113093
10/18 Stephen Warren     [PATCH v2 REPOST 2/3] image: Implement IH_TYPE_KERNEL_REL
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/113079
10/18 Stephen Warren     [PATCH v2 REPOST 3/3] tegra2: Enable CONFIG_SYS_RELATIVE_IMAGES
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/113080


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
After a heated argument on some trivial matter Nancy [Astor]  .  .  .
shouted,  ``If  I were your wife I would put poison in your coffee!''
Whereupon Winston Churchill with equal heat and  sincerity  answered,
``And if I were your husband I would drink it.''

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Marek Vasut]

> Dear Marek Vasut,
> 
> In message <201111052141.27086.marek.vasut@gmail.com> you wrote:
> > I'd be more open to adding some kind of a flag to uImage, rather than
> > using address 0xffffffff. For this is quite fragile and seems a lot like
> > a hack.
> 
> We don't want to have 0xffffffff.
> 
> The agreement was to support relative imagaes.
> 
> See
> 10/18 Stephen Warren     [PATCH v2 REPOST 1/3] [COSMETIC] checkpatch
> whitespace cleanups
> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/113093
> 10/18 Stephen Warren     [PATCH v2 REPOST 2/3] image: Implement
> IH_TYPE_KERNEL_REL
> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/113079
> 10/18 Stephen Warren     [PATCH v2 REPOST 3/3] tegra2: Enable
> CONFIG_SYS_RELATIVE_IMAGES
> http://article.gmane.org/gmane.comp.boot-loaders.u-boot/113080
> 
> 
> Best regards,
> 
> Wolfgang Denk

Ok I see, I definitelly missed those. Thanks

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Wolfgang Denk]

Dear Marek Vasut,

In message <201111052039.34646.marek.vasut@gmail.com> you wrote:
>
> > Just a quick Q. What is the ultimate intent here? Should we be aiming
> > to have U-Boot copy and decompress the data into RAM ready for Linux?
> 
> Nope, not at all. We have a problem with booting linux images which support 
> multiple different SoCs (because the RAM might be elsewhere for different SoCs).

You are wrong.  The agreement was to allow for addresses (load
address, entry point address) that are relative to the start of system
RAM.

> That's not the point. We need to load FDT, load zImage, setup the regs and boot 
> it from where we load the zImage. The uImage envelope contains fixed address to 
> where the kernel image is loaded, which interferes with kernel's runtime 
> patching of the kernel base address (zreladdr).

You ignore the discussions and proposals that were made.

> uImage blocks this because it forces u-boot to copy zImage to fixed address.

Nonsense.

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
Very ugly or very beautiful women should be flattered on their
understanding, and mediocre ones on their beauty.
                                       -- Philip Earl of Chesterfield

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ0bbhvZ1VPwY=0y9YZJdf+EOBgyKKVPFuAkBqy2z4L3HA@mail.gmail.com> you wrote:
> 
> Just a quick Q. What is the ultimate intent here? Should we be aiming
> to have U-Boot copy and decompress the data into RAM ready for Linux?

Yes.

Rigth from the beginning of PPCBoot / U-Boot we designed it that
U-Boot would do all needed steps to verify, load and uncompress an
image.  It make no sense to attach the uncompression and loading code
to each and every image, and to download it and store it again and
again and again.  This works really well for example on Power, only
ARM is one of the examples where the PTB never bothered to acquaint
themself with ideas that went beyond the capabilities of Blob or
similar boot loaders.

> In theory this should be slightly faster since U-Boot already has the
> data in its cache. I think zImage now supports having an FDT inside
> but what is the advantage of zImage over a uImage with compressed
> portions?

There is none.  Also, there is no advantage in attaching the DT blob
to the Linux image/. This is only of use to braindead boot loaders.

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
A stone was placed at a ford in a river with the inscription:
"When this stone is covered it is dangerous to ford here."

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Marek Vasut]

> Dear Simon Glass,
> 
> In message 
<CAPnjgZ0bbhvZ1VPwY=0y9YZJdf+EOBgyKKVPFuAkBqy2z4L3HA@mail.gmail.com> you wrote:
> > Just a quick Q. What is the ultimate intent here? Should we be aiming
> > to have U-Boot copy and decompress the data into RAM ready for Linux?
> 
> Yes.
> 
> Rigth from the beginning of PPCBoot / U-Boot we designed it that
> U-Boot would do all needed steps to verify, load and uncompress an
> image.  It make no sense to attach the uncompression and loading code
> to each and every image, and to download it and store it again and
> again and again.  This works really well for example on Power, only
> ARM is one of the examples where the PTB never bothered to acquaint
> themself with ideas that went beyond the capabilities of Blob or
> similar boot loaders.

Right, there's no negotiation between linux and uboot on this topic. I think 
this is going on for ages now.

> 
> > In theory this should be slightly faster since U-Boot already has the
> > data in its cache. I think zImage now supports having an FDT inside
> > but what is the advantage of zImage over a uImage with compressed
> > portions?
> 
> There is none.  Also, there is no advantage in attaching the DT blob
> to the Linux image/. This is only of use to braindead boot loaders.
> 
> Best regards,
> 
> Wolfgang Denk

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Wolfgang Denk]

Dear Marek Vasut,

In message <201111052306.47844.marek.vasut@gmail.com> you wrote:
>
> Right, there's no negotiation between linux and uboot on this topic. I think 
> this is going on for ages now.

What kind of "negotiation" do you have in mind when patches to add
such Linux make targets get routinely rejected?

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
The complexity of software is an essential property, not an  acciden-
tal  one. Hence, descriptions of a software entity that abstract away
its complexity often abstract away its essence.    - Fred Brooks, Jr.

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Simon Glass]

Hi Wolfgang,

On Sat, Nov 5, 2011 at 2:53 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Simon Glass,
>
> In message <CAPnjgZ0bbhvZ1VPwY=0y9YZJdf+EOBgyKKVPFuAkBqy2z4L3HA@mail.gmail.com> you wrote:
>>
>> Just a quick Q. What is the ultimate intent here? Should we be aiming
>> to have U-Boot copy and decompress the data into RAM ready for Linux?
>
> Yes.
>
> Rigth from the beginning of PPCBoot / U-Boot we designed it that
> U-Boot would do all needed steps to verify, load and uncompress an
> image. ?It make no sense to attach the uncompression and loading code
> to each and every image, and to download it and store it again and
> again and again. ?This works really well for example on Power, only
> ARM is one of the examples where the PTB never bothered to acquaint
> themself with ideas that went beyond the capabilities of Blob or
> similar boot loaders.

OK, it makes sense to me. If U-Boot is doing some unpacking it should
do it all IMO. It is supposed to be the boot loader, not half a boot
loader. Perhaps for other boot loaders the situation is different.

>
>> In theory this should be slightly faster since U-Boot already has the
>> data in its cache. I think zImage now supports having an FDT inside
>> but what is the advantage of zImage over a uImage with compressed
>> portions?
>
> There is none. ?Also, there is no advantage in attaching the DT blob
> to the Linux image/. This is only of use to braindead boot loaders.

If so can you please point me to it?

Searching for "DTB append ARM zImage" provides lots of recent
activity. The justification for this series seems to be old boot
loaders. Is there a LKML thread about why it should/shouldn't be done
in U-Boot specifically - other boot loaders may require the kernel to
do it, but for U-Boot there would need to be some sort of reason I
think.

I am currently using a hybrid solution (U-Boot loads the zImage and
DTB, then kernel decompresses zImage) and we have discussed changing
this. It may need a new build target in the kernel, but not rocket
science.

Regards,
Simon

>
> 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
> A stone was placed at a ford in a river with the inscription:
> "When this stone is covered it is dangerous to ford here."
>

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ2mKwMyYVyGyYwMm8dNMfNPwgbJA+YTH_G1X-uzbkDfpQ@mail.gmail.com> you wrote:
> 
> Searching for "DTB append ARM zImage" provides lots of recent
> activity. The justification for this series seems to be old boot
> loaders. ...

True.  In my understanding, this is only relevant to less capable boot
loaders (which not always are old - things like UEFI are not much
better in this respect).  But U-Boot has full DT support, and we are
even on the way to configure U-Boot itself through a DT, so we should
at least optionally be able to use these features.

>      ... Is there a LKML thread about why it should/shouldn't be done
> in U-Boot specifically - other boot loaders may require the kernel to
> do it, but for U-Boot there would need to be some sort of reason I
> think.

I cannot see a single reason for such format in U-Boot context.  We
have been using the device tree for years on PPC, and never had any
such need.  Dt is actually standard technology now.  It's only new for
ARM, like all the other stuff as FPU support, cache coherency, PCI,
SMP,  ... you name it.

> I am currently using a hybrid solution (U-Boot loads the zImage and
> DTB, then kernel decompresses zImage) and we have discussed changing
> this. It may need a new build target in the kernel, but not rocket
> science.

Patches for this have been submitted (and rejected) several times,
years ago. Ditto for other useful things (like being able to pass the
kernel a ramdisk / initrd address in NOR flash, so we can avoid
copying it into RAM first).  Search the ML archives if you are
interesteed.

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
If a group of N persons implements a COBOL compiler,  there  will  be
N-1 passes. Someone in the group has to be the manager. - T. Cheatham

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Wolfgang Denk]

Dear Marek Vasut,

In message <201111051941.38920.marek.vasut@gmail.com> you wrote:
>
> this patchset is good and all, but can we not also introduce cmd_zload to load 
> zImages? Wolfgang, today's ARM hardware will really benefit from that, uImage 
> holds us back a lot these days. Other option is to extend cmd_bootm() to load 
> zImages.

uImage holds us back? And hardware will benefit from that?

I can't parse that.  Could you please elucidate?

What exactly _is_ the problem of uImage?

Or have you ever considered using a FIT image?

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
But the only way of discovering the limits  of  the  possible  is  to
venture a little way past them into the impossible.
                         - _Profiles of the Future_ (1962; rev. 1973)
                  ``Hazards of Prophecy: The Failure of Imagination''

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Marek Vasut]

> Dear Marek Vasut,
> 
> In message <201111051941.38920.marek.vasut@gmail.com> you wrote:
> > this patchset is good and all, but can we not also introduce cmd_zload to
> > load zImages? Wolfgang, today's ARM hardware will really benefit from
> > that, uImage holds us back a lot these days. Other option is to extend
> > cmd_bootm() to load zImages.
> 
> uImage holds us back? And hardware will benefit from that?

Well you put out quite a good reason in the other mail for uImage. Now the 
problem basically is the negotiation between linux and uboot as of who has to 
wrap the image into what envelope. I don't see this happening actually, which 
sucks.
> 
> I can't parse that.  Could you please elucidate?
> 
> What exactly _is_ the problem of uImage?

The exact problem here is it loads the kernel to fixed address.

> 
> Or have you ever considered using a FIT image?

That looks good, yes.

> 
> Best regards,
> 
> Wolfgang Denk

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Wolfgang Denk]

Dear Marek Vasut,

In message <201111052306.23919.marek.vasut@gmail.com> you wrote:
>
> Well you put out quite a good reason in the other mail for uImage. Now the 
> problem basically is the negotiation between linux and uboot as of who has to 
> wrap the image into what envelope. I don't see this happening actually, which 
> sucks.

Look at the PPC code.  There we have a plain simple uImage make target
in Linux, and everything is fine.  In ARM, we only have a target which
gives us a pre-wrapped image, i. e. which always includes the kernel's
wrapper and decompressor.  How can we change this?  Patches are not
accepted.

> > What exactly _is_ the problem of uImage?
> 
> The exact problem here is it loads the kernel to fixed address.

This has been addressed, and patches are queued.


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
"The POP3 server service depends on the SMTP  server  service,  which
failed to start because of the following error: The operation comple-
ted successfully."                         -- Windows NT Server v3.51

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Marek Vasut]

> Dear Marek Vasut,
> 
> In message <201111052306.23919.marek.vasut@gmail.com> you wrote:
> > Well you put out quite a good reason in the other mail for uImage. Now
> > the problem basically is the negotiation between linux and uboot as of
> > who has to wrap the image into what envelope. I don't see this happening
> > actually, which sucks.
> 
> Look at the PPC code.  There we have a plain simple uImage make target
> in Linux, and everything is fine.  In ARM, we only have a target which
> gives us a pre-wrapped image, i. e. which always includes the kernel's
> wrapper and decompressor.  How can we change this?  Patches are not
> accepted.
> 
> > > What exactly _is_ the problem of uImage?
> > 
> > The exact problem here is it loads the kernel to fixed address.
> 
> This has been addressed, and patches are queued.
> 

I went through the previous discussions, sorry for the noise.
> 
> Best regards,
> 
> Wolfgang Denk

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Loïc Minier]

On Sat, Nov 05, 2011, Marek Vasut wrote:
> this patchset is good and all, but can we not also introduce cmd_zload
> to load zImages? Wolfgang, today's ARM hardware will really benefit
> from that, uImage holds us back a lot these days. Other option is to
> extend cmd_bootm() to load zImages.

 Other architectures like x86 and sh seem to use zboot, perhaps we
 should stick to zboot for all architectures rather than introducing
 bootm -z or bootz?  AFAIK the file formats are slightly different per
 architecture, but the U-Boot cmdline usage would be the same.

-- 
Lo?c Minier

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Wolfgang Denk]

Dear Stephen Warren,

In message <1320164902-24190-1-git-send-email-swarren@nvidia.com> you wrote:
> image_get_ram_disk() and image_get_kernel() perform operations in a
> consistent order. Modify image_get_fdt() to do things the same way.
> This allows a later change to insert some image header manipulations
> into these three functions in a consistent fashion.
> 
> v2: New patch
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  common/image.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/common/image.c b/common/image.c
> index 555d9d9..b773505 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -1131,14 +1131,19 @@ static const image_header_t *image_get_fdt(ulong fdt_addr)
>  {
>  	const image_header_t *fdt_hdr = (const image_header_t *)fdt_addr;
>  
> -	image_print_contents(fdt_hdr);
> +	if (!image_check_magic(fdt_hdr)) {
> +		fdt_error("fdt header bad magic number\n");
> +		return NULL;
> +	}
>  
> -	puts("   Verifying Checksum ... ");
>  	if (!image_check_hcrc(fdt_hdr)) {
>  		fdt_error("fdt header checksum invalid");
>  		return NULL;
>  	}
>  
> +	image_print_contents(fdt_hdr);
> +
> +	puts("   Verifying Checksum ... ");
>  	if (!image_check_dcrc(fdt_hdr)) {
>  		fdt_error("fdt checksum invalid");
>  		return NULL;

The rule in U-Boot when generating output is to print a message
before you start an action, and then either print an OK or an error
message.  The reason for this is debug support: if neither an OK nor
an error comes you know that the test somehow crashed.

Here this principle is violated as image_check_magic() and
image_check_hcrc() will run without being announced.

Please move the output so we get a message printed before starting to
perform the actual tests.

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
Old programmers never die, they just branch to a new address.

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Stephen Warren]

(Resending due to MIME encoding last time. Sorry; I really have to stop
using Outlook for this list for some reason)

On 11/08/2011 09:06 AM, Wolfgang Denk wrote:
> In message <1320164902-24190-1-git-send-email-swarren@nvidia.com> you wrote:
>> image_get_ram_disk() and image_get_kernel() perform operations in a
>> consistent order. Modify image_get_fdt() to do things the same way.
>> This allows a later change to insert some image header manipulations
>> into these three functions in a consistent fashion.
...
>> @@ -1131,14 +1131,19 @@ static const image_header_t *image_get_fdt(ulong fdt_addr)
>>  {
>>  	const image_header_t *fdt_hdr = (const image_header_t *)fdt_addr;
>>  
>> -	image_print_contents(fdt_hdr);
>> +	if (!image_check_magic(fdt_hdr)) {
>> +		fdt_error("fdt header bad magic number\n");
>> +		return NULL;
>> +	}
>>  
>> -	puts("   Verifying Checksum ... ");
>>  	if (!image_check_hcrc(fdt_hdr)) {
>>  		fdt_error("fdt header checksum invalid");
>>  		return NULL;
>>  	}
>>  
>> +	image_print_contents(fdt_hdr);
>> +
>> +	puts("   Verifying Checksum ... ");
>>  	if (!image_check_dcrc(fdt_hdr)) {
>>  		fdt_error("fdt checksum invalid");
>>  		return NULL;
> 
> The rule in U-Boot when generating output is to print a message
> before you start an action, and then either print an OK or an error
> message.  The reason for this is debug support: if neither an OK nor
> an error comes you know that the test somehow crashed.
> 
> Here this principle is violated as image_check_magic() and
> image_check_hcrc() will run without being announced.
> 
> Please move the output so we get a message printed before starting to
> perform the actual tests.

The new code is exactly the same as the existing image_get_kernel() and
image_get_ramdisk(). Are those wrong? I wouldn't want to fix my patch to
conform to some supposed standard when the existing code that's been
accepted doesn't conform to that standard, or would I be responsible for
fixing up that too?

But anyway, I imagine there's no point discussing this patch further,
because its sole purpose is to support the uImage load_address=-1 patch
that follows, and it's pretty clear that won't be accepted, so please
consider this patch series withdrawn too.

-- 
nvpublic

[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}

[Wolfgang Denk]

Dear Stephen Warren,

In message <74CDBE0F657A3D45AFBB94109FB122FF173F9A5415@HQMAIL01.nvidia.com> you wrote:
> Wolfgang Denk wrote at Tuesday, November 08, 2011 9:07 AM:
> > In message <1320164902-24190-1-git-send-email-swarren@nvidia.com> you wrote:
> > > image_get_ram_disk() and image_get_kernel() perform operations in a
> > > consistent order. Modify image_get_fdt() to do things the same way.
> > > This allows a later change to insert some image header manipulations
> > > into these three functions in a consistent fashion.
> ...
> > > @@ -1131,14 +1131,19 @@ static const image_header_t *image_get_fdt(ulong fdt_addr)
> > >  {
> > >  	const image_header_t *fdt_hdr = (const image_header_t *)fdt_addr;
> > >
> > > -	image_print_contents(fdt_hdr);
> > > +	if (!image_check_magic(fdt_hdr)) {
> > > +		fdt_error("fdt header bad magic number\n");
> > > +		return NULL;
> > > +	}
> > >
> > > -	puts("   Verifying Checksum ... ");
> > >  	if (!image_check_hcrc(fdt_hdr)) {
> > >  		fdt_error("fdt header checksum invalid");
> > >  		return NULL;
> > >  	}
> > >
> > > +	image_print_contents(fdt_hdr);
> > > +
> > > +	puts("   Verifying Checksum ... ");
> > >  	if (!image_check_dcrc(fdt_hdr)) {
> > >  		fdt_error("fdt checksum invalid");
> > >  		return NULL;
...
> > The rule in U-Boot when generating output is to print a message
> > before you start an action, and then either print an OK or an error
> > message.  The reason for this is debug support: if neither an OK nor
> > an error comes you know that the test somehow crashed.
...
> The new code is exactly the same as the existing image_get_kernel() and
> image_get_ramdisk(). Are those wrong? I wouldn't want to fix my patch to
> conform to some supposed standard when the existing code that's been
> accepted doesn't conform to that standard, or would I be responsible for
> fixing up that too?

The new code is different from what was there before.

Now we have:

1134         image_print_contents(fdt_hdr);
1135
1136         puts("   Verifying Checksum ... ");
1137         if (!image_check_hcrc(fdt_hdr)) {
1138                 fdt_error("fdt header checksum invalid");
1139                 return NULL;
1140         }
1141
1142         if (!image_check_dcrc(fdt_hdr)) {
1143                 fdt_error("fdt checksum invalid");
1144                 return NULL;

i.e.
		image_print_contents(fdt_hdr);
		puts("   Verifying Checksum ... ");
		image_check_hcrc(fdt_hdr);
		image_check_dcrc(fdt_hdr)

After your patch we have:

		image_check_magic();
		image_check_hcrc()
		image_print_contents(();
		puts("   Verifying Checksum ... ");
		image_check_dcrc();

So before, we have the "Verifying Checksum" before running any of the
image_check_*() functions, while with your patch we run two of them
before that.

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
Intel's new motto: United we stand. Divided we fall!