From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?ISO-8859-1?Q?Andreas_F=E4rber?= Subject: Re: [PATCH ppc-next v2 42/52] target-ppc: Convert CPU definitions Date: Fri, 22 Feb 2013 17:31:06 +0100 Message-ID: <51279D4A.5080106@suse.de> References: <1361179011-7226-1-git-send-email-afaerber@suse.de> <1361179011-7226-43-git-send-email-afaerber@suse.de> <9BB58CE9-0948-4621-83A0-F692EEDA8F84@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Gleb Natapov , Marcelo Tosatti , kvm , Eduardo Habkost , Igor Mammedov To: Alexander Graf Return-path: Received: from cantor2.suse.de ([195.135.220.15]:36476 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756818Ab3BVQbJ (ORCPT ); Fri, 22 Feb 2013 11:31:09 -0500 In-Reply-To: <9BB58CE9-0948-4621-83A0-F692EEDA8F84@suse.de> Sender: kvm-owner@vger.kernel.org List-ID: Am 22.02.2013 15:23, schrieb Alexander Graf: >=20 > On 18.02.2013, at 10:16, Andreas F=E4rber wrote: >=20 >> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c >> index 2c64c63..e601059 100644 >> --- a/target-ppc/kvm.c >> +++ b/target-ppc/kvm.c >> @@ -1263,7 +1263,7 @@ static void kvmppc_host_cpu_initfn(Object *obj= ) >> >> assert(kvm_enabled()); >> >> - if (pcc->info->pvr !=3D mfpvr()) { >> + if (pcc->pvr !=3D mfpvr()) { >> fprintf(stderr, "Your host CPU is unsupported.\n" >> "Please choose a supported model instead, see -cpu ?= =2E\n"); >> exit(1); >> @@ -1275,30 +1275,38 @@ static void kvmppc_host_cpu_class_init(Objec= tClass *oc, void *data) >> PowerPCCPUClass *pcc =3D POWERPC_CPU_CLASS(oc); >> uint32_t host_pvr =3D mfpvr(); >> PowerPCCPUClass *pvr_pcc; >> - ppc_def_t *spec; >> uint32_t vmx =3D kvmppc_get_vmx(); >> uint32_t dfp =3D kvmppc_get_dfp(); >> >> - spec =3D g_malloc0(sizeof(*spec)); >> - >> pvr_pcc =3D ppc_cpu_class_by_pvr(host_pvr); >> if (pvr_pcc !=3D NULL) { >> - memcpy(spec, pvr_pcc->info, sizeof(*spec)); >> + pcc->pvr =3D pvr_pcc->pvr; >> + pcc->svr =3D pvr_pcc->svr; >> + pcc->insns_flags =3D pvr_pcc->insns_flags; >> + pcc->insns_flags2 =3D pvr_pcc->insns_flags2; >> + pcc->msr_mask =3D pvr_pcc->msr_mask; >> + pcc->mmu_model =3D pvr_pcc->mmu_model; >> + pcc->excp_model =3D pvr_pcc->excp_model; >> + pcc->bus_model =3D pvr_pcc->bus_model; >> + pcc->flags =3D pvr_pcc->flags; >> + pcc->bfd_mach =3D pvr_pcc->bfd_mach; >> +#ifdef TARGET_PPC64 >> + pcc->sps =3D pvr_pcc->sps; >> +#endif >> + pcc->init_proc =3D pvr_pcc->init_proc; >> + pcc->check_pow =3D pvr_pcc->check_pow; >=20 > It would be nice to have field copying more streamlined. This way, wh= oever adds a new field to the class needs to know that he also has to c= hange this piece of code, which is non-obvious. >=20 > Speaking of which, why aren't you copying parent_reset for example? parent_reset is already assigned by the .parent's class_init before thi= s class_init is executed. > Or asked differently: Why can't we do a memcpy? We're really trying t= o do a subclass of the parent class here, no? I did suggest making it a subclass in the cover letter, as follow-up. := ) The issue is we need to know which parent class. And we do not have any guarantee that in ..._register_types() the types corresponding to our PVR have already been registered. Therefore we would need to move host CPU type registration to kvm_arch_init(), as suggested by Eduardo for x86. A side effect would b= e that the type is not yet registered at -cpu ? time. If that is acceptable to you (we might hard-code its output within CONFIG_KVM), I can send you a patch. Andreas --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrn= berg