Re: [Qemu-ppc] [Qemu-devel] [PATCH v4] ppc: introduce CPUPPCState::cpu_dt_id and CPUState::kvm_cpu_id

[Andreas Färber]

Am 15.11.2013 18:02, schrieb Alexey Kardashevskiy:
> On 16.11.2013 2:11, Andreas Färber wrote:
>> Am 15.11.2013 06:14, schrieb Alexey Kardashevskiy:
>>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>>> index 7739e00..52fc76d 100644
>>> --- a/include/qom/cpu.h
>>> +++ b/include/qom/cpu.h
>>> @@ -197,6 +197,7 @@ struct CPUState {
>>>      bool kvm_vcpu_dirty;
>>>      struct KVMState *kvm_state;
>>>      struct kvm_run *kvm_run;
>>> +    int kvm_cpu_id;
>>>  
>>>      /* TODO Move common fields from CPUArchState here. */
>>>      int cpu_index; /* used by alpha TCG */
>>
>> Here you are adding a field to CPUState, fine with me. (Please add a
>> documentation line above the struct then.)
>>
>>> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
>>> index bb84767..8052f6b 100644
>>> --- a/target-ppc/cpu.h
>>> +++ b/target-ppc/cpu.h
>>> @@ -1074,6 +1074,9 @@ struct CPUPPCState {
>>>       */
>>>      uint8_t fit_period[4];
>>>      uint8_t wdt_period[4];
>>> +
>>> +    /* The CPU index used in the device tree. KVM uses this index too */
>>> +    int cpu_dt_id;
>>
>> But I believe I have requested a number of times not to add random
>> fields to CPUPPCState unless they are accessed by TCG. Please place the
>> new field in PowerPCCPU instead and put the description into the struct
>> documentation.
> 
> 
> It is accessed by xics.c which is used in TCG. Or I misinterpret the
> rule, do not I?

Yes, you do. ;)

The question is, is the field accessed using offsetof(CPUPPCState, x)
such as in target-ppc/translate.c (search for TCG_AREG0). That's what
CPUPPCState is still needed for - mainly GPRs and the TLBs that TCG ops
load from / store to using host instructions with immediate offset.

Whether it is used in a HELPER() / helper_* function called from TCG
(target-ppc/*helper.c) has become less relevant since the optimization
of ppc_env_get_cpu() macro.

Neither of that seem to be the case here, unless I missed something.

I still hope we can one day get rid of CPUPPCState by having only
numeric-offset accesses from cpu+sizeof(CPUState) and all fields
(including TLB, as Paolo recently pointed out) directly in PowerPCCPU.

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg