Re: [PATCH ppc-next v2 42/52] target-ppc: Convert CPU definitions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


Alex

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


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux