* [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully
@ 2019-05-03 17:13 Peter Maydell
2019-05-03 17:13 ` Peter Maydell
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Peter Maydell @ 2019-05-03 17:13 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Mark Rutland
This patchset attempts to fix https://bugs.launchpad.net/qemu/+bug/1823998
which reports that we don't handle kernels larger than 128MB
correctly, because we allow the initrd to be placed over the
tail end of the kernel. AArch64 kernel Image files (since v3.17)
report the total size they require (including any BSS area that
isn't in the Image itself), so we can use that to be sure we
place the initrd sufficiently far into the RAM.
Patch 1 in this series adjusts our "where do we put the initrd"
heuristic so that it always places it at least after whatever
our best guess at the kernel size is. (This might still not
be right for images like self-decompressing 32-bit kernels, where
there's no way to know how big the kernel will be after
decompression.) Patch 2 makes load_aarch64_image() return the
kernel size as indicated in the Image file header, so that for
the specific case of AArch64 Image files we will definitely not
put the initrd on top of them.
I've given this a quick smoke test but I don't have a very large
Image kernel to hand, so testing appreciated.
thanks
-- PMM
Peter Maydell (2):
hw/arm/boot: Avoid placing the initrd on top of the kernel
hw/arm/boot: Honour image size field in AArch64 Image format kernels
hw/arm/boot.c | 51 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 35 insertions(+), 16 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully
2019-05-03 17:13 [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully Peter Maydell
@ 2019-05-03 17:13 ` Peter Maydell
2019-05-03 17:13 ` [Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel Peter Maydell
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2019-05-03 17:13 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Mark Rutland
This patchset attempts to fix https://bugs.launchpad.net/qemu/+bug/1823998
which reports that we don't handle kernels larger than 128MB
correctly, because we allow the initrd to be placed over the
tail end of the kernel. AArch64 kernel Image files (since v3.17)
report the total size they require (including any BSS area that
isn't in the Image itself), so we can use that to be sure we
place the initrd sufficiently far into the RAM.
Patch 1 in this series adjusts our "where do we put the initrd"
heuristic so that it always places it at least after whatever
our best guess at the kernel size is. (This might still not
be right for images like self-decompressing 32-bit kernels, where
there's no way to know how big the kernel will be after
decompression.) Patch 2 makes load_aarch64_image() return the
kernel size as indicated in the Image file header, so that for
the specific case of AArch64 Image files we will definitely not
put the initrd on top of them.
I've given this a quick smoke test but I don't have a very large
Image kernel to hand, so testing appreciated.
thanks
-- PMM
Peter Maydell (2):
hw/arm/boot: Avoid placing the initrd on top of the kernel
hw/arm/boot: Honour image size field in AArch64 Image format kernels
hw/arm/boot.c | 51 +++++++++++++++++++++++++++++++++++----------------
1 file changed, 35 insertions(+), 16 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel
2019-05-03 17:13 [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully Peter Maydell
2019-05-03 17:13 ` Peter Maydell
@ 2019-05-03 17:13 ` Peter Maydell
2019-05-03 17:13 ` Peter Maydell
2019-05-03 17:33 ` Peter Maydell
2019-05-03 17:13 ` [Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels Peter Maydell
2019-05-09 16:35 ` [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully Mark Rutland
3 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2019-05-03 17:13 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Mark Rutland
We currently put the initrd at the smaller of:
* 128MB into RAM
* halfway into the RAM
(with the dtb following it).
However for large kernels this might mean that the kernel
overlaps the initrd. For some kinds of kernel (self-decompressing
32-bit kernels, and ELF images with a BSS section at the end)
we don't know the exact size, but even there we have a
minimum size. Put the initrd at least further into RAM than
that. For image formats that can give us an exact kernel size, this
will mean that we definitely avoid overlaying kernel and initrd.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/boot.c | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index a830655e1af..7c978fedde4 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -998,20 +998,6 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
if (info->nb_cpus == 0)
info->nb_cpus = 1;
- /*
- * We want to put the initrd far enough into RAM that when the
- * kernel is uncompressed it will not clobber the initrd. However
- * on boards without much RAM we must ensure that we still leave
- * enough room for a decent sized initrd, and on boards with large
- * amounts of RAM we must avoid the initrd being so far up in RAM
- * that it is outside lowmem and inaccessible to the kernel.
- * So for boards with less than 256MB of RAM we put the initrd
- * halfway into RAM, and for boards with 256MB of RAM or more we put
- * the initrd at 128MB.
- */
- info->initrd_start = info->loader_start +
- MIN(info->ram_size / 2, 128 * 1024 * 1024);
-
/* Assume that raw images are linux kernels, and ELF images are not. */
kernel_size = arm_load_elf(info, &elf_entry, &elf_low_addr,
&elf_high_addr, elf_machine, as);
@@ -1056,6 +1042,26 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
exit(1);
}
info->entry = entry;
+
+ /*
+ * We want to put the initrd far enough into RAM that when the
+ * kernel is uncompressed it will not clobber the initrd. However
+ * on boards without much RAM we must ensure that we still leave
+ * enough room for a decent sized initrd, and on boards with large
+ * amounts of RAM we must avoid the initrd being so far up in RAM
+ * that it is outside lowmem and inaccessible to the kernel.
+ * So for boards with less than 256MB of RAM we put the initrd
+ * halfway into RAM, and for boards with 256MB of RAM or more we put
+ * the initrd at 128MB.
+ * We also refuse to put the initrd somewhere that will definitely
+ * overlay the kernel we just loaded, though for kernel formats which
+ * don't tell us their exact size (eg self-decompressing 32-bit kernels)
+ * we might still make a bad choice here.
+ */
+ info->initrd_start = info->loader_start +
+ MAX(MIN(info->ram_size / 2, 128 * 1024 * 1024), kernel_size);
+ info->initrd_start = TARGET_PAGE_ALIGN(info->initrd_start);
+
if (is_linux) {
uint32_t fixupcontext[FIXUP_MAX];
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel
2019-05-03 17:13 ` [Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel Peter Maydell
@ 2019-05-03 17:13 ` Peter Maydell
2019-05-03 17:33 ` Peter Maydell
1 sibling, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2019-05-03 17:13 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Mark Rutland
We currently put the initrd at the smaller of:
* 128MB into RAM
* halfway into the RAM
(with the dtb following it).
However for large kernels this might mean that the kernel
overlaps the initrd. For some kinds of kernel (self-decompressing
32-bit kernels, and ELF images with a BSS section at the end)
we don't know the exact size, but even there we have a
minimum size. Put the initrd at least further into RAM than
that. For image formats that can give us an exact kernel size, this
will mean that we definitely avoid overlaying kernel and initrd.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/boot.c | 34 ++++++++++++++++++++--------------
1 file changed, 20 insertions(+), 14 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index a830655e1af..7c978fedde4 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -998,20 +998,6 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
if (info->nb_cpus == 0)
info->nb_cpus = 1;
- /*
- * We want to put the initrd far enough into RAM that when the
- * kernel is uncompressed it will not clobber the initrd. However
- * on boards without much RAM we must ensure that we still leave
- * enough room for a decent sized initrd, and on boards with large
- * amounts of RAM we must avoid the initrd being so far up in RAM
- * that it is outside lowmem and inaccessible to the kernel.
- * So for boards with less than 256MB of RAM we put the initrd
- * halfway into RAM, and for boards with 256MB of RAM or more we put
- * the initrd at 128MB.
- */
- info->initrd_start = info->loader_start +
- MIN(info->ram_size / 2, 128 * 1024 * 1024);
-
/* Assume that raw images are linux kernels, and ELF images are not. */
kernel_size = arm_load_elf(info, &elf_entry, &elf_low_addr,
&elf_high_addr, elf_machine, as);
@@ -1056,6 +1042,26 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
exit(1);
}
info->entry = entry;
+
+ /*
+ * We want to put the initrd far enough into RAM that when the
+ * kernel is uncompressed it will not clobber the initrd. However
+ * on boards without much RAM we must ensure that we still leave
+ * enough room for a decent sized initrd, and on boards with large
+ * amounts of RAM we must avoid the initrd being so far up in RAM
+ * that it is outside lowmem and inaccessible to the kernel.
+ * So for boards with less than 256MB of RAM we put the initrd
+ * halfway into RAM, and for boards with 256MB of RAM or more we put
+ * the initrd at 128MB.
+ * We also refuse to put the initrd somewhere that will definitely
+ * overlay the kernel we just loaded, though for kernel formats which
+ * don't tell us their exact size (eg self-decompressing 32-bit kernels)
+ * we might still make a bad choice here.
+ */
+ info->initrd_start = info->loader_start +
+ MAX(MIN(info->ram_size / 2, 128 * 1024 * 1024), kernel_size);
+ info->initrd_start = TARGET_PAGE_ALIGN(info->initrd_start);
+
if (is_linux) {
uint32_t fixupcontext[FIXUP_MAX];
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels
2019-05-03 17:13 [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully Peter Maydell
2019-05-03 17:13 ` Peter Maydell
2019-05-03 17:13 ` [Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel Peter Maydell
@ 2019-05-03 17:13 ` Peter Maydell
2019-05-03 17:13 ` Peter Maydell
2019-05-04 3:54 ` Richard Henderson
2019-05-09 16:35 ` [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully Mark Rutland
3 siblings, 2 replies; 11+ messages in thread
From: Peter Maydell @ 2019-05-03 17:13 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Mark Rutland
Since Linux v3.17, the kernel's Image header includes a field image_size,
which gives the total size of the kernel including unpopulated data
sections such as the BSS). If this is present, then return it from
load_aarch64_image() as the true size of the kernel rather than
just using the size of the Image file itself. This allows the code
which calculates where to put the initrd to avoid putting it in
the kernel's BSS area.
This means that we should be able to reliably load kernel images
which are larger than 128MB without accidentally putting the
initrd or dtb in locations that clash with the kernel itself.
Fixes: https://bugs.launchpad.net/qemu/+bug/1823998
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/boot.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 7c978fedde4..34bdd151df8 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -910,6 +910,7 @@ static uint64_t load_aarch64_image(const char *filename, hwaddr mem_base,
hwaddr *entry, AddressSpace *as)
{
hwaddr kernel_load_offset = KERNEL64_LOAD_ADDR;
+ uint64_t kernel_size = 0;
uint8_t *buffer;
int size;
@@ -937,7 +938,10 @@ static uint64_t load_aarch64_image(const char *filename, hwaddr mem_base,
* is only valid if the image_size is non-zero.
*/
memcpy(&hdrvals, buffer + ARM64_TEXT_OFFSET_OFFSET, sizeof(hdrvals));
- if (hdrvals[1] != 0) {
+
+ kernel_size = le64_to_cpu(hdrvals[1]);
+
+ if (kernel_size != 0) {
kernel_load_offset = le64_to_cpu(hdrvals[0]);
/*
@@ -955,12 +959,21 @@ static uint64_t load_aarch64_image(const char *filename, hwaddr mem_base,
}
}
+ /*
+ * Kernels before v3.17 don't populate the image_size field, and
+ * raw images have no header. For those our best guess at the size
+ * is the size of the Image file itself.
+ */
+ if (kernel_size == 0) {
+ kernel_size = size;
+ }
+
*entry = mem_base + kernel_load_offset;
rom_add_blob_fixed_as(filename, buffer, size, *entry, as);
g_free(buffer);
- return size;
+ return kernel_size;
}
static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels
2019-05-03 17:13 ` [Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels Peter Maydell
@ 2019-05-03 17:13 ` Peter Maydell
2019-05-04 3:54 ` Richard Henderson
1 sibling, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2019-05-03 17:13 UTC (permalink / raw)
To: qemu-arm, qemu-devel; +Cc: Mark Rutland
Since Linux v3.17, the kernel's Image header includes a field image_size,
which gives the total size of the kernel including unpopulated data
sections such as the BSS). If this is present, then return it from
load_aarch64_image() as the true size of the kernel rather than
just using the size of the Image file itself. This allows the code
which calculates where to put the initrd to avoid putting it in
the kernel's BSS area.
This means that we should be able to reliably load kernel images
which are larger than 128MB without accidentally putting the
initrd or dtb in locations that clash with the kernel itself.
Fixes: https://bugs.launchpad.net/qemu/+bug/1823998
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
hw/arm/boot.c | 17 +++++++++++++++--
1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 7c978fedde4..34bdd151df8 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -910,6 +910,7 @@ static uint64_t load_aarch64_image(const char *filename, hwaddr mem_base,
hwaddr *entry, AddressSpace *as)
{
hwaddr kernel_load_offset = KERNEL64_LOAD_ADDR;
+ uint64_t kernel_size = 0;
uint8_t *buffer;
int size;
@@ -937,7 +938,10 @@ static uint64_t load_aarch64_image(const char *filename, hwaddr mem_base,
* is only valid if the image_size is non-zero.
*/
memcpy(&hdrvals, buffer + ARM64_TEXT_OFFSET_OFFSET, sizeof(hdrvals));
- if (hdrvals[1] != 0) {
+
+ kernel_size = le64_to_cpu(hdrvals[1]);
+
+ if (kernel_size != 0) {
kernel_load_offset = le64_to_cpu(hdrvals[0]);
/*
@@ -955,12 +959,21 @@ static uint64_t load_aarch64_image(const char *filename, hwaddr mem_base,
}
}
+ /*
+ * Kernels before v3.17 don't populate the image_size field, and
+ * raw images have no header. For those our best guess at the size
+ * is the size of the Image file itself.
+ */
+ if (kernel_size == 0) {
+ kernel_size = size;
+ }
+
*entry = mem_base + kernel_load_offset;
rom_add_blob_fixed_as(filename, buffer, size, *entry, as);
g_free(buffer);
- return size;
+ return kernel_size;
}
static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel
2019-05-03 17:13 ` [Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel Peter Maydell
2019-05-03 17:13 ` Peter Maydell
@ 2019-05-03 17:33 ` Peter Maydell
2019-05-03 17:33 ` Peter Maydell
1 sibling, 1 reply; 11+ messages in thread
From: Peter Maydell @ 2019-05-03 17:33 UTC (permalink / raw)
To: qemu-arm, QEMU Developers; +Cc: Mark Rutland
On Fri, 3 May 2019 at 18:13, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> We currently put the initrd at the smaller of:
> * 128MB into RAM
> * halfway into the RAM
> (with the dtb following it).
>
> However for large kernels this might mean that the kernel
> overlaps the initrd. For some kinds of kernel (self-decompressing
> 32-bit kernels, and ELF images with a BSS section at the end)
> we don't know the exact size, but even there we have a
> minimum size. Put the initrd at least further into RAM than
> that. For image formats that can give us an exact kernel size, this
> will mean that we definitely avoid overlaying kernel and initrd.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/boot.c | 34 ++++++++++++++++++++--------------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index a830655e1af..7c978fedde4 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -998,20 +998,6 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> if (info->nb_cpus == 0)
> info->nb_cpus = 1;
>
> - /*
> - * We want to put the initrd far enough into RAM that when the
> - * kernel is uncompressed it will not clobber the initrd. However
> - * on boards without much RAM we must ensure that we still leave
> - * enough room for a decent sized initrd, and on boards with large
> - * amounts of RAM we must avoid the initrd being so far up in RAM
> - * that it is outside lowmem and inaccessible to the kernel.
> - * So for boards with less than 256MB of RAM we put the initrd
> - * halfway into RAM, and for boards with 256MB of RAM or more we put
> - * the initrd at 128MB.
> - */
> - info->initrd_start = info->loader_start +
> - MIN(info->ram_size / 2, 128 * 1024 * 1024);
> -
> /* Assume that raw images are linux kernels, and ELF images are not. */
> kernel_size = arm_load_elf(info, &elf_entry, &elf_low_addr,
> &elf_high_addr, elf_machine, as);
> @@ -1056,6 +1042,26 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> exit(1);
> }
> info->entry = entry;
> +
> + /*
> + * We want to put the initrd far enough into RAM that when the
> + * kernel is uncompressed it will not clobber the initrd. However
> + * on boards without much RAM we must ensure that we still leave
> + * enough room for a decent sized initrd, and on boards with large
> + * amounts of RAM we must avoid the initrd being so far up in RAM
> + * that it is outside lowmem and inaccessible to the kernel.
> + * So for boards with less than 256MB of RAM we put the initrd
> + * halfway into RAM, and for boards with 256MB of RAM or more we put
> + * the initrd at 128MB.
> + * We also refuse to put the initrd somewhere that will definitely
> + * overlay the kernel we just loaded, though for kernel formats which
> + * don't tell us their exact size (eg self-decompressing 32-bit kernels)
> + * we might still make a bad choice here.
> + */
> + info->initrd_start = info->loader_start +
> + MAX(MIN(info->ram_size / 2, 128 * 1024 * 1024), kernel_size);
> + info->initrd_start = TARGET_PAGE_ALIGN(info->initrd_start);
> +
I belatedly realized that we should probably check here whether
this is off the end of the ram, as otherwise following code
will get an underflow in "info->ram_size - info->initrd_start"
which is unlikely to result in useful behaviour.
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel
2019-05-03 17:33 ` Peter Maydell
@ 2019-05-03 17:33 ` Peter Maydell
0 siblings, 0 replies; 11+ messages in thread
From: Peter Maydell @ 2019-05-03 17:33 UTC (permalink / raw)
To: qemu-arm, QEMU Developers; +Cc: Mark Rutland
On Fri, 3 May 2019 at 18:13, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> We currently put the initrd at the smaller of:
> * 128MB into RAM
> * halfway into the RAM
> (with the dtb following it).
>
> However for large kernels this might mean that the kernel
> overlaps the initrd. For some kinds of kernel (self-decompressing
> 32-bit kernels, and ELF images with a BSS section at the end)
> we don't know the exact size, but even there we have a
> minimum size. Put the initrd at least further into RAM than
> that. For image formats that can give us an exact kernel size, this
> will mean that we definitely avoid overlaying kernel and initrd.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/boot.c | 34 ++++++++++++++++++++--------------
> 1 file changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index a830655e1af..7c978fedde4 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -998,20 +998,6 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> if (info->nb_cpus == 0)
> info->nb_cpus = 1;
>
> - /*
> - * We want to put the initrd far enough into RAM that when the
> - * kernel is uncompressed it will not clobber the initrd. However
> - * on boards without much RAM we must ensure that we still leave
> - * enough room for a decent sized initrd, and on boards with large
> - * amounts of RAM we must avoid the initrd being so far up in RAM
> - * that it is outside lowmem and inaccessible to the kernel.
> - * So for boards with less than 256MB of RAM we put the initrd
> - * halfway into RAM, and for boards with 256MB of RAM or more we put
> - * the initrd at 128MB.
> - */
> - info->initrd_start = info->loader_start +
> - MIN(info->ram_size / 2, 128 * 1024 * 1024);
> -
> /* Assume that raw images are linux kernels, and ELF images are not. */
> kernel_size = arm_load_elf(info, &elf_entry, &elf_low_addr,
> &elf_high_addr, elf_machine, as);
> @@ -1056,6 +1042,26 @@ static void arm_setup_direct_kernel_boot(ARMCPU *cpu,
> exit(1);
> }
> info->entry = entry;
> +
> + /*
> + * We want to put the initrd far enough into RAM that when the
> + * kernel is uncompressed it will not clobber the initrd. However
> + * on boards without much RAM we must ensure that we still leave
> + * enough room for a decent sized initrd, and on boards with large
> + * amounts of RAM we must avoid the initrd being so far up in RAM
> + * that it is outside lowmem and inaccessible to the kernel.
> + * So for boards with less than 256MB of RAM we put the initrd
> + * halfway into RAM, and for boards with 256MB of RAM or more we put
> + * the initrd at 128MB.
> + * We also refuse to put the initrd somewhere that will definitely
> + * overlay the kernel we just loaded, though for kernel formats which
> + * don't tell us their exact size (eg self-decompressing 32-bit kernels)
> + * we might still make a bad choice here.
> + */
> + info->initrd_start = info->loader_start +
> + MAX(MIN(info->ram_size / 2, 128 * 1024 * 1024), kernel_size);
> + info->initrd_start = TARGET_PAGE_ALIGN(info->initrd_start);
> +
I belatedly realized that we should probably check here whether
this is off the end of the ram, as otherwise following code
will get an underflow in "info->ram_size - info->initrd_start"
which is unlikely to result in useful behaviour.
thanks
-- PMM
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels
2019-05-03 17:13 ` [Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels Peter Maydell
2019-05-03 17:13 ` Peter Maydell
@ 2019-05-04 3:54 ` Richard Henderson
2019-05-04 3:54 ` Richard Henderson
1 sibling, 1 reply; 11+ messages in thread
From: Richard Henderson @ 2019-05-04 3:54 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Mark Rutland
On 5/3/19 10:13 AM, Peter Maydell wrote:
> Since Linux v3.17, the kernel's Image header includes a field image_size,
> which gives the total size of the kernel including unpopulated data
> sections such as the BSS). If this is present, then return it from
> load_aarch64_image() as the true size of the kernel rather than
> just using the size of the Image file itself. This allows the code
> which calculates where to put the initrd to avoid putting it in
> the kernel's BSS area.
>
> This means that we should be able to reliably load kernel images
> which are larger than 128MB without accidentally putting the
> initrd or dtb in locations that clash with the kernel itself.
>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1823998
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/boot.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels
2019-05-04 3:54 ` Richard Henderson
@ 2019-05-04 3:54 ` Richard Henderson
0 siblings, 0 replies; 11+ messages in thread
From: Richard Henderson @ 2019-05-04 3:54 UTC (permalink / raw)
To: Peter Maydell, qemu-arm, qemu-devel; +Cc: Mark Rutland
On 5/3/19 10:13 AM, Peter Maydell wrote:
> Since Linux v3.17, the kernel's Image header includes a field image_size,
> which gives the total size of the kernel including unpopulated data
> sections such as the BSS). If this is present, then return it from
> load_aarch64_image() as the true size of the kernel rather than
> just using the size of the Image file itself. This allows the code
> which calculates where to put the initrd to avoid putting it in
> the kernel's BSS area.
>
> This means that we should be able to reliably load kernel images
> which are larger than 128MB without accidentally putting the
> initrd or dtb in locations that clash with the kernel itself.
>
> Fixes: https://bugs.launchpad.net/qemu/+bug/1823998
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> hw/arm/boot.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully
2019-05-03 17:13 [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully Peter Maydell
` (2 preceding siblings ...)
2019-05-03 17:13 ` [Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels Peter Maydell
@ 2019-05-09 16:35 ` Mark Rutland
3 siblings, 0 replies; 11+ messages in thread
From: Mark Rutland @ 2019-05-09 16:35 UTC (permalink / raw)
To: Peter Maydell; +Cc: qemu-arm, qemu-devel
On Fri, May 03, 2019 at 06:13:45PM +0100, Peter Maydell wrote:
> This patchset attempts to fix https://bugs.launchpad.net/qemu/+bug/1823998
> which reports that we don't handle kernels larger than 128MB
> correctly, because we allow the initrd to be placed over the
> tail end of the kernel. AArch64 kernel Image files (since v3.17)
> report the total size they require (including any BSS area that
> isn't in the Image itself), so we can use that to be sure we
> place the initrd sufficiently far into the RAM.
>
> Patch 1 in this series adjusts our "where do we put the initrd"
> heuristic so that it always places it at least after whatever
> our best guess at the kernel size is. (This might still not
> be right for images like self-decompressing 32-bit kernels, where
> there's no way to know how big the kernel will be after
> decompression.) Patch 2 makes load_aarch64_image() return the
> kernel size as indicated in the Image file header, so that for
> the specific case of AArch64 Image files we will definitely not
> put the initrd on top of them.
>
> I've given this a quick smoke test but I don't have a very large
> Image kernel to hand, so testing appreciated.
I've just given this a go with three very large images built from v5.1:
* ~113.6MiB raw, ~134.5MiB effective
* ~131.0MiB raw, ~152.0MiB effective
* ~225.6MiB raw, ~247.0MiB effective
Prior to these patches (with pristine QEMU commit c56247e55bde4386) both
would silently fail to boot, and with these patches applied both all
three begin booting and produce console output. The first two get to
userspace, and the third crashes due to an unrelated Linux bug.
So FWIW:
Tested-by: Mark Rutland <mark.rutland@arm.com>
Thanks for putting this together!
Thanks,
Mark.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-05-09 16:37 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-03 17:13 [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully Peter Maydell
2019-05-03 17:13 ` Peter Maydell
2019-05-03 17:13 ` [Qemu-devel] [PATCH 1/2] hw/arm/boot: Avoid placing the initrd on top of the kernel Peter Maydell
2019-05-03 17:13 ` Peter Maydell
2019-05-03 17:33 ` Peter Maydell
2019-05-03 17:33 ` Peter Maydell
2019-05-03 17:13 ` [Qemu-devel] [PATCH 2/2] hw/arm/boot: Honour image size field in AArch64 Image format kernels Peter Maydell
2019-05-03 17:13 ` Peter Maydell
2019-05-04 3:54 ` Richard Henderson
2019-05-04 3:54 ` Richard Henderson
2019-05-09 16:35 ` [Qemu-devel] [PATCH 0/2] hw/arm/boot: handle large Images more gracefully Mark Rutland
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).