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

[Paolo Bonzini <pbonzini@redhat.com>]

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 <sanjayl@kymasys.com>
>>
>> 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 <sanjayl@kymasys.com>
>> [james.hogan@imgtec.com: Convert to VMState]
>> Signed-off-by: James Hogan <james.hogan@imgtec.com>
>> Cc: Aurelien Jarno <aurelien@aurel32.net>
>> ---
>> 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 <aurelien@aurel32.net>
>

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

Paolo