Re: [Qemu-devel] [PATCH v2 6/7] hw/arm/boot: Add boot support for AArch64 processor

[Peter Crosthwaite]

On Tue, Dec 17, 2013 at 10:15 PM, Peter Maydell
<address@hidden> wrote:
> From: "Mian M. Hamayun" <address@hidden>
>
> This commit adds support for booting a single AArch64 CPU by setting
> appropriate registers. The bootloader includes placehoders for Board-ID

"placeholders"

> that are used to implement uniform indexing across different bootloaders.
>
> Signed-off-by: Mian M. Hamayun <address@hidden>
> Signed-off-by: Peter Maydell <address@hidden>
> Message-id: address@hidden
> [PMM:
>  * updated to use ARMInsnFixup style bootloader fragments
>  * dropped virt.c additions
>  * use runtime checks for "is this an AArch64 core" rather than ifdefs
>  * drop some unnecessary setting of registers in reset hook
> ]
> Signed-off-by: Peter Maydell <address@hidden>
> Reviewed-by: Christoffer Dall <address@hidden>
> ---
>  hw/arm/boot.c | 43 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 0c05a64..90e9534 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -17,8 +17,13 @@
>  #include "sysemu/device_tree.h"
>  #include "qemu/config-file.h"
>
> +/* Kernel boot protocol is specified in the kernel docs
> + * Documentation/arm/Booting and Documentation/arm64/booting.txt
> + * They have different preferred image load offsets from system RAM base.
> + */
>  #define KERNEL_ARGS_ADDR 0x100
>  #define KERNEL_LOAD_ADDR 0x00010000

So out of context, but I have been booting an ARMv7 multi defconfig on
a few qemu platforms recently (spec. allwinner, highbank and zynq) and
I found I had to patch this to 0x8000 due to some image alignment
expectations of head.S. Could we patch this to 0x8000 -  is there this
same sense of preferred image offset in KERNEL32_LOAD_ADDR?

> +#define KERNEL64_LOAD_ADDR 0x00080000
>
>  typedef enum {
>      FIXUP_NONE = 0,   /* do nothing */
> @@ -37,6 +42,20 @@ typedef struct ARMInsnFixup {
>      FixupType fixup;
>  } ARMInsnFixup;
>
> +static const ARMInsnFixup bootloader_aarch64[] = {
> +    { 0x580000c0 }, /* ldr x0, arg ; Load the lower 32-bits of DTB */
> +    { 0xaa1f03e1 }, /* mov x1, xzr */
> +    { 0xaa1f03e2 }, /* mov x2, xzr */
> +    { 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_TERMINATOR }
> +};
> +
>  /* The worlds second smallest bootloader.  Set r0-r2, then jump to kernel.  
> */
>  static const ARMInsnFixup bootloader[] = {
>      { 0xe3a00000 }, /* mov     r0, #0 */
> @@ -396,7 +415,12 @@ static void do_cpu_reset(void *opaque)
>              env->thumb = info->entry & 1;
>          } else {
>              if (CPU(cpu) == first_cpu) {
> -                env->regs[15] = info->loader_start;
> +                if (env->aarch64) {

Curious, why does this 'if' directly deref env, while the one below
(for primary_loader selection) uses ARM_FEATURE?

Regards,
Peter

> +                    env->pc = info->loader_start;
> +                } else {
> +                    env->regs[15] = info->loader_start;
> +                }
> +
>                  if (!info->dtb_filename) {
>                      if (old_param) {
>                          set_kernel_args_old(info);
> @@ -418,8 +442,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>      int initrd_size;
>      int is_linux = 0;
>      uint64_t elf_entry;
> -    hwaddr entry;
> +    hwaddr entry, kernel_load_offset;
>      int big_endian;
> +    static const ARMInsnFixup *primary_loader;
>
>      /* Load the kernel.  */
>      if (!info->kernel_filename) {
> @@ -429,6 +454,14 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>          return;
>      }
>
> +    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
> +        primary_loader = bootloader_aarch64;
> +        kernel_load_offset = KERNEL64_LOAD_ADDR;
> +    } else {
> +        primary_loader = bootloader;
> +        kernel_load_offset = KERNEL_LOAD_ADDR;
> +    }
> +
>      info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
>
>      if (!info->secondary_cpu_reset_hook) {
> @@ -469,9 +502,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>                                    &is_linux);
>      }
>      if (kernel_size < 0) {
> -        entry = info->loader_start + KERNEL_LOAD_ADDR;
> +        entry = info->loader_start + kernel_load_offset;
>          kernel_size = load_image_targphys(info->kernel_filename, entry,
> -                                          info->ram_size - KERNEL_LOAD_ADDR);
> +                                          info->ram_size - 
> kernel_load_offset);
>          is_linux = 1;
>      }
>      if (kernel_size < 0) {
> @@ -532,7 +565,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>          fixupcontext[FIXUP_ENTRYPOINT] = entry;
>
>          write_bootloader("bootloader", info->loader_start,
> -                         bootloader, fixupcontext);
> +                         primary_loader, fixupcontext);
>
>          if (info->nb_cpus > 1) {
>              info->write_secondary_boot(cpu, info);
> --
> 1.8.5
>
>