[Qemu-devel] [PATCH] [PATCH] Allow AArch64 processors to boot from a kernel placed over 4GB.

Perez Blanco, Ricardo (Nokia - BE/Antwerp) posted 1 patch 6 years, 11 months ago
Test asan passed
Test checkpatch failed
Test docker-quick@centos7 passed
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181126191454.9455-1-ricardo.perez_blanco@nokia.com
There is a newer version of this series
hw/arm/boot.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
[Qemu-devel] [PATCH] [PATCH] Allow AArch64 processors to boot from a kernel placed over 4GB.
Posted by Perez Blanco, Ricardo (Nokia - BE/Antwerp) 6 years, 11 months ago
Some machine based on AArch64 can have its main memory over 4GBs. With
the current path, these machines can support "-kernel" in qemu

Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com>
---
 hw/arm/boot.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 586baa9b64..183c5860bd 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -64,7 +64,9 @@ typedef enum {
     FIXUP_BOARDID,      /* overwrite with board ID number */
     FIXUP_BOARD_SETUP,  /* overwrite with board specific setup code address */
     FIXUP_ARGPTR,       /* overwrite with pointer to kernel args */
+    FIXUP_ARGPTR_HIGHER_32BITS,       /* overwrite with pointer to kernel args (higher 32 bits) */
     FIXUP_ENTRYPOINT,   /* overwrite with kernel entry point */
+    FIXUP_ENTRYPOINT_HIGHER_32BITS,   /* overwrite with kernel entry point (higher 32 bits) */
     FIXUP_GIC_CPU_IF,   /* overwrite with GIC CPU interface address */
     FIXUP_BOOTREG,      /* overwrite with boot register address */
     FIXUP_DSB,          /* overwrite with correct DSB insn for cpu */
@@ -84,9 +86,9 @@ static const ARMInsnFixup bootloader_aarch64[] = {
     { 0x58000084 }, /* ldr x4, entry ; Load the lower 32-bits of kernel entry */
     { 0xd61f0080 }, /* br x4      ; Jump to the kernel entry point */
     { 0, FIXUP_ARGPTR }, /* arg: .word @DTB Lower 32-bits */
-    { 0 }, /* .word @DTB Higher 32-bits */
+    { 0, FIXUP_ARGPTR_HIGHER_32BITS}, /* .word @DTB Higher 32-bits */
     { 0, FIXUP_ENTRYPOINT }, /* entry: .word @Kernel Entry Lower 32-bits */
-    { 0 }, /* .word @Kernel Entry Higher 32-bits */
+    { 0, FIXUP_ENTRYPOINT_HIGHER_32BITS }, /* .word @Kernel Entry Higher 32-bits */
     { 0, FIXUP_TERMINATOR }
 };
 
@@ -175,7 +177,9 @@ static void write_bootloader(const char *name, hwaddr addr,
         case FIXUP_BOARDID:
         case FIXUP_BOARD_SETUP:
         case FIXUP_ARGPTR:
+        case FIXUP_ARGPTR_HIGHER_32BITS:
         case FIXUP_ENTRYPOINT:
+        case FIXUP_ENTRYPOINT_HIGHER_32BITS:
         case FIXUP_GIC_CPU_IF:
         case FIXUP_BOOTREG:
         case FIXUP_DSB:
@@ -939,7 +943,6 @@ static uint64_t load_aarch64_image(const char *filename, hwaddr mem_base,
             }
         }
     }
-
     *entry = mem_base + kernel_load_offset;
     rom_add_blob_fixed_as(filename, buffer, size, *entry, as);
 
@@ -1153,8 +1156,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
             info->dtb_start = QEMU_ALIGN_UP(info->initrd_start + initrd_size,
                                            align);
             fixupcontext[FIXUP_ARGPTR] = info->dtb_start;
+            fixupcontext[FIXUP_ARGPTR_HIGHER_32BITS] = info->dtb_start >> 32;
         } else {
             fixupcontext[FIXUP_ARGPTR] = info->loader_start + KERNEL_ARGS_ADDR;
+            fixupcontext[FIXUP_ARGPTR_HIGHER_32BITS] = (info->loader_start + KERNEL_ARGS_ADDR) >> 32;
             if (info->ram_size >= (1ULL << 32)) {
                 error_report("RAM size must be less than 4GB to boot"
                              " Linux kernel using ATAGS (try passing a device tree"
@@ -1163,6 +1168,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
             }
         }
         fixupcontext[FIXUP_ENTRYPOINT] = entry;
+        fixupcontext[FIXUP_ENTRYPOINT_HIGHER_32BITS] = entry >> 32;
 
         write_bootloader("bootloader", info->loader_start,
                          primary_loader, fixupcontext, as);
-- 
2.14.1


Re: [Qemu-devel] [PATCH] [PATCH] Allow AArch64 processors to boot from a kernel placed over 4GB.
Posted by Peter Maydell 6 years, 11 months ago
On Mon, 26 Nov 2018 at 19:15, Perez Blanco, Ricardo (Nokia -
BE/Antwerp) <ricardo.perez_blanco@nokia.com> wrote:
>
> Some machine based on AArch64 can have its main memory over 4GBs. With
> the current path, these machines can support "-kernel" in qemu
>
> Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com>

Hi; I think it would be worth noting in the commit message that
this doesn't affect any machines QEMU currently emulates.

> ---
>  hw/arm/boot.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 586baa9b64..183c5860bd 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -64,7 +64,9 @@ typedef enum {
>      FIXUP_BOARDID,      /* overwrite with board ID number */
>      FIXUP_BOARD_SETUP,  /* overwrite with board specific setup code address */
>      FIXUP_ARGPTR,       /* overwrite with pointer to kernel args */
> +    FIXUP_ARGPTR_HIGHER_32BITS,       /* overwrite with pointer to kernel args (higher 32 bits) */
>      FIXUP_ENTRYPOINT,   /* overwrite with kernel entry point */
> +    FIXUP_ENTRYPOINT_HIGHER_32BITS,   /* overwrite with kernel entry point (higher 32 bits) */

I recommend naming these FIXUP_ARGPTR_HI and FIXUP_ENTRYPOINT_HI.
As a second followup patch we can then rename FIXUP_ARGPTR and
FIXUP_ENTRYPOINT to FIXUP_ARGPTR_LO and FIXUP_ENTRYPOINT_LO.

>      FIXUP_GIC_CPU_IF,   /* overwrite with GIC CPU interface address */
>      FIXUP_BOOTREG,      /* overwrite with boot register address */
>      FIXUP_DSB,          /* overwrite with correct DSB insn for cpu */
> @@ -84,9 +86,9 @@ static const ARMInsnFixup bootloader_aarch64[] = {
>      { 0x58000084 }, /* ldr x4, entry ; Load the lower 32-bits of kernel entry */
>      { 0xd61f0080 }, /* br x4      ; Jump to the kernel entry point */
>      { 0, FIXUP_ARGPTR }, /* arg: .word @DTB Lower 32-bits */
> -    { 0 }, /* .word @DTB Higher 32-bits */
> +    { 0, FIXUP_ARGPTR_HIGHER_32BITS}, /* .word @DTB Higher 32-bits */
>      { 0, FIXUP_ENTRYPOINT }, /* entry: .word @Kernel Entry Lower 32-bits */
> -    { 0 }, /* .word @Kernel Entry Higher 32-bits */
> +    { 0, FIXUP_ENTRYPOINT_HIGHER_32BITS }, /* .word @Kernel Entry Higher 32-bits */
>      { 0, FIXUP_TERMINATOR }
>  };
>
> @@ -175,7 +177,9 @@ static void write_bootloader(const char *name, hwaddr addr,
>          case FIXUP_BOARDID:
>          case FIXUP_BOARD_SETUP:
>          case FIXUP_ARGPTR:
> +        case FIXUP_ARGPTR_HIGHER_32BITS:
>          case FIXUP_ENTRYPOINT:
> +        case FIXUP_ENTRYPOINT_HIGHER_32BITS:
>          case FIXUP_GIC_CPU_IF:
>          case FIXUP_BOOTREG:
>          case FIXUP_DSB:
> @@ -939,7 +943,6 @@ static uint64_t load_aarch64_image(const char *filename, hwaddr mem_base,
>              }
>          }
>      }
> -
>      *entry = mem_base + kernel_load_offset;
>      rom_add_blob_fixed_as(filename, buffer, size, *entry, as);
>

Stray whitespace change.

> @@ -1153,8 +1156,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>              info->dtb_start = QEMU_ALIGN_UP(info->initrd_start + initrd_size,
>                                             align);
>              fixupcontext[FIXUP_ARGPTR] = info->dtb_start;
> +            fixupcontext[FIXUP_ARGPTR_HIGHER_32BITS] = info->dtb_start >> 32;
>          } else {
>              fixupcontext[FIXUP_ARGPTR] = info->loader_start + KERNEL_ARGS_ADDR;
> +            fixupcontext[FIXUP_ARGPTR_HIGHER_32BITS] = (info->loader_start + KERNEL_ARGS_ADDR) >> 32;
>              if (info->ram_size >= (1ULL << 32)) {
>                  error_report("RAM size must be less than 4GB to boot"
>                               " Linux kernel using ATAGS (try passing a device tree"
> @@ -1163,6 +1168,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
>              }
>          }
>          fixupcontext[FIXUP_ENTRYPOINT] = entry;
> +        fixupcontext[FIXUP_ENTRYPOINT_HIGHER_32BITS] = entry >> 32;
>
>          write_bootloader("bootloader", info->loader_start,
>                           primary_loader, fixupcontext, as);
> --

Otherwise the patch looks good.

thanks
-- PMM

Re: [Qemu-devel] [PATCH] [PATCH] Allow AArch64 processors to boot from a kernel placed over 4GB.
Posted by no-reply@patchew.org 6 years, 11 months ago
Hi,

This series seems to have some coding style problems. See output below for
more information:

Message-id: 20181126191454.9455-1-ricardo.perez_blanco@nokia.com
Subject: [Qemu-devel] [PATCH] [PATCH] Allow AArch64 processors to boot from a kernel placed over 4GB.
Type: series

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
96e23e4 Allow AArch64 processors to boot from a kernel placed over 4GB.

=== OUTPUT BEGIN ===
Checking PATCH 1/1: Allow AArch64 processors to boot from a kernel placed over 4GB....
ERROR: line over 90 characters
#22: FILE: hw/arm/boot.c:67:
+    FIXUP_ARGPTR_HIGHER_32BITS,       /* overwrite with pointer to kernel args (higher 32 bits) */

ERROR: line over 90 characters
#24: FILE: hw/arm/boot.c:69:
+    FIXUP_ENTRYPOINT_HIGHER_32BITS,   /* overwrite with kernel entry point (higher 32 bits) */

WARNING: line over 80 characters
#36: FILE: hw/arm/boot.c:91:
+    { 0, FIXUP_ENTRYPOINT_HIGHER_32BITS }, /* .word @Kernel Entry Higher 32-bits */

ERROR: line over 90 characters
#65: FILE: hw/arm/boot.c:1162:
+            fixupcontext[FIXUP_ARGPTR_HIGHER_32BITS] = (info->loader_start + KERNEL_ARGS_ADDR) >> 32;

total: 3 errors, 1 warnings, 53 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [Qemu-devel] [PATCH] [PATCH] Allow AArch64 processors to boot from a kernel placed over 4GB.
Posted by Peter Maydell 6 years, 11 months ago
On Mon, 26 Nov 2018 at 19:15, Perez Blanco, Ricardo (Nokia -
BE/Antwerp) <ricardo.perez_blanco@nokia.com> wrote:
>
> Some machine based on AArch64 can have its main memory over 4GBs. With
> the current path, these machines can support "-kernel" in qemu
>
> Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com>

Does this fix an issue with one of the current boards we
have in QEMU? I think they all have RAM below 4GB...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] [PATCH] Allow AArch64 processors to boot from a kernel placed over 4GB.
Posted by Perez Blanco, Ricardo (Nokia - BE/Antwerp) 6 years, 11 months ago
Hi Peter,

AFAIK, there is no board in QEMU that has its memory over 4GiB, however, we are working in some prototypes and proof of concepts of boards with memory over 4GiB.

Kind regards,

Ricardo

> -----Original Message-----
> From: Peter Maydell <peter.maydell@linaro.org>
> Sent: Monday, November 26, 2018 9:35 PM
> To: Perez Blanco, Ricardo (Nokia - BE/Antwerp)
> <ricardo.perez_blanco@nokia.com>
> Cc: ricardoperezblanco@gmail.com; qemu-arm <qemu-arm@nongnu.org>;
> QEMU Developers <qemu-devel@nongnu.org>
> Subject: Re: [PATCH] [PATCH] Allow AArch64 processors to boot from a kernel
> placed over 4GB.
> 
> On Mon, 26 Nov 2018 at 19:15, Perez Blanco, Ricardo (Nokia -
> BE/Antwerp) <ricardo.perez_blanco@nokia.com> wrote:
> >
> > Some machine based on AArch64 can have its main memory over 4GBs. With
> > the current path, these machines can support "-kernel" in qemu
> >
> > Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com>
> 
> Does this fix an issue with one of the current boards we have in QEMU? I think
> they all have RAM below 4GB...
> 
> thanks
> -- PMM
Re: [Qemu-devel] [PATCH] [PATCH] Allow AArch64 processors to boot from a kernel placed over 4GB.
Posted by Peter Maydell 6 years, 11 months ago
On Tue, 27 Nov 2018 at 08:43, Perez Blanco, Ricardo (Nokia -
BE/Antwerp) <ricardo.perez_blanco@nokia.com> wrote:
>
> Hi Peter,
>
> AFAIK, there is no board in QEMU that has its memory over 4GiB, however, we are working in some prototypes and proof of concepts of boards with memory over 4GiB.

OK, thanks. It's a reasonable missing feature to fix, but
since it doesn't affect any current boards we can defer
it to the next (4.0) release. I'll send some code
review comments in a moment.

thanks
-- PMM

[Qemu-devel] [PATCH v2] Allow AArch64 processors to boot from a kernel placed over 4GB.
Posted by Perez Blanco, Ricardo (Nokia - BE/Antwerp) 6 years, 11 months ago
Some machine based on AArch64 can have its main memory over 4GBs. With
the current path, these machines can support "-kernel" in qemu.

Note that, currently, none of the QEMU machines have their main memory over
4GBs.

Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com>
---
 hw/arm/boot.c | 33 ++++++++++++++++++++-------------
 1 file changed, 20 insertions(+), 13 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 586baa9b64..2e443c2bd1 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -63,8 +63,10 @@ typedef enum {
     FIXUP_TERMINATOR,   /* end of insns */
     FIXUP_BOARDID,      /* overwrite with board ID number */
     FIXUP_BOARD_SETUP,  /* overwrite with board specific setup code address */
-    FIXUP_ARGPTR,       /* overwrite with pointer to kernel args */
-    FIXUP_ENTRYPOINT,   /* overwrite with kernel entry point */
+    FIXUP_ARGPTR_LO,       /* overwrite with pointer to kernel args */
+    FIXUP_ARGPTR_HI,       /* overwrite with pointer to kernel args (higher 32 bits) */
+    FIXUP_ENTRYPOINT_LO,   /* overwrite with kernel entry point */
+    FIXUP_ENTRYPOINT_HI,   /* overwrite with kernel entry point (higher 32 bits) */
     FIXUP_GIC_CPU_IF,   /* overwrite with GIC CPU interface address */
     FIXUP_BOOTREG,      /* overwrite with boot register address */
     FIXUP_DSB,          /* overwrite with correct DSB insn for cpu */
@@ -83,10 +85,10 @@ static const ARMInsnFixup bootloader_aarch64[] = {
     { 0xaa1f03e3 }, /* mov x3, xzr */
     { 0x58000084 }, /* ldr x4, entry ; Load the lower 32-bits of kernel entry */
     { 0xd61f0080 }, /* br x4      ; Jump to the kernel entry point */
-    { 0, FIXUP_ARGPTR }, /* arg: .word @DTB Lower 32-bits */
-    { 0 }, /* .word @DTB Higher 32-bits */
-    { 0, FIXUP_ENTRYPOINT }, /* entry: .word @Kernel Entry Lower 32-bits */
-    { 0 }, /* .word @Kernel Entry Higher 32-bits */
+    { 0, FIXUP_ARGPTR_LO }, /* arg: .word @DTB Lower 32-bits */
+    { 0, FIXUP_ARGPTR_HI}, /* .word @DTB Higher 32-bits */
+    { 0, FIXUP_ENTRYPOINT_LO }, /* entry: .word @Kernel Entry Lower 32-bits */
+    { 0, FIXUP_ENTRYPOINT_HI }, /* .word @Kernel Entry Higher 32-bits */
     { 0, FIXUP_TERMINATOR }
 };
 
@@ -106,8 +108,8 @@ static const ARMInsnFixup bootloader[] = {
     { 0xe59f2004 }, /* ldr     r2, [pc, #4] */
     { 0xe59ff004 }, /* ldr     pc, [pc, #4] */
     { 0, FIXUP_BOARDID },
-    { 0, FIXUP_ARGPTR },
-    { 0, FIXUP_ENTRYPOINT },
+    { 0, FIXUP_ARGPTR_LO },
+    { 0, FIXUP_ENTRYPOINT_LO },
     { 0, FIXUP_TERMINATOR }
 };
 
@@ -174,8 +176,10 @@ static void write_bootloader(const char *name, hwaddr addr,
             break;
         case FIXUP_BOARDID:
         case FIXUP_BOARD_SETUP:
-        case FIXUP_ARGPTR:
-        case FIXUP_ENTRYPOINT:
+        case FIXUP_ARGPTR_LO:
+        case FIXUP_ARGPTR_HI:
+        case FIXUP_ENTRYPOINT_LO:
+        case FIXUP_ENTRYPOINT_HI:
         case FIXUP_GIC_CPU_IF:
         case FIXUP_BOOTREG:
         case FIXUP_DSB:
@@ -1152,9 +1156,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
             /* Place the DTB after the initrd in memory with alignment. */
             info->dtb_start = QEMU_ALIGN_UP(info->initrd_start + initrd_size,
                                            align);
-            fixupcontext[FIXUP_ARGPTR] = info->dtb_start;
+            fixupcontext[FIXUP_ARGPTR_LO] = info->dtb_start;
+            fixupcontext[FIXUP_ARGPTR_HI] = info->dtb_start >> 32;
         } else {
-            fixupcontext[FIXUP_ARGPTR] = info->loader_start + KERNEL_ARGS_ADDR;
+            fixupcontext[FIXUP_ARGPTR_LO] = info->loader_start + KERNEL_ARGS_ADDR;
+            fixupcontext[FIXUP_ARGPTR_HI] = (info->loader_start + KERNEL_ARGS_ADDR) >> 32;
             if (info->ram_size >= (1ULL << 32)) {
                 error_report("RAM size must be less than 4GB to boot"
                              " Linux kernel using ATAGS (try passing a device tree"
@@ -1162,7 +1168,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
                 exit(1);
             }
         }
-        fixupcontext[FIXUP_ENTRYPOINT] = entry;
+        fixupcontext[FIXUP_ENTRYPOINT_LO] = entry;
+        fixupcontext[FIXUP_ENTRYPOINT_HI] = entry >> 32;
 
         write_bootloader("bootloader", info->loader_start,
                          primary_loader, fixupcontext, as);
-- 
2.14.1


Re: [Qemu-devel] [PATCH v2] Allow AArch64 processors to boot from a kernel placed over 4GB.
Posted by Peter Maydell 6 years, 11 months ago
On Tue, 27 Nov 2018 at 20:25, Perez Blanco, Ricardo (Nokia -
BE/Antwerp) <ricardo.perez_blanco@nokia.com> wrote:
>
> Some machine based on AArch64 can have its main memory over 4GBs. With
> the current path, these machines can support "-kernel" in qemu.
>
> Note that, currently, none of the QEMU machines have their main memory over
> 4GBs.
>
> Signed-off-by: Ricardo Perez Blanco <ricardo.perez_blanco@nokia.com>
> ---

Thanks for the respin; I've added it to target-arm.next for the
4.0 release. (I rephrased the commit message a bit and fixed up
the overlong lines.)

-- PMM