[U-Boot] [PATCH v2 1/3] image: Make image_get_fdt work like image_get_{ram_disk, kernel}
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;
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;
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
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);
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
[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.
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.
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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
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.
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
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.
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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@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."
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 donein 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
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
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
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
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
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.
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
(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.
participants (5)
-
Loïc Minier -
Marek Vasut -
Simon Glass -
Stephen Warren -
Wolfgang Denk