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.
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
