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

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

Andreas

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