Re: [Qemu-devel] [RFC PATCH v3 4/4] hw/intc/arm_gicv3_common: Add vmstate descriptors

[Peter Maydell]

On 22 October 2015 at 15:02, Pavel Fedin <address@hidden> wrote:
> Add state structure descriptors and actually enable live migration.
>
> In order to describe fixed-size bitmaps, VMSTATE_BITMAP_STATIC() macro is
> introduced.
>
> Signed-off-by: Pavel Fedin <address@hidden>
> ---
>  hw/intc/arm_gicv3_common.c  | 58 
> ++++++++++++++++++++++++++++++++++++++++++++-
>  include/migration/vmstate.h |  9 +++++++
>  2 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 2082d05..12d9de1 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -45,11 +45,67 @@ static int gicv3_post_load(void *opaque, int version_id)
>      return 0;
>  }
>
> +static const VMStateDescription vmstate_gicv3_sgi = {
> +    .name = "arm_gicv3_sgi",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_BITMAP(pending, GICv3SGISource, 0, size),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
> +static const VMStateDescription vmstate_gicv3_cpu = {
> +    .name = "arm_gicv3_cpu",
> +    .version_id = 1,
> +    .minimum_version_id = 1,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_BOOL(cpu_enabled, GICv3CPUState),
> +        VMSTATE_UINT32(redist_ctlr, GICv3CPUState),
> +        VMSTATE_UINT32(group, GICv3CPUState),
> +        VMSTATE_UINT32(enabled, GICv3CPUState),
> +        VMSTATE_UINT32(pending, GICv3CPUState),
> +        VMSTATE_UINT32(active, GICv3CPUState),
> +        VMSTATE_UINT32(edge_trigger, GICv3CPUState),
> +        VMSTATE_UINT8_ARRAY(priority, GICv3CPUState, GIC_INTERNAL),
> +        VMSTATE_UINT64(propbaser, GICv3CPUState),
> +        VMSTATE_UINT64(pendbaser, GICv3CPUState),
> +        VMSTATE_UINT32_ARRAY(ctlr, GICv3CPUState, 2),
> +        VMSTATE_UINT32(priority_mask, GICv3CPUState),
> +        VMSTATE_UINT32_ARRAY(bpr, GICv3CPUState, 2),
> +        VMSTATE_UINT32_2DARRAY(apr, GICv3CPUState, 4, 2),
> +        VMSTATE_UINT32(legacy_ctlr, GICv3CPUState),

I think we should keep all the legacy-configs-only state in its
own VMStateDescription and have that only be included in the
migration state if the GIC has legacy enabled (by defining a
suitable .needed function for it). That will let us keep the
legacy-emulation state (which might have to change as we
implement that emulation) away from the state that we care about
for KVM VMs (which are non-legacy-only), reducing the chances we
end up with an awkward migration compat break in future.

(Maybe we could do the same with state which is only for "if
the GIC supports the security extensions", and "if the GIC
supports virtualization". Haven't thought too hard about this yet.)

> +        VMSTATE_STRUCT_ARRAY(sgi, GICv3CPUState, GIC_NR_SGIS, 1,
> +                             vmstate_gicv3_sgi, GICv3SGISource),
> +        VMSTATE_UINT16(current_pending, GICv3CPUState),
> +        VMSTATE_UINT16(running_irq, GICv3CPUState),
> +        VMSTATE_UINT16(running_priority, GICv3CPUState),
> +        VMSTATE_UINT16_ARRAY(last_active, GICv3CPUState, GICV3_MAXIRQ),
> +        VMSTATE_END_OF_LIST()
> +    }
> +};
> +
>  static const VMStateDescription vmstate_gicv3 = {
>      .name = "arm_gicv3",
> -    .unmigratable = 1,
> +    .version_id = 1,
> +    .minimum_version_id = 1,
>      .pre_save = gicv3_pre_save,
>      .post_load = gicv3_post_load,
> +    .fields = (VMStateField[]) {
> +        VMSTATE_UINT32(ctlr, GICv3State),
> +        VMSTATE_BITMAP_STATIC(group, GICv3State , 0, GICV3_MAXSPI),
> +        VMSTATE_BITMAP_STATIC(enabled, GICv3State , 0, GICV3_MAXSPI),
> +        VMSTATE_BITMAP_STATIC(pending, GICv3State , 0, GICV3_MAXSPI),
> +        VMSTATE_BITMAP_STATIC(active, GICv3State , 0, GICV3_MAXSPI),
> +        VMSTATE_BITMAP_STATIC(level, GICv3State , 0, GICV3_MAXSPI),
> +        VMSTATE_BITMAP_STATIC(edge_trigger, GICv3State , 0, GICV3_MAXSPI),
> +        VMSTATE_UINT8_ARRAY(priority, GICv3State, GICV3_MAXSPI),
> +        VMSTATE_UINT8_ARRAY(irq_target, GICv3State, GICV3_MAXSPI),
> +        VMSTATE_UINT64_ARRAY(irq_route, GICv3State, GICV3_MAXSPI),
> +        VMSTATE_STRUCT_VARRAY_POINTER_UINT32(cpu, GICv3State, num_cpu,
> +                                             vmstate_gicv3_cpu, 
> GICv3CPUState),
> +        VMSTATE_END_OF_LIST()
> +    }
>  };
>
>  void gicv3_init_irqs_and_mmio(GICv3State *s, qemu_irq_handler handler,
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 9a65522..7d060e9 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -537,6 +537,15 @@ extern const VMStateInfo vmstate_info_bitmap;
>      .offset       = offsetof(_state, _field),                        \
>  }
>
> +#define VMSTATE_BITMAP_STATIC(_field, _state, _version, _size) {     \
> +    .name         = (stringify(_field)),                             \
> +    .version_id   = (_version),                                      \
> +    .size         = (_size),                                         \
> +    .info         = &vmstate_info_bitmap,                            \
> +    .flags        = VMS_BUFFER,                                      \
> +    .offset       = offsetof(_state, _field),                        \
> +}
> +
>  /* _f : field name
>     _f_n : num of elements field_name
>     _n : num of elements
> --
> 2.4.4

thanks
-- PMM