Re: [Qemu-devel] [PATCH v3] hw/mips: gt64xxx_pci: Add VMStateDescription

[Paolo Bonzini]

Il 20/06/2014 12:58, Aurelien Jarno ha scritto:
> On Fri, Jun 20, 2014 at 11:25:47AM +0100, James Hogan wrote:
> > From: Sanjay Lal <address@hidden>
> >
> > Add VMStateDescription for GT64120 PCI emulation used by the Malta
> > platform, to allow it to work with savevm/loadvm and live migration.
> >
> > The entire register array is saved/restored using VMSTATE_UINT32_ARRAY
> > (fixed length GT_REGS = 1024).
> >
> > Signed-off-by: Sanjay Lal <address@hidden>
> > address@hidden: Convert to VMState]
> > Signed-off-by: James Hogan <address@hidden>
> > Cc: Aurelien Jarno <address@hidden>
> > ---
> > This is based on "[Patch 03/12] KVM/MIPS: Add save/restore state APIs
> > for saving/restoring KVM guests."[1].
> >
> > Changes in v3:
> >  - Save entire register array using VMSTATE_UINT32_ARRAY (which is fixed
> >    length of GT_REGS = 1024) rather than individual registers. This is
> >    safer in case an important register is missed or new emulated
> >    functionality is added. (Aurelien Jarno)
> >
> > Changes in v2:
> >  - Expand commit message
> >  - Convert to VMState (Peter Maydell)
> >
> > [1] https://lists.gnu.org/archive/html/qemu-devel/2013-03/msg00195.html
> > ---
> >  hw/mips/gt64xxx_pci.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c
> > index 6398514c99d2..22f63ce0c8af 100644
> > --- a/hw/mips/gt64xxx_pci.c
> > +++ b/hw/mips/gt64xxx_pci.c
> > @@ -312,6 +312,27 @@ static void gt64120_pci_mapping(GT64120State *s)
> >      }
> >  }
> >
> > +static int gt64120_post_load(void *opaque, int version_id)
> > +{
> > +    GT64120State *s = opaque;
> > +
> > +    gt64120_isd_mapping(s);
> > +    gt64120_pci_mapping(s);
> > +
> > +    return 0;
> > +}
> > +
> > +static const VMStateDescription vmstate_gt64120 = {
> > +    .name = "gt64120",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .post_load = gt64120_post_load,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_UINT32_ARRAY(regs, GT64120State, GT_REGS),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +
> >  static void gt64120_writel (void *opaque, hwaddr addr,
> >                              uint64_t val, unsigned size)
> >  {
> > @@ -1174,9 +1195,11 @@ static const TypeInfo gt64120_pci_info = {
> >
> >  static void gt64120_class_init(ObjectClass *klass, void *data)
> >  {
> > +    DeviceClass *dc = DEVICE_CLASS(klass);
> >      SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
> >
> >      sdc->init = gt64120_init;
> > +    dc->vmsd = &vmstate_gt64120;
> >  }
> >
> >  static const TypeInfo gt64120_info = {
>
>
> Thanks for this new version, I'll apply it in the next days unless
> someone comes with good arguments why we should not do that.
>
> Reviewed-by: Aurelien Jarno <address@hidden>

FWIW it looks good to me.  It's simpler this way.

Paolo