Am 22.02.2013 17:32, schrieb Alexander Graf: > > On 22.02.2013, at 17:31, Andreas Färber wrote: > >> Am 22.02.2013 15:23, schrieb Alexander Graf: >>> >>> On 18.02.2013, at 10:16, Andreas Färber wrote: >>> >>>> 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 != mfpvr()) { >>>> + if (pcc->pvr != mfpvr()) { >>>> fprintf(stderr, "Your host CPU is unsupported.\n" >>>> "Please choose a supported model instead, see -cpu ?.\n"); >>>> exit(1); >>>> @@ -1275,30 +1275,38 @@ static void kvmppc_host_cpu_class_init(ObjectClass *oc, void *data) >>>> PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc); >>>> uint32_t host_pvr = mfpvr(); >>>> PowerPCCPUClass *pvr_pcc; >>>> - ppc_def_t *spec; >>>> uint32_t vmx = kvmppc_get_vmx(); >>>> uint32_t dfp = kvmppc_get_dfp(); >>>> >>>> - spec = g_malloc0(sizeof(*spec)); >>>> - >>>> pvr_pcc = ppc_cpu_class_by_pvr(host_pvr); >>>> if (pvr_pcc != NULL) { >>>> - memcpy(spec, pvr_pcc->info, sizeof(*spec)); >>>> + pcc->pvr = pvr_pcc->pvr; >>>> + pcc->svr = pvr_pcc->svr; >>>> + pcc->insns_flags = pvr_pcc->insns_flags; >>>> + pcc->insns_flags2 = pvr_pcc->insns_flags2; >>>> + pcc->msr_mask = pvr_pcc->msr_mask; >>>> + pcc->mmu_model = pvr_pcc->mmu_model; >>>> + pcc->excp_model = pvr_pcc->excp_model; >>>> + pcc->bus_model = pvr_pcc->bus_model; >>>> + pcc->flags = pvr_pcc->flags; >>>> + pcc->bfd_mach = pvr_pcc->bfd_mach; >>>> +#ifdef TARGET_PPC64 >>>> + pcc->sps = pvr_pcc->sps; >>>> +#endif >>>> + pcc->init_proc = pvr_pcc->init_proc; >>>> + pcc->check_pow = pvr_pcc->check_pow; >>> >>> It would be nice to have field copying more streamlined. This way, whoever adds a new field to the class needs to know that he also has to change this piece of code, which is non-obvious. >>> >>> Speaking of which, why aren't you copying parent_reset for example? >> >> parent_reset is already assigned by the .parent's class_init before this >> class_init is executed. >> >>> Or asked differently: Why can't we do a memcpy? We're really trying to 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 be >> 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. > > Yes, I think that's the most reasonable way forward. We can always print it explicitly in -cpu ?. Done: http://patchwork.ozlabs.org/patch/222735/ Tested the POWER5+ case of no matching PVR and the TCG case. Wasn't able to test on POWER7 after messing up my command line. ;) Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html