diff mbox series

Allow AArch64 processors to boot from a kernel placed over 4GB.

Message ID 20181126191454.9455-1-ricardo.perez_blanco@nokia.com (mailing list archive)
State New, archived
Headers show
Series Allow AArch64 processors to boot from a kernel placed over 4GB. | expand

Commit Message

Perez Blanco, Ricardo (Nokia - BE/Antwerp) Nov. 26, 2018, 7:15 p.m. UTC
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(-)

Comments

Peter Maydell Nov. 26, 2018, 8:35 p.m. UTC | #1
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
Perez Blanco, Ricardo (Nokia - BE/Antwerp) Nov. 27, 2018, 8:43 a.m. UTC | #2
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
Peter Maydell Nov. 27, 2018, 10:40 a.m. UTC | #3
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
Peter Maydell Nov. 27, 2018, 10:43 a.m. UTC | #4
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
no-reply@patchew.org Nov. 28, 2018, 10:51 a.m. UTC | #5
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
diff mbox series

Patch

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);