Re: [Qemu-devel] [RFC qom-cpu v2 6/8] target-alpha: Register VMStateDescription for AlphaCPU

[Andreas Färber]

Am 22.02.2013 16:27, schrieb Andreas Färber:
> Am 22.02.2013 15:14, schrieb Juan Quintela:
>> Eduardo Habkost <address@hidden> wrote:
>>> On Fri, Feb 22, 2013 at 02:22:43PM +0100, Juan Quintela wrote:
>>>> Andreas Färber <address@hidden> wrote:
>>>>> Commit b758aca1f6cdb175634812b79f5560c36c902d00 (target-alpha: Enable
>>>>> the alpha-softmmu target.) introduced cpu_{save,load}() functions but
>>>>> didn't define CPU_SAVE_VERSION, so they were never registered.
>>>>>
>>>>> Drop cpu_{save,load}() and register the VMStateDescription via CPUClass.
>>>>> This operates on the AlphaCPU object instead of CPUAlphaState.
>>>>>
>>>>> Signed-off-by: Andreas Färber <address@hidden>
>>>>
>>>>
>>>> Seeing that we are repeating the code all around.  Could we change this
>>>> to something like:
>>>>
>>>>>  
>>>>>  #define ENV_GET_CPU(e) CPU(alpha_env_get_cpu(e))
>>>>>  
>>>>> +#ifdef CONFIG_USER_ONLY
>>>>> +#define vmstate_alpha_cpu vmstate_dummy
>>>>> +#else
>>>>> +extern const struct VMStateDescription vmstate_alpha_cpu;
>>>>> +#endif
>>>>> +
>>>>
>>>> Change this to:
>>>>
>>>> #ifdef CONFIG_USER_ONLY
>>>> #define vmstate_register_cpu(unused, unused)
>>>> #else
>>>> #define vmstate_register_cpu(env, vmstate_cpu)   (env->vmsd = vmstate_cpu)
>>>> #endif
>>>
>>> I like this approach. But using a macro is going to cause unexpected
>>> "variable is unused" gcc warnings. Can we make it a static inline
>>> function instead?
>>
>> types are different, we can pas a pointer to the vmstate, so (untested))
>>
>> static inline void vmstate_register_cpu(struct VMStateDescritpion **arg,
>>                                         struct VMStateDescription 
>> *vmstate_cpu)
>> {
>>          *arg = vmstate_cpu;
>> }
>>
>> Andreas?
> 
> The problem that vmstate_dummy tackles is that vmstate_foo_cpu doesn't
> exist in the non-softmmu case. I don't see how that would work with an
> inline function. We could do a redundant cc->vmsd = NULL; in the
> user-only case.
> 
> As for the argument type, void cpu_class_set_vmsd(CPUClass *cc,
> VMStateDescription *vmsd) would work if we find a solution to the
> extern-referencing problem. I.e., if we have to #define vmstate_foo_cpu
> for a cpu_class_set_vmsd() function call then it doesn't buy us much.

Sent out v3 series with half function, half macro solution.
Would be great if you could look at it this week. ;)

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