On Thu, Nov 28, 2013 at 01:33:20PM +0000, Peter Maydell 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.
>
> Signed-off-by: Peter Maydell <peter.maydell@xxxxxxxxxx>
Minor thing below, otherwise it looks quite nice:
Reviewed-by: Christoffer Dall <christoffer.dall@xxxxxxxxxx>
> ---
> 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 },
couldn't we add the gic_cpu_if and startaddr "labels" as comments to the
two lines above? (alternatively also rename the reference to startaddr
to bootret in the second instruction comment).
> + { 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
>
>
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/cucslists/listinfo/kvmarm
