* [PATCH 0/4] bootm: Handle compressed arm64 images with bootm
@ 2023-11-05 20:03 Simon Glass
2023-11-05 20:03 ` [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload Simon Glass
` (5 more replies)
0 siblings, 6 replies; 21+ messages in thread
From: Simon Glass @ 2023-11-05 20:03 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Marek Vasut, Simon Glass, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Joe Hershberger,
Marc Kleine-Budde, Marek Vasut, Mattijs Korpershoek,
Ralph Siemsen, Safae Ouajih, Sean Anderson, Sean Anderson
This little series corrects a problem I noticed with arm64 images,
where the kernel is not recognised:
## Loading kernel from FIT Image at 10000000 ...
Using 'conf-930' configuration
Verifying Hash Integrity ... OK
Trying 'kernel' kernel subimage
Description: Linux-6.6.0-rc7-next-20231024-00003-g259c196f194c
Type: Kernel Image (no loading done)
Compression: gzip compressed
Data Start: 0x10000138
Data Size: 13667956 Bytes = 13 MiB
Verifying Hash Integrity ... OK
Bad Linux ARM64 Image magic!
The problem is that the arm64 magic is checked before the image is
decompressed.
Another issue is that the load address is read from the 'load' property
even with a kernel_noload image. This means that the kernel is loaded
to address 0, which may not be valid on the board. We can use the
kernel_addr_r environment variable instead.
A patch is included to show the kernel load-address, so it is easy to
see what is going on.
Simon Glass (4):
bootm: Allow ignoring the load address with kernel_noload
bootm: Move arm64-image processing later
image: Show the load address when decompressing
image: Correct load_bug typo
boot/bootm.c | 61 ++++++++++++++++++++++++++++++-------------------
boot/image.c | 13 +++++++----
include/image.h | 2 +-
3 files changed, 48 insertions(+), 28 deletions(-)
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload
2023-11-05 20:03 [PATCH 0/4] bootm: Handle compressed arm64 images with bootm Simon Glass
@ 2023-11-05 20:03 ` Simon Glass
2023-11-05 21:19 ` Tom Rini
2023-11-05 20:03 ` [PATCH 2/4] bootm: Move arm64-image processing later Simon Glass
` (4 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2023-11-05 20:03 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Marek Vasut, Simon Glass, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Mattijs Korpershoek,
Safae Ouajih
This image type is supposed to ignore the load address. But at present
it fails if the load address is missing. If it is zero, the image is
loaded at address 0, which may not work on all boards.
Make use of the kernel_addr_r environment variable, instead, since this
seems to be a more reliable final address for the kernel.
Another option would be to create a new Kconfig for this, or to use a
region of memory known to be free, e.g. calculated from the DRAM banks.
But in any case we should try to avoid conflicting with the
kernel_addr_r variable. So the approach in this patch seems reasonable
to me.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
boot/bootm.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c
index cb61485c226c..7583be5a4515 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -176,7 +176,13 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
images.os.end = fit_get_end(images.fit_hdr_os);
- if (fit_image_get_load(images.fit_hdr_os, images.fit_noffset_os,
+ if (images.os.type == IH_TYPE_KERNEL_NOLOAD) {
+ ulong load;
+
+ load = env_get_hex("kernel_addr_r", -1UL);
+ printf("Using kernel load address %lx\n", load);
+ images.os.load = load;
+ } else if (fit_image_get_load(images.fit_hdr_os, images.fit_noffset_os,
&images.os.load)) {
puts("Can't get image load address!\n");
bootstage_error(BOOTSTAGE_ID_FIT_LOADADDR);
@@ -229,7 +235,7 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
ret = fit_image_get_entry(images.fit_hdr_os,
images.fit_noffset_os, &images.ep);
- if (ret) {
+ if (ret && images.os.type == IH_TYPE_KERNEL_NOLOAD) {
puts("Can't get entry point property!\n");
return 1;
}
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 2/4] bootm: Move arm64-image processing later
2023-11-05 20:03 [PATCH 0/4] bootm: Handle compressed arm64 images with bootm Simon Glass
2023-11-05 20:03 ` [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload Simon Glass
@ 2023-11-05 20:03 ` Simon Glass
2023-11-05 21:20 ` Tom Rini
2023-11-05 20:03 ` [PATCH 3/4] image: Show the load address when decompressing Simon Glass
` (3 subsequent siblings)
5 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2023-11-05 20:03 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Marek Vasut, Simon Glass, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Mattijs Korpershoek,
Safae Ouajih
If the image is compressed, then the existing check fails, since the
header is wrong.
Move the check later in the boot process, after the kernel is
decompressed. This allows use of bootm with compressed kernels, while
still permitting an uncompressed kernel to be used.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
boot/bootm.c | 51 ++++++++++++++++++++++++++++++---------------------
1 file changed, 30 insertions(+), 21 deletions(-)
diff --git a/boot/bootm.c b/boot/bootm.c
index 7583be5a4515..8c35afb09937 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -245,27 +245,6 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc,
return 1;
}
- if (images.os.type == IH_TYPE_KERNEL_NOLOAD) {
- if (IS_ENABLED(CONFIG_CMD_BOOTI) &&
- images.os.arch == IH_ARCH_ARM64 &&
- images.os.os == IH_OS_LINUX) {
- ulong image_addr;
- ulong image_size;
-
- ret = booti_setup(images.os.image_start, &image_addr,
- &image_size, true);
- if (ret != 0)
- return 1;
-
- images.os.type = IH_TYPE_KERNEL;
- images.os.load = image_addr;
- images.ep = image_addr;
- } else {
- images.os.load = images.os.image_start;
- images.ep += images.os.image_start;
- }
- }
-
images.os.start = map_to_sysmem(os_hdr);
return 0;
@@ -472,6 +451,36 @@ static int bootm_load_os(struct bootm_headers *images, int boot_progress)
}
}
+ if (IS_ENABLED(CONFIG_CMD_BOOTI) &&
+ images->os.type == IH_TYPE_KERNEL_NOLOAD &&
+ images->os.arch == IH_ARCH_ARM64 &&
+ images->os.os == IH_OS_LINUX) {
+ ulong relocated_addr;
+ ulong image_size;
+ int ret;
+
+ ret = booti_setup(load, &relocated_addr, &image_size,
+ false);
+ if (ret) {
+ printf("Failed to prep arm64 kernel (err=%d)\n",
+ ret);
+ return BOOTM_ERR_RESET;
+ }
+
+ /* Handle BOOTM_STATE_LOADOS */
+ if (relocated_addr != load) {
+ printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n",
+ load,
+ relocated_addr, relocated_addr + image_size);
+ memmove((void *)relocated_addr,
+ load_buf, image_size);
+ }
+
+ images->ep = relocated_addr;
+ images->os.start = relocated_addr;
+ images->os.end = relocated_addr + image_size;
+ }
+
lmb_reserve(&images->lmb, images->os.load, (load_end -
images->os.load));
return 0;
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 3/4] image: Show the load address when decompressing
2023-11-05 20:03 [PATCH 0/4] bootm: Handle compressed arm64 images with bootm Simon Glass
2023-11-05 20:03 ` [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload Simon Glass
2023-11-05 20:03 ` [PATCH 2/4] bootm: Move arm64-image processing later Simon Glass
@ 2023-11-05 20:03 ` Simon Glass
2023-11-05 20:03 ` [PATCH 4/4] image: Correct load_bug typo Simon Glass
` (2 subsequent siblings)
5 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2023-11-05 20:03 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Marek Vasut, Simon Glass, Heinrich Schuchardt,
Joe Hershberger, Marc Kleine-Budde, Marek Vasut, Ralph Siemsen,
Sean Anderson
The destination address for decompression (or copying) is useful
information. Show this to the user while booting, e.g.:
Uncompressing Kernel Image (no loading done) to 2080000
Signed-off-by: Simon Glass <sjg@chromium.org>
---
boot/image.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/boot/image.c b/boot/image.c
index 88b67bc3a199..675b5dd77f94 100644
--- a/boot/image.c
+++ b/boot/image.c
@@ -415,15 +415,20 @@ void image_print_contents(const void *ptr)
* @type: OS type (IH_OS_...)
* @comp_type: Compression type being used (IH_COMP_...)
* @is_xip: true if the load address matches the image start
+ * @load: Load address for printing
*/
-static void print_decomp_msg(int comp_type, int type, bool is_xip)
+static void print_decomp_msg(int comp_type, int type, bool is_xip,
+ ulong load)
{
const char *name = genimg_get_type_name(type);
+ /* Shows "Loading Kernel Image" for example */
if (comp_type == IH_COMP_NONE)
- printf(" %s %s\n", is_xip ? "XIP" : "Loading", name);
+ printf(" %s %s", is_xip ? "XIP" : "Loading", name);
else
- printf(" Uncompressing %s\n", name);
+ printf(" Uncompressing %s", name);
+
+ printf(" to %lx\n", load);
}
int image_decomp_type(const unsigned char *buf, ulong len)
@@ -448,7 +453,7 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
int ret = -ENOSYS;
*load_end = load;
- print_decomp_msg(comp, type, load == image_start);
+ print_decomp_msg(comp, type, load == image_start, load);
/*
* Load the image to the right place, decompressing if needed. After
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH 4/4] image: Correct load_bug typo
2023-11-05 20:03 [PATCH 0/4] bootm: Handle compressed arm64 images with bootm Simon Glass
` (2 preceding siblings ...)
2023-11-05 20:03 ` [PATCH 3/4] image: Show the load address when decompressing Simon Glass
@ 2023-11-05 20:03 ` Simon Glass
2023-11-05 20:06 ` [PATCH 0/4] bootm: Handle compressed arm64 images with bootm Simon Glass
2023-11-07 9:56 ` Rasmus Villemoes
5 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2023-11-05 20:03 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Marek Vasut, Simon Glass, Heinrich Schuchardt,
Joe Hershberger, Marc Kleine-Budde, Mattijs Korpershoek,
Safae Ouajih, Sean Anderson
Correct a typo in the function comment for image_decomp().
Signed-off-by: Simon Glass <sjg@chromium.org>
---
include/image.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/image.h b/include/image.h
index 2e3cf839ee36..0fe67852c563 100644
--- a/include/image.h
+++ b/include/image.h
@@ -936,7 +936,7 @@ int image_decomp_type(const unsigned char *buf, ulong len);
* @load: Destination load address in U-Boot memory
* @image_start Image start address (where we are decompressing from)
* @type: OS type (IH_OS_...)
- * @load_bug: Place to decompress to
+ * @load_buf: Place to decompress to
* @image_buf: Address to decompress from
* @image_len: Number of bytes in @image_buf to decompress
* @unc_len: Available space for decompression
--
2.42.0.869.gea05f2083d-goog
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] bootm: Handle compressed arm64 images with bootm
2023-11-05 20:03 [PATCH 0/4] bootm: Handle compressed arm64 images with bootm Simon Glass
` (3 preceding siblings ...)
2023-11-05 20:03 ` [PATCH 4/4] image: Correct load_bug typo Simon Glass
@ 2023-11-05 20:06 ` Simon Glass
2023-11-07 9:56 ` Rasmus Villemoes
5 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2023-11-05 20:06 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Tom Rini, Marek Vasut, Eddie James, Heinrich Schuchardt,
Ilias Apalodimas, Joe Hershberger, Marc Kleine-Budde, Marek Vasut,
Mattijs Korpershoek, Ralph Siemsen, Safae Ouajih, Sean Anderson,
Sean Anderson
Hi,
On Sun, 5 Nov 2023 at 20:05, Simon Glass <sjg@chromium.org> wrote:
>
> This little series corrects a problem I noticed with arm64 images,
> where the kernel is not recognised:
>
> ## Loading kernel from FIT Image at 10000000 ...
> Using 'conf-930' configuration
> Verifying Hash Integrity ... OK
> Trying 'kernel' kernel subimage
> Description: Linux-6.6.0-rc7-next-20231024-00003-g259c196f194c
> Type: Kernel Image (no loading done)
> Compression: gzip compressed
> Data Start: 0x10000138
> Data Size: 13667956 Bytes = 13 MiB
> Verifying Hash Integrity ... OK
> Bad Linux ARM64 Image magic!
>
> The problem is that the arm64 magic is checked before the image is
> decompressed.
>
> Another issue is that the load address is read from the 'load' property
> even with a kernel_noload image. This means that the kernel is loaded
> to address 0, which may not be valid on the board. We can use the
> kernel_addr_r environment variable instead.
>
> A patch is included to show the kernel load-address, so it is easy to
> see what is going on.
>
>
> Simon Glass (4):
> bootm: Allow ignoring the load address with kernel_noload
> bootm: Move arm64-image processing later
> image: Show the load address when decompressing
> image: Correct load_bug typo
>
> boot/bootm.c | 61 ++++++++++++++++++++++++++++++-------------------
> boot/image.c | 13 +++++++----
> include/image.h | 2 +-
> 3 files changed, 48 insertions(+), 28 deletions(-)
>
> --
> 2.42.0.869.gea05f2083d-goog
>
This series has a few problems still, but I am sending it now since I
believe it fixes a real problem on arm64. I will tidy it up soon.
Regards,
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload
2023-11-05 20:03 ` [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload Simon Glass
@ 2023-11-05 21:19 ` Tom Rini
2023-11-06 17:25 ` Simon Glass
0 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2023-11-05 21:19 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Mattijs Korpershoek,
Safae Ouajih
[-- Attachment #1: Type: text/plain, Size: 925 bytes --]
On Sun, Nov 05, 2023 at 01:03:51PM -0700, Simon Glass wrote:
> This image type is supposed to ignore the load address. But at present
> it fails if the load address is missing. If it is zero, the image is
> loaded at address 0, which may not work on all boards.
>
> Make use of the kernel_addr_r environment variable, instead, since this
> seems to be a more reliable final address for the kernel.
>
> Another option would be to create a new Kconfig for this, or to use a
> region of memory known to be free, e.g. calculated from the DRAM banks.
> But in any case we should try to avoid conflicting with the
> kernel_addr_r variable. So the approach in this patch seems reasonable
> to me.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
How are you creating the image in question here? A noload FIT is
supposed to just supposed to go from where it is. Where do things fall
down later?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] bootm: Move arm64-image processing later
2023-11-05 20:03 ` [PATCH 2/4] bootm: Move arm64-image processing later Simon Glass
@ 2023-11-05 21:20 ` Tom Rini
2023-11-06 17:25 ` Simon Glass
0 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2023-11-05 21:20 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Mattijs Korpershoek,
Safae Ouajih
[-- Attachment #1: Type: text/plain, Size: 465 bytes --]
On Sun, Nov 05, 2023 at 01:03:52PM -0700, Simon Glass wrote:
> If the image is compressed, then the existing check fails, since the
> header is wrong.
>
> Move the check later in the boot process, after the kernel is
> decompressed. This allows use of bootm with compressed kernels, while
> still permitting an uncompressed kernel to be used.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
How are we getting in to this case exactly?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload
2023-11-05 21:19 ` Tom Rini
@ 2023-11-06 17:25 ` Simon Glass
2023-11-06 18:30 ` Tom Rini
0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2023-11-06 17:25 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Mattijs Korpershoek,
Safae Ouajih
Hi Tom,
On Sun, 5 Nov 2023 at 14:19, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Nov 05, 2023 at 01:03:51PM -0700, Simon Glass wrote:
>
> > This image type is supposed to ignore the load address. But at present
> > it fails if the load address is missing. If it is zero, the image is
> > loaded at address 0, which may not work on all boards.
> >
> > Make use of the kernel_addr_r environment variable, instead, since this
> > seems to be a more reliable final address for the kernel.
> >
> > Another option would be to create a new Kconfig for this, or to use a
> > region of memory known to be free, e.g. calculated from the DRAM banks.
> > But in any case we should try to avoid conflicting with the
> > kernel_addr_r variable. So the approach in this patch seems reasonable
> > to me.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> How are you creating the image in question here? A noload FIT is
> supposed to just supposed to go from where it is. Where do things fall
> down later?
The image is Image.gz built by Linux, for example. So compression =
"gzip" which means that it has to be decompressed.
Things fall down as soon as U-Boot looks at the image, since it
doesn't have the ARM64 magic.
Regards,
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 2/4] bootm: Move arm64-image processing later
2023-11-05 21:20 ` Tom Rini
@ 2023-11-06 17:25 ` Simon Glass
0 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2023-11-06 17:25 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Mattijs Korpershoek,
Safae Ouajih
Hi Tom,
On Sun, 5 Nov 2023 at 14:20, Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Nov 05, 2023 at 01:03:52PM -0700, Simon Glass wrote:
>
> > If the image is compressed, then the existing check fails, since the
> > header is wrong.
> >
> > Move the check later in the boot process, after the kernel is
> > decompressed. This allows use of bootm with compressed kernels, while
> > still permitting an uncompressed kernel to be used.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
>
> How are we getting in to this case exactly?
By using a compressed ARM64 image, e.g. Image.gz
Regards,
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload
2023-11-06 17:25 ` Simon Glass
@ 2023-11-06 18:30 ` Tom Rini
2023-11-06 19:58 ` Simon Glass
0 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2023-11-06 18:30 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Mattijs Korpershoek,
Safae Ouajih
[-- Attachment #1: Type: text/plain, Size: 1525 bytes --]
On Mon, Nov 06, 2023 at 10:25:00AM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Sun, 5 Nov 2023 at 14:19, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Sun, Nov 05, 2023 at 01:03:51PM -0700, Simon Glass wrote:
> >
> > > This image type is supposed to ignore the load address. But at present
> > > it fails if the load address is missing. If it is zero, the image is
> > > loaded at address 0, which may not work on all boards.
> > >
> > > Make use of the kernel_addr_r environment variable, instead, since this
> > > seems to be a more reliable final address for the kernel.
> > >
> > > Another option would be to create a new Kconfig for this, or to use a
> > > region of memory known to be free, e.g. calculated from the DRAM banks.
> > > But in any case we should try to avoid conflicting with the
> > > kernel_addr_r variable. So the approach in this patch seems reasonable
> > > to me.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> >
> > How are you creating the image in question here? A noload FIT is
> > supposed to just supposed to go from where it is. Where do things fall
> > down later?
>
> The image is Image.gz built by Linux, for example. So compression =
> "gzip" which means that it has to be decompressed.
>
> Things fall down as soon as U-Boot looks at the image, since it
> doesn't have the ARM64 magic.
Can you provide logs and env? "booti" is supposed to handle this case
already, and if it's not we should figure out when / why it broke.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload
2023-11-06 18:30 ` Tom Rini
@ 2023-11-06 19:58 ` Simon Glass
2023-11-06 20:15 ` Tom Rini
0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2023-11-06 19:58 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Mattijs Korpershoek,
Safae Ouajih
Hi Tom,
On Mon, 6 Nov 2023 at 11:30, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Nov 06, 2023 at 10:25:00AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sun, 5 Nov 2023 at 14:19, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Sun, Nov 05, 2023 at 01:03:51PM -0700, Simon Glass wrote:
> > >
> > > > This image type is supposed to ignore the load address. But at present
> > > > it fails if the load address is missing. If it is zero, the image is
> > > > loaded at address 0, which may not work on all boards.
> > > >
> > > > Make use of the kernel_addr_r environment variable, instead, since this
> > > > seems to be a more reliable final address for the kernel.
> > > >
> > > > Another option would be to create a new Kconfig for this, or to use a
> > > > region of memory known to be free, e.g. calculated from the DRAM banks.
> > > > But in any case we should try to avoid conflicting with the
> > > > kernel_addr_r variable. So the approach in this patch seems reasonable
> > > > to me.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > >
> > > How are you creating the image in question here? A noload FIT is
> > > supposed to just supposed to go from where it is. Where do things fall
> > > down later?
> >
> > The image is Image.gz built by Linux, for example. So compression =
> > "gzip" which means that it has to be decompressed.
> >
> > Things fall down as soon as U-Boot looks at the image, since it
> > doesn't have the ARM64 magic.
>
> Can you provide logs and env? "booti" is supposed to handle this case
> already, and if it's not we should figure out when / why it broke.
Do you mean booti handles compression? Yes, I can see that in the code.
But in my case I am using bootm, since it is a FIT.
Regards,
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload
2023-11-06 19:58 ` Simon Glass
@ 2023-11-06 20:15 ` Tom Rini
2023-11-07 1:09 ` Simon Glass
0 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2023-11-06 20:15 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Mattijs Korpershoek,
Safae Ouajih
[-- Attachment #1: Type: text/plain, Size: 2143 bytes --]
On Mon, Nov 06, 2023 at 12:58:46PM -0700, Simon Glass wrote:
> Hi Tom,
>
>
> On Mon, 6 Nov 2023 at 11:30, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Mon, Nov 06, 2023 at 10:25:00AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Sun, 5 Nov 2023 at 14:19, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Sun, Nov 05, 2023 at 01:03:51PM -0700, Simon Glass wrote:
> > > >
> > > > > This image type is supposed to ignore the load address. But at present
> > > > > it fails if the load address is missing. If it is zero, the image is
> > > > > loaded at address 0, which may not work on all boards.
> > > > >
> > > > > Make use of the kernel_addr_r environment variable, instead, since this
> > > > > seems to be a more reliable final address for the kernel.
> > > > >
> > > > > Another option would be to create a new Kconfig for this, or to use a
> > > > > region of memory known to be free, e.g. calculated from the DRAM banks.
> > > > > But in any case we should try to avoid conflicting with the
> > > > > kernel_addr_r variable. So the approach in this patch seems reasonable
> > > > > to me.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > >
> > > > How are you creating the image in question here? A noload FIT is
> > > > supposed to just supposed to go from where it is. Where do things fall
> > > > down later?
> > >
> > > The image is Image.gz built by Linux, for example. So compression =
> > > "gzip" which means that it has to be decompressed.
> > >
> > > Things fall down as soon as U-Boot looks at the image, since it
> > > doesn't have the ARM64 magic.
> >
> > Can you provide logs and env? "booti" is supposed to handle this case
> > already, and if it's not we should figure out when / why it broke.
>
> Do you mean booti handles compression? Yes, I can see that in the code.
Yes, you use "booti" with an Image.gz.
> But in my case I am using bootm, since it is a FIT.
Shouldn't this be handled by the normal compression = "foo" logic? And in
turn is that what's not working? If so, the commit messages aren't
clear.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload
2023-11-06 20:15 ` Tom Rini
@ 2023-11-07 1:09 ` Simon Glass
0 siblings, 0 replies; 21+ messages in thread
From: Simon Glass @ 2023-11-07 1:09 UTC (permalink / raw)
To: Tom Rini
Cc: U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Mattijs Korpershoek,
Safae Ouajih
Hi Tom,
On Mon, 6 Nov 2023 at 13:15, Tom Rini <trini@konsulko.com> wrote:
>
> On Mon, Nov 06, 2023 at 12:58:46PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> >
> > On Mon, 6 Nov 2023 at 11:30, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Mon, Nov 06, 2023 at 10:25:00AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Sun, 5 Nov 2023 at 14:19, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Sun, Nov 05, 2023 at 01:03:51PM -0700, Simon Glass wrote:
> > > > >
> > > > > > This image type is supposed to ignore the load address. But at present
> > > > > > it fails if the load address is missing. If it is zero, the image is
> > > > > > loaded at address 0, which may not work on all boards.
> > > > > >
> > > > > > Make use of the kernel_addr_r environment variable, instead, since this
> > > > > > seems to be a more reliable final address for the kernel.
> > > > > >
> > > > > > Another option would be to create a new Kconfig for this, or to use a
> > > > > > region of memory known to be free, e.g. calculated from the DRAM banks.
> > > > > > But in any case we should try to avoid conflicting with the
> > > > > > kernel_addr_r variable. So the approach in this patch seems reasonable
> > > > > > to me.
> > > > > >
> > > > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > >
> > > > > How are you creating the image in question here? A noload FIT is
> > > > > supposed to just supposed to go from where it is. Where do things fall
> > > > > down later?
> > > >
> > > > The image is Image.gz built by Linux, for example. So compression =
> > > > "gzip" which means that it has to be decompressed.
> > > >
> > > > Things fall down as soon as U-Boot looks at the image, since it
> > > > doesn't have the ARM64 magic.
> > >
> > > Can you provide logs and env? "booti" is supposed to handle this case
> > > already, and if it's not we should figure out when / why it broke.
> >
> > Do you mean booti handles compression? Yes, I can see that in the code.
>
> Yes, you use "booti" with an Image.gz.
>
> > But in my case I am using bootm, since it is a FIT.
>
> Shouldn't this be handled by the normal compression = "foo" logic? And in
> turn is that what's not working? If so, the commit messages aren't
> clear.
We are on the wrong commit here. See this one:
https://patchwork.ozlabs.org/project/uboot/patch/20231105130351.2.Ie34b75c75347a1c04bffa780220afe9011b0e2bb@changeid/
Perhaps this is supposed to work some other way, but to me it seems
quite broken at present.
Regards,
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] bootm: Handle compressed arm64 images with bootm
2023-11-05 20:03 [PATCH 0/4] bootm: Handle compressed arm64 images with bootm Simon Glass
` (4 preceding siblings ...)
2023-11-05 20:06 ` [PATCH 0/4] bootm: Handle compressed arm64 images with bootm Simon Glass
@ 2023-11-07 9:56 ` Rasmus Villemoes
2023-11-07 12:23 ` Simon Glass
5 siblings, 1 reply; 21+ messages in thread
From: Rasmus Villemoes @ 2023-11-07 9:56 UTC (permalink / raw)
To: Simon Glass, U-Boot Mailing List
Cc: Tom Rini, Marek Vasut, Eddie James, Heinrich Schuchardt,
Ilias Apalodimas, Joe Hershberger, Marc Kleine-Budde, Marek Vasut,
Mattijs Korpershoek, Ralph Siemsen, Safae Ouajih, Sean Anderson,
Sean Anderson
On 05/11/2023 21.03, Simon Glass wrote:
> This little series corrects a problem I noticed with arm64 images,
> where the kernel is not recognised:
The $subject is misleading, bootm works just fine with compressed arm64
images, with the type set to "kernel".
> Type: Kernel Image (no loading done)
> Compression: gzip compressed
Isn't that a non-sensical combination to begin with? Decompressing the
Image.gz kernel image to any location (however you determine that
destination) _is_ loading it.
If you want XIP, obviously the image must be uncompressed in the FIT. I
don't understand what you're trying to do here.
Rasmus
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] bootm: Handle compressed arm64 images with bootm
2023-11-07 9:56 ` Rasmus Villemoes
@ 2023-11-07 12:23 ` Simon Glass
2023-11-07 13:04 ` Tom Rini
0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2023-11-07 12:23 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: U-Boot Mailing List, Tom Rini, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Joe Hershberger,
Marc Kleine-Budde, Marek Vasut, Mattijs Korpershoek,
Ralph Siemsen, Safae Ouajih, Sean Anderson, Sean Anderson
Hi Rasmus,
On Tue, 7 Nov 2023 at 02:56, Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> On 05/11/2023 21.03, Simon Glass wrote:
> > This little series corrects a problem I noticed with arm64 images,
> > where the kernel is not recognised:
>
> The $subject is misleading, bootm works just fine with compressed arm64
> images, with the type set to "kernel".
>
> > Type: Kernel Image (no loading done)
> > Compression: gzip compressed
>
> Isn't that a non-sensical combination to begin with? Decompressing the
> Image.gz kernel image to any location (however you determine that
> destination) _is_ loading it.
Yes, I agree.
>
> If you want XIP, obviously the image must be uncompressed in the FIT. I
> don't understand what you're trying to do here.
Hmmm, I think I have just got confused about all of this, perhaps
because ChromeOS uses kernel_noload with compression. Is that an
invalid combination?
But then how does loading actually work? We don't want to put the load
address in the FIT, since we don't know what it is...we want to use
the address provided by the board. Which is kernel_noload...so how
should this be implemented?
Regards,
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] bootm: Handle compressed arm64 images with bootm
2023-11-07 12:23 ` Simon Glass
@ 2023-11-07 13:04 ` Tom Rini
2023-11-07 13:31 ` Simon Glass
0 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2023-11-07 13:04 UTC (permalink / raw)
To: Simon Glass
Cc: Rasmus Villemoes, U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Joe Hershberger,
Marc Kleine-Budde, Marek Vasut, Mattijs Korpershoek,
Ralph Siemsen, Safae Ouajih, Sean Anderson, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 2768 bytes --]
On Tue, Nov 07, 2023 at 05:23:05AM -0700, Simon Glass wrote:
> Hi Rasmus,
>
> On Tue, 7 Nov 2023 at 02:56, Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> wrote:
> >
> > On 05/11/2023 21.03, Simon Glass wrote:
> > > This little series corrects a problem I noticed with arm64 images,
> > > where the kernel is not recognised:
> >
> > The $subject is misleading, bootm works just fine with compressed arm64
> > images, with the type set to "kernel".
> >
> > > Type: Kernel Image (no loading done)
> > > Compression: gzip compressed
> >
> > Isn't that a non-sensical combination to begin with? Decompressing the
> > Image.gz kernel image to any location (however you determine that
> > destination) _is_ loading it.
>
> Yes, I agree.
>
> >
> > If you want XIP, obviously the image must be uncompressed in the FIT. I
> > don't understand what you're trying to do here.
>
> Hmmm, I think I have just got confused about all of this, perhaps
> because ChromeOS uses kernel_noload with compression. Is that an
> invalid combination?
Yes, that sounds like an invalid combination.
> But then how does loading actually work? We don't want to put the load
> address in the FIT, since we don't know what it is...we want to use
> the address provided by the board. Which is kernel_noload...so how
> should this be implemented?
What do you mean, provided by the board? With kernel_noload we XIP the
payload, because the board provided (by loading us to) a safe place to
execute whatevers in there. In the olden days, that would mean (almost
certainly) the zImage which in turn was already compressed and
self-relocating. I know technically one could use the raw vmlinux
instead. With the Linux Kernel and ARCH=arm64 (and a few other arches
now too), they dropped the self-decompression part and the whole payload
must be decompressed. We handle this case in "booti" today by having to
have the board (via environment) say where to decompress to (and how
much space is available). Then we move it back to where we started from,
which is likely not necessary.
Looking at
https://www.kernel.org/doc/html/latest/arch/arm64/booting.html and
https://www.kernel.org/doc/html/latest/riscv/boot-image-header.html
stating "we're just like arm64" we can do better than we do today for
this format of OS image. If aren't compressed, we only need to ensure
that we are correctly aligned and move (and tell the user) if not. We
can even put the 2MB check under some legacy kernel CONFIG option (3.17
is over 9 years old). With respect to automatic decompression, if we
don't have something telling us where a buffer is and we can't pull from
the environment, we should tell the user and stop?
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] bootm: Handle compressed arm64 images with bootm
2023-11-07 13:04 ` Tom Rini
@ 2023-11-07 13:31 ` Simon Glass
2023-11-07 13:49 ` Tom Rini
0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2023-11-07 13:31 UTC (permalink / raw)
To: Tom Rini
Cc: Rasmus Villemoes, U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Joe Hershberger,
Marc Kleine-Budde, Marek Vasut, Mattijs Korpershoek,
Ralph Siemsen, Safae Ouajih, Sean Anderson, Sean Anderson
Hi Tom,
On Tue, 7 Nov 2023 at 06:04, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Nov 07, 2023 at 05:23:05AM -0700, Simon Glass wrote:
> > Hi Rasmus,
> >
> > On Tue, 7 Nov 2023 at 02:56, Rasmus Villemoes
> > <rasmus.villemoes@prevas.dk> wrote:
> > >
> > > On 05/11/2023 21.03, Simon Glass wrote:
> > > > This little series corrects a problem I noticed with arm64 images,
> > > > where the kernel is not recognised:
> > >
> > > The $subject is misleading, bootm works just fine with compressed arm64
> > > images, with the type set to "kernel".
> > >
> > > > Type: Kernel Image (no loading done)
> > > > Compression: gzip compressed
> > >
> > > Isn't that a non-sensical combination to begin with? Decompressing the
> > > Image.gz kernel image to any location (however you determine that
> > > destination) _is_ loading it.
> >
> > Yes, I agree.
> >
> > >
> > > If you want XIP, obviously the image must be uncompressed in the FIT. I
> > > don't understand what you're trying to do here.
> >
> > Hmmm, I think I have just got confused about all of this, perhaps
> > because ChromeOS uses kernel_noload with compression. Is that an
> > invalid combination?
>
> Yes, that sounds like an invalid combination.
>
> > But then how does loading actually work? We don't want to put the load
> > address in the FIT, since we don't know what it is...we want to use
> > the address provided by the board. Which is kernel_noload...so how
> > should this be implemented?
>
> What do you mean, provided by the board? With kernel_noload we XIP the
> payload, because the board provided (by loading us to) a safe place to
> execute whatevers in there. In the olden days, that would mean (almost
> certainly) the zImage which in turn was already compressed and
> self-relocating. I know technically one could use the raw vmlinux
> instead. With the Linux Kernel and ARCH=arm64 (and a few other arches
> now too), they dropped the self-decompression part and the whole payload
> must be decompressed. We handle this case in "booti" today by having to
> have the board (via environment) say where to decompress to (and how
> much space is available). Then we move it back to where we started from,
> which is likely not necessary.
>
> Looking at
> https://www.kernel.org/doc/html/latest/arch/arm64/booting.html and
> https://www.kernel.org/doc/html/latest/riscv/boot-image-header.html
> stating "we're just like arm64" we can do better than we do today for
> this format of OS image. If aren't compressed, we only need to ensure
> that we are correctly aligned and move (and tell the user) if not. We
> can even put the 2MB check under some legacy kernel CONFIG option (3.17
> is over 9 years old). With respect to automatic decompression, if we
> don't have something telling us where a buffer is and we can't pull from
> the environment, we should tell the user and stop?
With the booti command, the kern_comp_addr_r and kern_comp_size vars
are used. Perhaps I should update my patch to use those too?
Do you understand what I am asking about, with bootm? It should be
possible to ignore the load address. In fact, load addresses in FIT
are mostly bad these days, IMO. If this is not kernel_noload, then
what should we use?
"provided by the board" means that the board knows the memory map, not
the FIT, so only the board can provide the addresses to use for
loading the kernel.
Regards,
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] bootm: Handle compressed arm64 images with bootm
2023-11-07 13:31 ` Simon Glass
@ 2023-11-07 13:49 ` Tom Rini
2023-11-07 14:30 ` Simon Glass
0 siblings, 1 reply; 21+ messages in thread
From: Tom Rini @ 2023-11-07 13:49 UTC (permalink / raw)
To: Simon Glass
Cc: Rasmus Villemoes, U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Joe Hershberger,
Marc Kleine-Budde, Marek Vasut, Mattijs Korpershoek,
Ralph Siemsen, Safae Ouajih, Sean Anderson, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 5066 bytes --]
On Tue, Nov 07, 2023 at 06:31:15AM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Tue, 7 Nov 2023 at 06:04, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Nov 07, 2023 at 05:23:05AM -0700, Simon Glass wrote:
> > > Hi Rasmus,
> > >
> > > On Tue, 7 Nov 2023 at 02:56, Rasmus Villemoes
> > > <rasmus.villemoes@prevas.dk> wrote:
> > > >
> > > > On 05/11/2023 21.03, Simon Glass wrote:
> > > > > This little series corrects a problem I noticed with arm64 images,
> > > > > where the kernel is not recognised:
> > > >
> > > > The $subject is misleading, bootm works just fine with compressed arm64
> > > > images, with the type set to "kernel".
> > > >
> > > > > Type: Kernel Image (no loading done)
> > > > > Compression: gzip compressed
> > > >
> > > > Isn't that a non-sensical combination to begin with? Decompressing the
> > > > Image.gz kernel image to any location (however you determine that
> > > > destination) _is_ loading it.
> > >
> > > Yes, I agree.
> > >
> > > >
> > > > If you want XIP, obviously the image must be uncompressed in the FIT. I
> > > > don't understand what you're trying to do here.
> > >
> > > Hmmm, I think I have just got confused about all of this, perhaps
> > > because ChromeOS uses kernel_noload with compression. Is that an
> > > invalid combination?
> >
> > Yes, that sounds like an invalid combination.
> >
> > > But then how does loading actually work? We don't want to put the load
> > > address in the FIT, since we don't know what it is...we want to use
> > > the address provided by the board. Which is kernel_noload...so how
> > > should this be implemented?
> >
> > What do you mean, provided by the board? With kernel_noload we XIP the
> > payload, because the board provided (by loading us to) a safe place to
> > execute whatevers in there. In the olden days, that would mean (almost
> > certainly) the zImage which in turn was already compressed and
> > self-relocating. I know technically one could use the raw vmlinux
> > instead. With the Linux Kernel and ARCH=arm64 (and a few other arches
> > now too), they dropped the self-decompression part and the whole payload
> > must be decompressed. We handle this case in "booti" today by having to
> > have the board (via environment) say where to decompress to (and how
> > much space is available). Then we move it back to where we started from,
> > which is likely not necessary.
> >
> > Looking at
> > https://www.kernel.org/doc/html/latest/arch/arm64/booting.html and
> > https://www.kernel.org/doc/html/latest/riscv/boot-image-header.html
> > stating "we're just like arm64" we can do better than we do today for
> > this format of OS image. If aren't compressed, we only need to ensure
> > that we are correctly aligned and move (and tell the user) if not. We
> > can even put the 2MB check under some legacy kernel CONFIG option (3.17
> > is over 9 years old). With respect to automatic decompression, if we
> > don't have something telling us where a buffer is and we can't pull from
> > the environment, we should tell the user and stop?
>
> With the booti command, the kern_comp_addr_r and kern_comp_size vars
> are used. Perhaps I should update my patch to use those too?
As a fallback, perhaps? But what do we do today in a FIT to handle
compression = "gzip" to know where to uncompress things to?
> Do you understand what I am asking about, with bootm? It should be
> possible to ignore the load address. In fact, load addresses in FIT
> are mostly bad these days, IMO. If this is not kernel_noload, then
> what should we use?
Well, your use case needs to be valid. I don't think "kernel_noload" is
valid with anything other than 'compression = "none"' because that's our
just execute things in place option. It's very dangerous to use and I
do not recall right now if we fixed the case where it will just let the
device tree be badly aligned and break the kernel, or if we say "no
load, but still fix THAT alignment". And your use case isn't "ignore the
load address" it's "ignore the load address and decompress the
payload(s)". That needs _something_ to be known somehow. And I'm asking
how we do that today even, before saying we can or can't support
"kernel_noload" along with a compression of the payload.
> "provided by the board" means that the board knows the memory map, not
> the FIT, so only the board can provide the addresses to use for
> loading the kernel.
To be clearer, where does the board document that knowledge and provide
it to be used? I'm not a super fan of environment variables especially
since we aren't always consistent with '_' or just nothing between
"words" of the variable. We have much better documentation these days
(once again, thanks everyone) so we can document environment variables
used here, and also what CONFIG options are used (as
CONFIG_SYS_BOOTM_LEN is part of the equation here and badly named as
been said in other threads about issues wrt decomp and overlap).
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] bootm: Handle compressed arm64 images with bootm
2023-11-07 13:49 ` Tom Rini
@ 2023-11-07 14:30 ` Simon Glass
2023-11-07 19:04 ` Tom Rini
0 siblings, 1 reply; 21+ messages in thread
From: Simon Glass @ 2023-11-07 14:30 UTC (permalink / raw)
To: Tom Rini
Cc: Rasmus Villemoes, U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Joe Hershberger,
Marc Kleine-Budde, Marek Vasut, Mattijs Korpershoek,
Ralph Siemsen, Safae Ouajih, Sean Anderson, Sean Anderson
Hi Tom,
On Tue, 7 Nov 2023 at 06:49, Tom Rini <trini@konsulko.com> wrote:
>
> On Tue, Nov 07, 2023 at 06:31:15AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Tue, 7 Nov 2023 at 06:04, Tom Rini <trini@konsulko.com> wrote:
> > >
> > > On Tue, Nov 07, 2023 at 05:23:05AM -0700, Simon Glass wrote:
> > > > Hi Rasmus,
> > > >
> > > > On Tue, 7 Nov 2023 at 02:56, Rasmus Villemoes
> > > > <rasmus.villemoes@prevas.dk> wrote:
> > > > >
> > > > > On 05/11/2023 21.03, Simon Glass wrote:
> > > > > > This little series corrects a problem I noticed with arm64 images,
> > > > > > where the kernel is not recognised:
> > > > >
> > > > > The $subject is misleading, bootm works just fine with compressed arm64
> > > > > images, with the type set to "kernel".
> > > > >
> > > > > > Type: Kernel Image (no loading done)
> > > > > > Compression: gzip compressed
> > > > >
> > > > > Isn't that a non-sensical combination to begin with? Decompressing the
> > > > > Image.gz kernel image to any location (however you determine that
> > > > > destination) _is_ loading it.
> > > >
> > > > Yes, I agree.
> > > >
> > > > >
> > > > > If you want XIP, obviously the image must be uncompressed in the FIT. I
> > > > > don't understand what you're trying to do here.
> > > >
> > > > Hmmm, I think I have just got confused about all of this, perhaps
> > > > because ChromeOS uses kernel_noload with compression. Is that an
> > > > invalid combination?
> > >
> > > Yes, that sounds like an invalid combination.
> > >
> > > > But then how does loading actually work? We don't want to put the load
> > > > address in the FIT, since we don't know what it is...we want to use
> > > > the address provided by the board. Which is kernel_noload...so how
> > > > should this be implemented?
> > >
> > > What do you mean, provided by the board? With kernel_noload we XIP the
> > > payload, because the board provided (by loading us to) a safe place to
> > > execute whatevers in there. In the olden days, that would mean (almost
> > > certainly) the zImage which in turn was already compressed and
> > > self-relocating. I know technically one could use the raw vmlinux
> > > instead. With the Linux Kernel and ARCH=arm64 (and a few other arches
> > > now too), they dropped the self-decompression part and the whole payload
> > > must be decompressed. We handle this case in "booti" today by having to
> > > have the board (via environment) say where to decompress to (and how
> > > much space is available). Then we move it back to where we started from,
> > > which is likely not necessary.
> > >
> > > Looking at
> > > https://www.kernel.org/doc/html/latest/arch/arm64/booting.html and
> > > https://www.kernel.org/doc/html/latest/riscv/boot-image-header.html
> > > stating "we're just like arm64" we can do better than we do today for
> > > this format of OS image. If aren't compressed, we only need to ensure
> > > that we are correctly aligned and move (and tell the user) if not. We
> > > can even put the 2MB check under some legacy kernel CONFIG option (3.17
> > > is over 9 years old). With respect to automatic decompression, if we
> > > don't have something telling us where a buffer is and we can't pull from
> > > the environment, we should tell the user and stop?
> >
> > With the booti command, the kern_comp_addr_r and kern_comp_size vars
> > are used. Perhaps I should update my patch to use those too?
>
> As a fallback, perhaps? But what do we do today in a FIT to handle
> compression = "gzip" to know where to uncompress things to?
In that case we decompress to the (kernel) load address in the FIT.
>
> > Do you understand what I am asking about, with bootm? It should be
> > possible to ignore the load address. In fact, load addresses in FIT
> > are mostly bad these days, IMO. If this is not kernel_noload, then
> > what should we use?
>
> Well, your use case needs to be valid. I don't think "kernel_noload" is
> valid with anything other than 'compression = "none"' because that's our
> just execute things in place option. It's very dangerous to use and I
> do not recall right now if we fixed the case where it will just let the
> device tree be badly aligned and break the kernel, or if we say "no
> load, but still fix THAT alignment". And your use case isn't "ignore the
> load address" it's "ignore the load address and decompress the
> payload(s)". That needs _something_ to be known somehow. And I'm asking
> how we do that today even, before saying we can or can't support
> "kernel_noload" along with a compression of the payload.
OK I see.
>
> > "provided by the board" means that the board knows the memory map, not
> > the FIT, so only the board can provide the addresses to use for
> > loading the kernel.
>
> To be clearer, where does the board document that knowledge and provide
> it to be used? I'm not a super fan of environment variables especially
> since we aren't always consistent with '_' or just nothing between
> "words" of the variable. We have much better documentation these days
> (once again, thanks everyone) so we can document environment variables
> used here, and also what CONFIG options are used (as
> CONFIG_SYS_BOOTM_LEN is part of the equation here and badly named as
> been said in other threads about issues wrt decomp and overlap).
I agree about env vars. We have CONFIG_SYS_LOAD_ADDR which seems like
a better model to me.
At the moment we have these, which have carried over from the
distroboot scripts:
https://u-boot.readthedocs.io/en/latest/develop/bootstd.html#environment-variables
So I think we should be using kern_comp_addr_r as the buffer to
decompress to, with kernel_noload. But as you say, having env vars
affect bootm (and boot ) is not ideal.
Another thing we need to clarify is what kernel_noload means. I have
taken it to mean 'ignore the load address in the FIT', not 'no loading
is done', since if compression is enabled, we have to load it to
somewhere.
IMO 'ignore the load in the FIT' should be the normal situation with bootstd.
Regards,
Simon
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 0/4] bootm: Handle compressed arm64 images with bootm
2023-11-07 14:30 ` Simon Glass
@ 2023-11-07 19:04 ` Tom Rini
0 siblings, 0 replies; 21+ messages in thread
From: Tom Rini @ 2023-11-07 19:04 UTC (permalink / raw)
To: Simon Glass
Cc: Rasmus Villemoes, U-Boot Mailing List, Marek Vasut, Eddie James,
Heinrich Schuchardt, Ilias Apalodimas, Joe Hershberger,
Marc Kleine-Budde, Marek Vasut, Mattijs Korpershoek,
Ralph Siemsen, Safae Ouajih, Sean Anderson, Sean Anderson
[-- Attachment #1: Type: text/plain, Size: 9237 bytes --]
On Tue, Nov 07, 2023 at 07:30:33AM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Tue, 7 Nov 2023 at 06:49, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Tue, Nov 07, 2023 at 06:31:15AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Tue, 7 Nov 2023 at 06:04, Tom Rini <trini@konsulko.com> wrote:
> > > >
> > > > On Tue, Nov 07, 2023 at 05:23:05AM -0700, Simon Glass wrote:
> > > > > Hi Rasmus,
> > > > >
> > > > > On Tue, 7 Nov 2023 at 02:56, Rasmus Villemoes
> > > > > <rasmus.villemoes@prevas.dk> wrote:
> > > > > >
> > > > > > On 05/11/2023 21.03, Simon Glass wrote:
> > > > > > > This little series corrects a problem I noticed with arm64 images,
> > > > > > > where the kernel is not recognised:
> > > > > >
> > > > > > The $subject is misleading, bootm works just fine with compressed arm64
> > > > > > images, with the type set to "kernel".
> > > > > >
> > > > > > > Type: Kernel Image (no loading done)
> > > > > > > Compression: gzip compressed
> > > > > >
> > > > > > Isn't that a non-sensical combination to begin with? Decompressing the
> > > > > > Image.gz kernel image to any location (however you determine that
> > > > > > destination) _is_ loading it.
> > > > >
> > > > > Yes, I agree.
> > > > >
> > > > > >
> > > > > > If you want XIP, obviously the image must be uncompressed in the FIT. I
> > > > > > don't understand what you're trying to do here.
> > > > >
> > > > > Hmmm, I think I have just got confused about all of this, perhaps
> > > > > because ChromeOS uses kernel_noload with compression. Is that an
> > > > > invalid combination?
> > > >
> > > > Yes, that sounds like an invalid combination.
> > > >
> > > > > But then how does loading actually work? We don't want to put the load
> > > > > address in the FIT, since we don't know what it is...we want to use
> > > > > the address provided by the board. Which is kernel_noload...so how
> > > > > should this be implemented?
> > > >
> > > > What do you mean, provided by the board? With kernel_noload we XIP the
> > > > payload, because the board provided (by loading us to) a safe place to
> > > > execute whatevers in there. In the olden days, that would mean (almost
> > > > certainly) the zImage which in turn was already compressed and
> > > > self-relocating. I know technically one could use the raw vmlinux
> > > > instead. With the Linux Kernel and ARCH=arm64 (and a few other arches
> > > > now too), they dropped the self-decompression part and the whole payload
> > > > must be decompressed. We handle this case in "booti" today by having to
> > > > have the board (via environment) say where to decompress to (and how
> > > > much space is available). Then we move it back to where we started from,
> > > > which is likely not necessary.
> > > >
> > > > Looking at
> > > > https://www.kernel.org/doc/html/latest/arch/arm64/booting.html and
> > > > https://www.kernel.org/doc/html/latest/riscv/boot-image-header.html
> > > > stating "we're just like arm64" we can do better than we do today for
> > > > this format of OS image. If aren't compressed, we only need to ensure
> > > > that we are correctly aligned and move (and tell the user) if not. We
> > > > can even put the 2MB check under some legacy kernel CONFIG option (3.17
> > > > is over 9 years old). With respect to automatic decompression, if we
> > > > don't have something telling us where a buffer is and we can't pull from
> > > > the environment, we should tell the user and stop?
> > >
> > > With the booti command, the kern_comp_addr_r and kern_comp_size vars
> > > are used. Perhaps I should update my patch to use those too?
> >
> > As a fallback, perhaps? But what do we do today in a FIT to handle
> > compression = "gzip" to know where to uncompress things to?
>
> In that case we decompress to the (kernel) load address in the FIT.
And handle overlap, so there's some small window where you just need to
not load the FIT to memory at that address as it's just Not Possible,
yes? I think that's how I recall how it works out.
> > > Do you understand what I am asking about, with bootm? It should be
> > > possible to ignore the load address. In fact, load addresses in FIT
> > > are mostly bad these days, IMO. If this is not kernel_noload, then
> > > what should we use?
> >
> > Well, your use case needs to be valid. I don't think "kernel_noload" is
> > valid with anything other than 'compression = "none"' because that's our
> > just execute things in place option. It's very dangerous to use and I
> > do not recall right now if we fixed the case where it will just let the
> > device tree be badly aligned and break the kernel, or if we say "no
> > load, but still fix THAT alignment". And your use case isn't "ignore the
> > load address" it's "ignore the load address and decompress the
> > payload(s)". That needs _something_ to be known somehow. And I'm asking
> > how we do that today even, before saying we can or can't support
> > "kernel_noload" along with a compression of the payload.
>
> OK I see.
>
> >
> > > "provided by the board" means that the board knows the memory map, not
> > > the FIT, so only the board can provide the addresses to use for
> > > loading the kernel.
> >
> > To be clearer, where does the board document that knowledge and provide
> > it to be used? I'm not a super fan of environment variables especially
> > since we aren't always consistent with '_' or just nothing between
> > "words" of the variable. We have much better documentation these days
> > (once again, thanks everyone) so we can document environment variables
> > used here, and also what CONFIG options are used (as
> > CONFIG_SYS_BOOTM_LEN is part of the equation here and badly named as
> > been said in other threads about issues wrt decomp and overlap).
>
> I agree about env vars. We have CONFIG_SYS_LOAD_ADDR which seems like
> a better model to me.
Maybe? Looking at the Kconfig entry for SYS_LOAD_ADDR the defaults here
are some really really really long-standing inherited values (why so
many times are we at base of DRAM + 32MB? It was probably good enough to
not be where we are or were running). Sometimes we then use that for
kernel_addr_r, sometimes we don't, and in turn that's because sometimes
we thought harder about how to avoid memory moves, and sometimes didn't.
I'm not immediately sure if this is a good idea to re-use, or a bad idea
to re-use, especially if we're then assuming the rest of the use case is
XIP.
> At the moment we have these, which have carried over from the
> distroboot scripts:
>
> https://u-boot.readthedocs.io/en/latest/develop/bootstd.html#environment-variables
>
> So I think we should be using kern_comp_addr_r as the buffer to
> decompress to
In that a lot of effort has been put in to documenting and setting these
variables, yes, re-using that variable as where FIT can be told where to
uncompress to and how much if we have no other value, is good.
> , with kernel_noload. But as you say, having env vars
> affect bootm (and boot ) is not ideal.
I'm still not sure about kernel_noload.
> Another thing we need to clarify is what kernel_noload means.
Yes, we do. I think it's unfortunately on the list of things that
weren't documented well enough in FIT and people assumed what it did and
didn't actually do.
> I have
> taken it to mean 'ignore the load address in the FIT', not 'no loading
> is done', since if compression is enabled, we have to load it to
> somewhere.
OK, but that's not what it means in code. The first step is to document
what cases do, and do not, work in a FIT image today, for a few
different payload options, probably at least
kernel/ramdisk/flatdt/kernel_noload and how it handles compression. I
know at one point there was some confusion about what happens when you
pass a ramdisk.gz and say compression = "gzip" and then what people
expected that to mean instead of what it did, and so what it does right
now.
> IMO 'ignore the load in the FIT' should be the normal situation with bootstd.
Oh no. That's getting things in the wrong order. We need to document
what kernel_noload actually does, today.
And we need to also look harder at our code for handling
Linux-Kernel-arm64-and-riscv-Image files because in the non-compressed
case the Image is probably at an XIP location (2MiB alignment is pretty
likely, we do need to check of course) and the initrd can be wherever,
and we only need to ensure the device tree is 8-byte aligned in memory.
So I think we may have some not-required moving happening at least in
the compressed case.
Then we can see if kernel_noload can sanely support something other than
'compression = "none"'.
Then whatever bootmeth is looking for and finding FIT images will take
whatever valid FIT images are found and boot them. And that may or may
not include having an Image that's compressed. I see that for example
the default v6.6 arm64 kernel has ~1k modules, but the Fedora 38 kernel
has ~4500 and so the distro Image is about the same size as the v6.6
Image.gz. As just a general size example.
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2023-11-07 19:04 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-11-05 20:03 [PATCH 0/4] bootm: Handle compressed arm64 images with bootm Simon Glass
2023-11-05 20:03 ` [PATCH 1/4] bootm: Allow ignoring the load address with kernel_noload Simon Glass
2023-11-05 21:19 ` Tom Rini
2023-11-06 17:25 ` Simon Glass
2023-11-06 18:30 ` Tom Rini
2023-11-06 19:58 ` Simon Glass
2023-11-06 20:15 ` Tom Rini
2023-11-07 1:09 ` Simon Glass
2023-11-05 20:03 ` [PATCH 2/4] bootm: Move arm64-image processing later Simon Glass
2023-11-05 21:20 ` Tom Rini
2023-11-06 17:25 ` Simon Glass
2023-11-05 20:03 ` [PATCH 3/4] image: Show the load address when decompressing Simon Glass
2023-11-05 20:03 ` [PATCH 4/4] image: Correct load_bug typo Simon Glass
2023-11-05 20:06 ` [PATCH 0/4] bootm: Handle compressed arm64 images with bootm Simon Glass
2023-11-07 9:56 ` Rasmus Villemoes
2023-11-07 12:23 ` Simon Glass
2023-11-07 13:04 ` Tom Rini
2023-11-07 13:31 ` Simon Glass
2023-11-07 13:49 ` Tom Rini
2023-11-07 14:30 ` Simon Glass
2023-11-07 19:04 ` Tom Rini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.