Message ID | 1403259947-32233-1-git-send-email-james.hogan@imgtec.com |
---|---|
State | Superseded |
Headers | show |
On 20/06/14 11:25, 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) For the record, Aurelien's question about saving all registers or only specific ones is still open (I also don't have enough experience to be sure, although I prefer the approach in v3). On 19/06/14 16:51, Aurelien Jarno wrote: > That said there might be some drawbacks in doing that, and I don't > really have enough experience to judge about that. It would be nice if > some more people can give their opinion there. Note, new emulated functionality may still require VMState versioning to handle unexpected zero register values in restored state. Cheers James
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>
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
Am 20.06.2014 12:58, schrieb Aurelien Jarno: > 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> No objection to the code, I just think that "hw/mips:" is redundant in the subject, although there's no precedent yet. http://git.qemu-project.org/?p=qemu.git;a=history;f=hw/mips/gt64xxx_pci.c;h=6398514c99d2b15cd047149c8457717e352e9347;hb=HEAD Same applies to the other MIPS patch. Cheers, Andreas
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 = {