Re: [Qemu-devel] [RFC PATCH v0 10/15] ppc: Factor out CPU initialization code to a new routine

[Igor Mammedov]

On Thu,  4 Sep 2014 11:36:20 +0530
Bharata B Rao <address@hidden> wrote:

> Separate out CPU initialization code into a new routine ppc_new_cpu()
> so that it can be used from CPU hotplug path too.
> 
> Signed-off-by: Bharata B Rao <address@hidden>
> ---
>  hw/ppc/spapr.c | 73 
> +++++++++++++++++++++++++++++++++-------------------------
>  1 file changed, 42 insertions(+), 31 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b2ca527..41207ae 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1603,6 +1603,45 @@ static SaveVMHandlers savevm_htab_handlers = {
>      .load_state = htab_load,
>  };
>  
looking at following code from POV of using CPU with device_add cmd.

> +static const char *current_cpu_model;
do PPC CPUs use full 'model-name,feature1,-feature2' format or only
model-name from cpu_model string?
 
> +static PowerPCCPU *ppc_new_cpu(const char *cpu_model)
> +{
> +    PowerPCCPU *cpu;
> +    CPUPPCState *env;
> +
> +    cpu = cpu_ppc_init(cpu_model);
> +    if (cpu == NULL) {
> +        fprintf(stderr, "Unable to find PowerPC CPU definition\n");
> +        exit(1);
> +    }

-- cut --
> +    env = &cpu->env;
> +
> +    /* Set time-base frequency to 512 MHz */
> +    cpu_ppc_tb_init(env, TIMEBASE_FREQ);
> +
> +    /* PAPR always has exception vectors in RAM not ROM. To ensure this,
> +     * MSR[IP] should never be set.
> +     */
> +    env->msr_mask &= ~(1 << 6);
> +
> +    /* Tell KVM that we're in PAPR mode */
> +    if (kvm_enabled()) {
> +        kvmppc_set_papr(cpu);
> +    }
> +
> +    if (cpu->max_compat) {
> +        if (ppc_set_compat(cpu, cpu->max_compat) < 0) {
> +            exit(1);
> +        }
> +    }
-- cut --
selected block looks like setting CPU internals, which could be done
inside of CPU's realizefn.

> +
> +    xics_cpu_setup(spapr->icp, cpu);


> +    qemu_register_reset(spapr_cpu_reset, cpu);
also could be put inside of CPU's realizefn, like it's done in
for x86 CPU.

> +
> +    return cpu;
> +}
> +
>  /* pSeries LPAR / sPAPR hardware init */
>  static void ppc_spapr_init(MachineState *machine)
>  {
> @@ -1612,8 +1651,6 @@ static void ppc_spapr_init(MachineState *machine)
>      const char *kernel_cmdline = machine->kernel_cmdline;
>      const char *initrd_filename = machine->initrd_filename;
>      const char *boot_device = machine->boot_order;
> -    PowerPCCPU *cpu;
> -    CPUPPCState *env;
>      PCIHostState *phb;
>      int i;
>      MemoryRegion *sysmem = get_system_memory();
> @@ -1693,36 +1730,10 @@ static void ppc_spapr_init(MachineState *machine)
>      if (cpu_model == NULL) {
>          cpu_model = kvm_enabled() ? "host" : "POWER7";
>      }
> -    for (i = 0; i < smp_cpus; i++) {
> -        cpu = cpu_ppc_init(cpu_model);
> -        if (cpu == NULL) {
> -            fprintf(stderr, "Unable to find PowerPC CPU definition\n");
> -            exit(1);
> -        }
> -        env = &cpu->env;
> -
> -        /* Set time-base frequency to 512 MHz */
> -        cpu_ppc_tb_init(env, TIMEBASE_FREQ);
> -
> -        /* PAPR always has exception vectors in RAM not ROM. To ensure this,
> -         * MSR[IP] should never be set.
> -         */
> -        env->msr_mask &= ~(1 << 6);
> +    current_cpu_model = cpu_model;
>  
> -        /* Tell KVM that we're in PAPR mode */
> -        if (kvm_enabled()) {
> -            kvmppc_set_papr(cpu);
> -        }
> -
> -        if (cpu->max_compat) {
> -            if (ppc_set_compat(cpu, cpu->max_compat) < 0) {
> -                exit(1);
> -            }
> -        }
> -
> -        xics_cpu_setup(spapr->icp, cpu);
> -
> -        qemu_register_reset(spapr_cpu_reset, cpu);
> +    for (i = 0; i < smp_cpus; i++) {
> +        ppc_new_cpu(current_cpu_model);
>      }
>  
>      /* allocate RAM */