On 19 January 2012 15:43, Mark Langsdorf <mark.langsd...@calxeda.com> wrote: > Re: [PATCH v11 4/6] arm: add secondary cpu book callbacks to arm_boot.c
Subject should say 'boot', not 'book'... > Create two functions, write_secondary_boot() and secondary_cpu_reset_hook(), > to allow platforms more control of how secondary CPUs are brought up. The > new functions default to NULL and aren't called unless they are populated > so there are no changes to existing platform models. > > Signed-off-by: Mark Langsdorf <mark.langsd...@calxeda.com> > --- > Changes from v2-v10 > Skipped > Changes from v1 > Added default versions of the functions > Created checks so that NULL function entries became default versions > Simplified calls to the secondary boot functions > Added comments on the use of these functions > > hw/arm-misc.h | 15 +++++++++++++++ > hw/arm_boot.c | 38 +++++++++++++++++++++++++++++--------- > 2 files changed, 44 insertions(+), 9 deletions(-) > > diff --git a/hw/arm-misc.h b/hw/arm-misc.h > index 6e8ae6b..9138eab 100644 > --- a/hw/arm-misc.h > +++ b/hw/arm-misc.h > @@ -30,12 +30,27 @@ struct arm_boot_info { > const char *kernel_cmdline; > const char *initrd_filename; > target_phys_addr_t loader_start; > + /* multicore boards that use the default secondary core boot functions > + * need to put the address of the secondary boot code, the boot reg, > + * and the GIC address in the next 3 values, respectively. boards that > + * have their own boot functions can use these values as they want. > + */ > target_phys_addr_t smp_loader_start; > target_phys_addr_t smp_bootreg_addr; > target_phys_addr_t smp_priv_base; > int nb_cpus; > int board_id; > int (*atag_board)(const struct arm_boot_info *info, void *p); > + /* multicore boards that use the default secondary core boot functions > + * can ignore these two function calls. If the default functions won't > + * work, then write_secondary_boot() should mimic the board's boot > + * loader code, and secondary_cpu_reset_hook() should operate handle any > + * polling response and reset functions. > + */ "then write_secondary_boot() should write a suitable blob of code mimicing the secondary CPU startup process used by the board's boot loader/boot ROM code, and secondary_cpu_reset_hook() should perform any necessary CPU reset handling and set the PC for the secondary CPUs to point at this boot blob." > +static void default_write_secondary(CPUState *env, > + const struct arm_boot_info *info) > +{ > + int n; > + smpboot[ARRAY_SIZE(smpboot) - 1] = info->smp_bootreg_addr; > + smpboot[ARRAY_SIZE(smpboot) - 2] = info->smp_priv_base; > + for (n = 0; n < ARRAY_SIZE(smpboot); n++) { > + smpboot[n] = tswap32(smpboot[n]); > + } > + rom_add_blob_fixed("smpboot", smpboot, sizeof(smpboot), > + info->smp_loader_start); > +} > + > +static void default_reset_secondary(CPUState *env, > + const struct arm_boot_info *info) > +{ > + stl_phys_notdirty(info->smp_bootreg_addr, 0); > + env->regs[15] = info->smp_loader_start; > +} > + Can you put these two functions immediately below the smpboot[] array, and replace the comment just above smpboot[] with this one: /* Handling for secondary CPU boot in a multicore system. * Unlike the uniprocessor/primary CPU boot, this is platform * dependent. The default code here is based on the secondary * CPU boot protocol used on realview/vexpress boards, with * some parameterisation to increase its flexibility. * QEMU platform models for which this code is not appropriate * should override write_secondary_boot and secondary_cpu_reset_hook * instead. * * This code enables the interrupt controllers for the secondary * CPUs and then puts all the secondary CPUs into a loop waiting * for an interprocessor interrupt and polling a configurable * location for the kernel secondary CPU entry point. */ Otherwise looks good. -- PMM