Re: [Qemu-devel] [PATCH qom-cpu v3 4/6] cpu: Introduce cpu_class_set_vmsd()

Re: [Qemu-devel] [PATCH qom-cpu v3 4/6] cpu: Introduce cpu_class_set_vmsd()

[Andreas Färber]

Am 26.02.2013 20:32, schrieb Eduardo Habkost:
> On Mon, Feb 25, 2013 at 07:22:48PM +0100, Andreas Färber wrote:
>> This setter avoids redefining each VMStateDescription value to
>> vmstate_dummy by not referencing the value for CONFIG_USER_ONLY.
>>
>> Suggested-by: Juan Quintela <quintela@redhat.com>
>> Signed-off-by: Andreas Färber <afaerber@suse.de>
>> ---
>>  include/qom/cpu.h |   17 +++++++++++++++++
>>  1 Datei geändert, 17 Zeilen hinzugefügt(+)
>>
>> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
>> index 1106e39..65e24d3 100644
>> --- a/include/qom/cpu.h
>> +++ b/include/qom/cpu.h
>> @@ -135,6 +135,23 @@ void cpu_reset(CPUState *cpu);
>>  ObjectClass *cpu_class_by_name(const char *typename, const char *cpu_model);
>>  
>>  /**
>> + * cpu_class_set_vmsd:
>> + * @cc: CPU class
>> + * @value: Value to set.
>> + *
>> + * Sets #VMStateDescription for @cc.
>> + */
>> +#ifndef CONFIG_USER_ONLY
>> +static inline void cpu_class_set_vmsd(CPUClass *cc,
>> +                                      const struct VMStateDescription *value)
>> +{
>> +    cc->vmsd = value;
>> +}
>> +#else
>> +#define cpu_class_set_vmsd(cpu, val) ((cpu)->vmsd = NULL)
> 
> Even after reading the description above, I was going to ask you why
> this was not an inline function as well. But then I read patch 5/6 and
> understood why.  :-)
> 
> I would add a coment here noting that "val" is actually expected to be
> never evaluated when CONFIG_USER_ONLY is defined.

Done. I also renamed the arguments to match those of the function.

> 
> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>

Thanks,
Andreas

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

Re: [Qemu-devel] [PATCH qom-cpu v3 0/6] QOM CPUState VMStateDescriptions

[Andreas Färber]

Am 25.02.2013 19:22, schrieb Andreas Färber:
> Andreas Färber (6):
>   vmstate: Make vmstate_register() static inline
>   stubs: Add a vmstate_dummy struct for CONFIG_USER_ONLY
>   cpu: Register VMStateDescription through CPUState
>   cpu: Introduce cpu_class_set_vmsd()
>   target-i386: Update VMStateDescription to X86CPU

Applied these to qom-cpu (with change suggested by Eduardo):
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

>   target-lm32: Update VMStateDescription to LM32CPU

Juan/Michael, could you take a look at this one?

Thanks,
Andreas

> 
>  exec.c                      |   11 ++-
>  include/migration/vmstate.h |   16 +++-
>  include/qom/cpu.h           |   20 ++++
>  savevm.c                    |    7 --
>  stubs/vmstate.c             |    2 +
>  target-i386/cpu-qom.h       |    4 +
>  target-i386/cpu.c           |    2 +
>  target-i386/cpu.h           |    2 -
>  target-i386/machine.c       |  215 +++++++++++++++++++++----------------------
>  target-lm32/cpu-qom.h       |    4 +
>  target-lm32/cpu.c           |    2 +
>  target-lm32/cpu.h           |    2 -
>  target-lm32/machine.c       |   25 ++---
>  13 Dateien geändert, 177 Zeilen hinzugefügt(+), 135 Zeilen entfernt(-)

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

Re: [Qemu-devel] [PATCH qom-cpu v3 6/6] target-lm32: Update VMStateDescription to LM32CPU

[Michael Walle]

Am Montag 25 Februar 2013, 19:22:50 schrieb Andreas Färber:
> Add a vmstate_lm32_cpu referencing the previous VMStateDescription as a
> sub-struct and hook it up to CPUClass::vmsd. Drop cpu_{save,load}().
> 
> Signed-off-by: Andreas Färber <afaerber@suse.de>

Acked-by: Michael Walle <michael@walle.cc>

> ---
>  target-lm32/cpu-qom.h |    4 ++++
>  target-lm32/cpu.c     |    2 ++
>  target-lm32/cpu.h     |    2 --
>  target-lm32/machine.c |   25 +++++++++++++------------
>  4 Dateien geändert, 19 Zeilen hinzugefügt(+), 14 Zeilen entfernt(-)
> 
> diff --git a/target-lm32/cpu-qom.h b/target-lm32/cpu-qom.h
> index c0b6ce5..e00cca6 100644
> --- a/target-lm32/cpu-qom.h
> +++ b/target-lm32/cpu-qom.h
> @@ -71,4 +71,8 @@ static inline LM32CPU *lm32_env_get_cpu(CPULM32State
> *env)
> 
>  #define ENV_OFFSET offsetof(LM32CPU, env)
> 
> +#ifndef CONFIG_USER_ONLY
> +extern const struct VMStateDescription vmstate_lm32_cpu;
> +#endif
> +
>  #endif
> diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c
> index a2badb5..3199102 100644
> --- a/target-lm32/cpu.c
> +++ b/target-lm32/cpu.c
> @@ -83,6 +83,8 @@ static void lm32_cpu_class_init(ObjectClass *oc, void
> *data)
> 
>      lcc->parent_reset = cc->reset;
>      cc->reset = lm32_cpu_reset;
> +
> +    cpu_class_set_vmsd(cc, &vmstate_lm32_cpu);
>  }
> 
>  static const TypeInfo lm32_cpu_type_info = {
> diff --git a/target-lm32/cpu.h b/target-lm32/cpu.h
> index 6948d0e..e0bbe04 100644
> --- a/target-lm32/cpu.h
> +++ b/target-lm32/cpu.h
> @@ -212,8 +212,6 @@ static inline CPULM32State *cpu_init(const char
> *cpu_model) #define cpu_gen_code cpu_lm32_gen_code
>  #define cpu_signal_handler cpu_lm32_signal_handler
> 
> -#define CPU_SAVE_VERSION 1
> -
>  int cpu_lm32_handle_mmu_fault(CPULM32State *env, target_ulong address, int
> rw, int mmu_idx);
>  #define cpu_handle_mmu_fault cpu_lm32_handle_mmu_fault
> diff --git a/target-lm32/machine.c b/target-lm32/machine.c
> index 6802e81..9e0919c 100644
> --- a/target-lm32/machine.c
> +++ b/target-lm32/machine.c
> @@ -1,9 +1,9 @@
>  #include "hw/hw.h"
>  #include "hw/boards.h"
> 
> -static const VMStateDescription vmstate_cpu = {
> -    .name = "cpu",
> -    .version_id = CPU_SAVE_VERSION,
> +static const VMStateDescription vmstate_env = {
> +    .name = "env",
> +    .version_id = 1,
>      .minimum_version_id = 1,
>      .minimum_version_id_old = 1,
>      .fields      = (VMStateField[]) {
> @@ -22,12 +22,13 @@ static const VMStateDescription vmstate_cpu = {
>      }
>  };
> 
> -void cpu_save(QEMUFile *f, void *opaque)
> -{
> -    vmstate_save_state(f, &vmstate_cpu, opaque);
> -}
> -
> -int cpu_load(QEMUFile *f, void *opaque, int version_id)
> -{
> -    return vmstate_load_state(f, &vmstate_cpu, opaque, version_id);
> -}
> +const VMStateDescription vmstate_lm32_cpu = {
> +    .name = "cpu",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .minimum_version_id_old = 1,
> +    .fields      = (VMStateField[]) {
> +        VMSTATE_STRUCT(env, LM32CPU, 1, vmstate_env, CPULM32State),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};