| Message ID | 20181126191454.9455-1-ricardo.perez_blanco@nokia.com |
|---|---|
| State | New |
| Headers | show |
| Series | Allow AArch64 processors to boot from a kernel placed over 4GB. | expand |
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
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
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
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
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 --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);
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(-)