From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34125) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YPnaW-0003eV-1K for qemu-devel@nongnu.org; Mon, 23 Feb 2015 02:37:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YPnaQ-0006DM-Rn for qemu-devel@nongnu.org; Mon, 23 Feb 2015 02:37:55 -0500 Received: from e23smtp09.au.ibm.com ([202.81.31.142]:33845) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YPnaP-0006Cv-Mu for qemu-devel@nongnu.org; Mon, 23 Feb 2015 02:37:50 -0500 Received: from /spool/local by e23smtp09.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 23 Feb 2015 17:37:45 +1000 Received: from d23relay06.au.ibm.com (d23relay06.au.ibm.com [9.185.63.219]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id 765AA2BB004D for ; Mon, 23 Feb 2015 18:37:41 +1100 (EST) Received: from d23av02.au.ibm.com (d23av02.au.ibm.com [9.190.235.138]) by d23relay06.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t1N7bXPH48300190 for ; Mon, 23 Feb 2015 18:37:41 +1100 Received: from d23av02.au.ibm.com (localhost [127.0.0.1]) by d23av02.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t1N7b7K1003816 for ; Mon, 23 Feb 2015 18:37:07 +1100 Date: Mon, 23 Feb 2015 13:06:48 +0530 From: Bharata B Rao Message-ID: <20150223073648.GA29891@in.ibm.com> References: <1420697420-16053-1-git-send-email-bharata@linux.vnet.ibm.com> <1420697420-16053-5-git-send-email-bharata@linux.vnet.ibm.com> <20150129010742.GO14681@voom> <20150130074939.GB24041@in.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150130074939.GB24041@in.ibm.com> Subject: Re: [Qemu-devel] [RFC PATCH v1 04/13] spapr: Factor out CPU initialization code into realizefn Reply-To: bharata@linux.vnet.ibm.com List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: imammedo@redhat.com, agraf@suse.de, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com On Fri, Jan 30, 2015 at 01:19:39PM +0530, Bharata B Rao wrote: > On Thu, Jan 29, 2015 at 12:07:42PM +1100, David Gibson wrote: > > On Thu, Jan 08, 2015 at 11:40:11AM +0530, Bharata B Rao wrote: > > > Move some CPU initialization code from machine init function to > > > CPU realizefn so that it can be used from CPU hotplug path too. > > > > > > With the inclusion of ppc.h in translate_init.c, explicit *irq_init() > > > function definitions aren't required, remove them. > > > > > > Signed-off-by: Bharata B Rao > > > --- > > > hw/ppc/spapr.c | 29 +---------------------------- > > > include/hw/ppc/spapr.h | 3 +++ > > > target-ppc/translate_init.c | 43 ++++++++++++++++++++++++++----------------- > > > 3 files changed, 30 insertions(+), 45 deletions(-) > > > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index 779d364..f49b0fa 100644 > > > --- a/hw/ppc/spapr.c > > > +++ b/hw/ppc/spapr.c > > > @@ -81,8 +81,6 @@ > > > > > > #define MIN_RMA_SLOF 128UL > > > > > > -#define TIMEBASE_FREQ 512000000ULL > > > - > > > #define MAX_CPUS 255 > > > > > > #define PHANDLE_XICP 0x00001111 > > > @@ -971,7 +969,7 @@ static void ppc_spapr_reset(void) > > > > > > } > > > > > > -static void spapr_cpu_reset(void *opaque) > > > +void spapr_cpu_reset(void *opaque) > > > { > > > PowerPCCPU *cpu = opaque; > > > CPUState *cs = CPU(cpu); > > > @@ -1387,7 +1385,6 @@ static void ppc_spapr_init(MachineState *machine) > > > 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(); > > > @@ -1472,30 +1469,6 @@ static void ppc_spapr_init(MachineState *machine) > > > 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); > > > - > > > - /* 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); > > > } > > > > > > /* allocate RAM */ > > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > > index b1a0838..831db6b 100644 > > > --- a/include/hw/ppc/spapr.h > > > +++ b/include/hw/ppc/spapr.h > > > @@ -478,6 +478,8 @@ struct sPAPRTCETable { > > > QLIST_ENTRY(sPAPRTCETable) list; > > > }; > > > > > > +#define TIMEBASE_FREQ 512000000ULL > > > + > > > void spapr_events_init(sPAPREnvironment *spapr); > > > void spapr_events_fdt_skel(void *fdt, uint32_t epow_irq); > > > int spapr_h_cas_compose_response(target_ulong addr, target_ulong size); > > > @@ -494,5 +496,6 @@ int spapr_tcet_dma_dt(void *fdt, int node_off, const char *propname, > > > sPAPRTCETable *tcet); > > > void spapr_hotplug_req_add_event(sPAPRDRConnector *drc); > > > void spapr_hotplug_req_remove_event(sPAPRDRConnector *drc); > > > +void spapr_cpu_reset(void *opaque); > > > > > > #endif /* !defined (__HW_SPAPR_H__) */ > > > diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c > > > index 72cc9d0..9c642a5 100644 > > > --- a/target-ppc/translate_init.c > > > +++ b/target-ppc/translate_init.c > > > @@ -30,29 +30,14 @@ > > > #include "qemu/error-report.h" > > > #include "qapi/visitor.h" > > > #include "hw/qdev-properties.h" > > > +#include "hw/ppc/spapr.h" > > > +#include "hw/ppc/ppc.h" > > > > > > //#define PPC_DUMP_CPU > > > //#define PPC_DEBUG_SPR > > > //#define PPC_DUMP_SPR_ACCESSES > > > /* #define USE_APPLE_GDB */ > > > > > > -/* For user-mode emulation, we don't emulate any IRQ controller */ > > > -#if defined(CONFIG_USER_ONLY) > > > -#define PPC_IRQ_INIT_FN(name) \ > > > -static inline void glue(glue(ppc, name),_irq_init) (CPUPPCState *env) \ > > > -{ \ > > > -} > > > -#else > > > -#define PPC_IRQ_INIT_FN(name) \ > > > -void glue(glue(ppc, name),_irq_init) (CPUPPCState *env); > > > -#endif > > > - > > > -PPC_IRQ_INIT_FN(40x); > > > -PPC_IRQ_INIT_FN(6xx); > > > -PPC_IRQ_INIT_FN(970); > > > -PPC_IRQ_INIT_FN(POWER7); > > > -PPC_IRQ_INIT_FN(e500); > > > - > > > /* Generic callbacks: > > > * do nothing but store/retrieve spr value > > > */ > > > @@ -8905,6 +8890,7 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp) > > > CPUState *cs = CPU(dev); > > > PowerPCCPU *cpu = POWERPC_CPU(dev); > > > PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu); > > > + CPUPPCState *env = &cpu->env; > > > Error *local_err = NULL; > > > #if !defined(CONFIG_USER_ONLY) > > > int max_smt = kvm_enabled() ? kvmppc_smt_threads() : 1; > > > @@ -8965,6 +8951,29 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp) > > > > > > qemu_init_vcpu(cs); > > > > > > + /* 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); > > > + } > > > + } > > > + > > > + xics_cpu_setup(spapr->icp, cpu); > > > + > > > + qemu_register_reset(spapr_cpu_reset, cpu); > > > + > > > pcc->parent_realize(dev, errp); > > > > This doesn't look right. Several of these are clearly PAPR specific > > operations, but you're now doing them from code that isn't PAPR specific. > > Ok, will re-work on this patch. There is only PowerPCCPU and no CPU specific class for sPAPR. So such things as above which should ideally be done in realizefn path but are sPAPR specific should sit where ? Under TARGET_PPC64 ? Regards, Bharata.