Re: [Qemu-devel] [PATCH 5/7] hw/arm/boot: Allow easier swapping in of different loader code

[Peter Crosthwaite]

On Thu, Nov 28, 2013 at 11:33 PM, Peter Maydell
<address@hidden> wrote:
> For AArch64 we will obviously require a different set of
> primary and secondary boot loader code fragments. However currently
> we hardcode the offsets into the loader code where we must write
> the entrypoint and other data into arm_load_kernel(). This makes it
> hard to substitute a different loader fragment, so switch to a more
> flexible scheme where instead of a raw array of instructions we use
> an array of (instruction, fixup-type) pairs that indicate which
> words need special action or data written into them.
>

Why do we need blobs at all? Cant we just fix arm/boot to directly
setup the CPU state to the desired? Rather than complex blobs that
execute ARM instructions just manipulate the regs directly. The
booloader already directly accesses r15 for setting the boot blob
entry point so I don't see whats wrong with setting board-id and dtb
directly either. See the microblaze bootloader for an example.

Regards,
Peter

> Signed-off-by: Peter Maydell <address@hidden>
> ---
>  hw/arm/boot.c |  152 
> ++++++++++++++++++++++++++++++++++++++++-----------------
>  1 file changed, 107 insertions(+), 45 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 55d552f..77d29a8 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -20,15 +20,33 @@
>  #define KERNEL_ARGS_ADDR 0x100
>  #define KERNEL_LOAD_ADDR 0x00010000
>
> +typedef enum {
> +    FIXUP_NONE = 0,   /* do nothing */
> +    FIXUP_TERMINATOR, /* end of insns */
> +    FIXUP_BOARDID,    /* overwrite with board ID number */
> +    FIXUP_ARGPTR,     /* overwrite with pointer to kernel args */
> +    FIXUP_ENTRYPOINT, /* overwrite with kernel entry point */
> +    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 */
> +    FIXUP_MAX,
> +} FixupType;
> +
> +typedef struct ARMInsnFixup {
> +    uint32_t insn;
> +    FixupType fixup;
> +} ARMInsnFixup;
> +
>  /* The worlds second smallest bootloader.  Set r0-r2, then jump to kernel.  
> */
> -static uint32_t bootloader[] = {
> -  0xe3a00000, /* mov     r0, #0 */
> -  0xe59f1004, /* ldr     r1, [pc, #4] */
> -  0xe59f2004, /* ldr     r2, [pc, #4] */
> -  0xe59ff004, /* ldr     pc, [pc, #4] */
> -  0, /* Board ID */
> -  0, /* Address of kernel args.  Set by integratorcp_init.  */
> -  0  /* Kernel entry point.  Set by integratorcp_init.  */
> +static const ARMInsnFixup bootloader[] = {
> +    { 0xe3a00000 }, /* mov     r0, #0 */
> +    { 0xe59f1004 }, /* ldr     r1, [pc, #4] */
> +    { 0xe59f2004 }, /* ldr     r2, [pc, #4] */
> +    { 0xe59ff004 }, /* ldr     pc, [pc, #4] */
> +    { 0, FIXUP_BOARDID },
> +    { 0, FIXUP_ARGPTR },
> +    { 0, FIXUP_ENTRYPOINT },
> +    { 0, FIXUP_TERMINATOR }
>  };
>
>  /* Handling for secondary CPU boot in a multicore system.
> @@ -48,39 +66,83 @@ static uint32_t bootloader[] = {
>  #define DSB_INSN 0xf57ff04f
>  #define CP15_DSB_INSN 0xee070f9a /* mcr cp15, 0, r0, c7, c10, 4 */
>
> -static uint32_t smpboot[] = {
> -  0xe59f2028, /* ldr r2, gic_cpu_if */
> -  0xe59f0028, /* ldr r0, startaddr */
> -  0xe3a01001, /* mov r1, #1 */
> -  0xe5821000, /* str r1, [r2] - set GICC_CTLR.Enable */
> -  0xe3a010ff, /* mov r1, #0xff */
> -  0xe5821004, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
> -  DSB_INSN,   /* dsb */
> -  0xe320f003, /* wfi */
> -  0xe5901000, /* ldr     r1, [r0] */
> -  0xe1110001, /* tst     r1, r1 */
> -  0x0afffffb, /* beq     <wfi> */
> -  0xe12fff11, /* bx      r1 */
> -  0,          /* gic_cpu_if: base address of GIC CPU interface */
> -  0           /* bootreg: Boot register address is held here */
> +static const ARMInsnFixup smpboot[] = {
> +    { 0xe59f2028 }, /* ldr r2, gic_cpu_if */
> +    { 0xe59f0028 }, /* ldr r0, startaddr */
> +    { 0xe3a01001 }, /* mov r1, #1 */
> +    { 0xe5821000 }, /* str r1, [r2] - set GICC_CTLR.Enable */
> +    { 0xe3a010ff }, /* mov r1, #0xff */
> +    { 0xe5821004 }, /* str r1, [r2, 4] - set GIC_PMR.Priority to 0xff */
> +    { 0, FIXUP_DSB },   /* dsb */
> +    { 0xe320f003 }, /* wfi */
> +    { 0xe5901000 }, /* ldr     r1, [r0] */
> +    { 0xe1110001 }, /* tst     r1, r1 */
> +    { 0x0afffffb }, /* beq     <wfi> */
> +    { 0xe12fff11 }, /* bx      r1 */
> +    { 0, FIXUP_GIC_CPU_IF },
> +    { 0, FIXUP_BOOTREG },
> +    { 0, FIXUP_TERMINATOR }
>  };
>
> +static void write_bootloader(const char *name, hwaddr addr,
> +                             const ARMInsnFixup *insns, uint32_t 
> *fixupcontext)
> +{
> +    /* Fix up the specified bootloader fragment and write it into
> +     * guest memory using rom_add_blob_fixed(). fixupcontext is
> +     * an array giving the values to write in for the fixup types
> +     * which write a value into the code array.
> +     */
> +    int i, len;
> +    uint32_t *code;
> +
> +    len = 0;
> +    while (insns[len].fixup != FIXUP_TERMINATOR) {
> +        len++;
> +    }
> +
> +    code = g_new0(uint32_t, len);
> +
> +    for (i = 0; i < len; i++) {
> +        uint32_t insn = insns[i].insn;
> +        FixupType fixup = insns[i].fixup;
> +
> +        switch (fixup) {
> +        case FIXUP_NONE:
> +            break;
> +        case FIXUP_BOARDID:
> +        case FIXUP_ARGPTR:
> +        case FIXUP_ENTRYPOINT:
> +        case FIXUP_GIC_CPU_IF:
> +        case FIXUP_BOOTREG:
> +        case FIXUP_DSB:
> +            insn = fixupcontext[fixup];
> +            break;
> +        default:
> +            abort();
> +        }
> +        code[i] = tswap32(insn);
> +    }
> +
> +    rom_add_blob_fixed(name, code, len * sizeof(uint32_t), addr);
> +
> +    g_free(code);
> +}
> +
>  static void default_write_secondary(ARMCPU *cpu,
>                                      const struct arm_boot_info *info)
>  {
> -    int n;
> -    smpboot[ARRAY_SIZE(smpboot) - 1] = info->smp_bootreg_addr;
> -    smpboot[ARRAY_SIZE(smpboot) - 2] = info->gic_cpu_if_addr;
> -    for (n = 0; n < ARRAY_SIZE(smpboot); n++) {
> -        /* Replace DSB with the pre-v7 DSB if necessary. */
> -        if (!arm_feature(&cpu->env, ARM_FEATURE_V7) &&
> -            smpboot[n] == DSB_INSN) {
> -            smpboot[n] = CP15_DSB_INSN;
> -        }
> -        smpboot[n] = tswap32(smpboot[n]);
> +    uint32_t fixupcontext[FIXUP_MAX];
> +
> +    fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
> +    fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
> +    if (arm_feature(&cpu->env, ARM_FEATURE_V7)) {
> +        fixupcontext[FIXUP_DSB] = DSB_INSN;
> +    } else {
> +        fixupcontext[FIXUP_DSB] = CP15_DSB_INSN;
>      }
> -    rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot),
> -                       info->smp_loader_start);
> +
> +    write_bootloader("smpboot", info->smp_loader_start,
> +                     smpboot, fixupcontext);
>  }
>
>  static void default_reset_secondary(ARMCPU *cpu,
> @@ -354,7 +416,6 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>      CPUState *cs = CPU(cpu);
>      int kernel_size;
>      int initrd_size;
> -    int n;
>      int is_linux = 0;
>      uint64_t elf_entry;
>      hwaddr entry;
> @@ -420,6 +481,8 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>      }
>      info->entry = entry;
>      if (is_linux) {
> +        uint32_t fixupcontext[FIXUP_MAX];
> +
>          if (info->initrd_filename) {
>              initrd_size = load_ramdisk(info->initrd_filename,
>                                         info->initrd_start,
> @@ -441,7 +504,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>          }
>          info->initrd_size = initrd_size;
>
> -        bootloader[4] = info->board_id;
> +        fixupcontext[FIXUP_BOARDID] = info->board_id;
>
>          /* for device tree boot, we pass the DTB directly in r2. Otherwise
>           * we point to the kernel args.
> @@ -456,9 +519,9 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>              if (load_dtb(dtb_start, info)) {
>                  exit(1);
>              }
> -            bootloader[5] = dtb_start;
> +            fixupcontext[FIXUP_ARGPTR] = dtb_start;
>          } else {
> -            bootloader[5] = info->loader_start + KERNEL_ARGS_ADDR;
> +            fixupcontext[FIXUP_ARGPTR] = info->loader_start + 
> KERNEL_ARGS_ADDR;
>              if (info->ram_size >= (1ULL << 32)) {
>                  fprintf(stderr, "qemu: RAM size must be less than 4GB to 
> boot"
>                          " Linux kernel using ATAGS (try passing a device 
> tree"
> @@ -466,12 +529,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>                  exit(1);
>              }
>          }
> -        bootloader[6] = entry;
> -        for (n = 0; n < sizeof(bootloader) / 4; n++) {
> -            bootloader[n] = tswap32(bootloader[n]);
> -        }
> -        rom_add_blob_fixed("bootloader", bootloader, sizeof(bootloader),
> -                           info->loader_start);
> +        fixupcontext[FIXUP_ENTRYPOINT] = entry;
> +
> +        write_bootloader("bootloader", info->loader_start,
> +                         bootloader, fixupcontext);
> +
>          if (info->nb_cpus > 1) {
>              info->write_secondary_boot(cpu, info);
>          }
> --
> 1.7.9.5
>
>