From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36673) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XXXSR-00029o-Gq for qemu-devel@nongnu.org; Fri, 26 Sep 2014 11:29:25 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XXXSL-0001d1-BE for qemu-devel@nongnu.org; Fri, 26 Sep 2014 11:29:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:23354) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XXXSL-0001cl-3W for qemu-devel@nongnu.org; Fri, 26 Sep 2014 11:29:13 -0400 Date: Fri, 26 Sep 2014 17:29:02 +0200 From: Igor Mammedov Message-ID: <20140926172902.3b0bdf4a@nial.usersys.redhat.com> In-Reply-To: <1409810785-12391-11-git-send-email-bharata@linux.vnet.ibm.com> References: <1409810785-12391-1-git-send-email-bharata@linux.vnet.ibm.com> <1409810785-12391-11-git-send-email-bharata@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH v0 10/15] ppc: Factor out CPU initialization code to a new routine List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Bharata B Rao Cc: qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com On Thu, 4 Sep 2014 11:36:20 +0530 Bharata B Rao 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 > --- > 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 */