* [Qemu-devel] [PATCH v3 0/3] arm_gic: convert to vmstate
@ 2013-03-22 18:02 Peter Maydell
2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 1/3] vmstate: Add support for two dimensional arrays Peter Maydell
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Peter Maydell @ 2013-03-22 18:02 UTC (permalink / raw)
To: qemu-devel; +Cc: Igor Mitsyanko, Gerd Hoffmann, Andreas Färber, patches
Convert the arm_gic save/load support from hand-coded save/load functions
to use VMState. This seems like a good thing to do before we get to the
point with KVM/ARM that we need to start supporting between-version
migration...
Changes v2->v3:
* implement 2D array support in vmstate.h so we don't need to abuse
VMSTATE_BUFFER_UNSAFE in a way that probably won't work for 16 bit
values when source and destination have different endianness
Changes v1->v2:
* fix true/false mixup that stopped armv7m from booting
Peter Maydell (3):
vmstate: Add support for two dimensional arrays
arm_gic: Fix sizes of state fields in preparation for vmstate support
hw/arm_gic_common: Use vmstate struct rather than save/load functions
hw/arm_gic_common.c | 112 +++++++++++++++++--------------------------
hw/arm_gic_internal.h | 42 ++++++++--------
hw/armv7m_nvic.c | 4 +-
include/migration/vmstate.h | 27 +++++++++++
4 files changed, 93 insertions(+), 92 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 6+ messages in thread* [Qemu-devel] [PATCH v3 1/3] vmstate: Add support for two dimensional arrays 2013-03-22 18:02 [Qemu-devel] [PATCH v3 0/3] arm_gic: convert to vmstate Peter Maydell @ 2013-03-22 18:02 ` Peter Maydell 2013-03-22 19:20 ` Igor Mitsyanko 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 2/3] arm_gic: Fix sizes of state fields in preparation for vmstate support Peter Maydell 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 3/3] hw/arm_gic_common: Use vmstate struct rather than save/load functions Peter Maydell 2 siblings, 1 reply; 6+ messages in thread From: Peter Maydell @ 2013-03-22 18:02 UTC (permalink / raw) To: qemu-devel; +Cc: Igor Mitsyanko, Gerd Hoffmann, Andreas Färber, patches Add support for migrating two dimensional arrays, by defining a set of new macros VMSTATE_*_2DARRAY paralleling the existing VMSTATE_*_ARRAY macros. 2D arrays are handled the same for actual state serialization; the only difference is that the type check has to change for a 2D array. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- include/migration/vmstate.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index 6666d27..24bc537 100644 --- a/include/migration/vmstate.h +++ b/include/migration/vmstate.h @@ -161,6 +161,7 @@ extern const VMStateInfo vmstate_info_buffer; extern const VMStateInfo vmstate_info_unused_buffer; extern const VMStateInfo vmstate_info_bitmap; +#define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0) #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0) #define type_check_pointer(t1,t2) ((t1**)0 - (t2*)0) @@ -176,6 +177,10 @@ extern const VMStateInfo vmstate_info_bitmap; (offsetof(_state, _field) + \ type_check_array(_type, typeof_field(_state, _field), _num)) +#define vmstate_offset_2darray(_state, _field, _type, _n1, _n2) \ + (offsetof(_state, _field) + \ + type_check_2darray(_type, typeof_field(_state, _field), _n1, _n2)) + #define vmstate_offset_sub_array(_state, _field, _type, _start) \ (offsetof(_state, _field[_start])) @@ -221,6 +226,16 @@ extern const VMStateInfo vmstate_info_bitmap; .offset = vmstate_offset_array(_state, _field, _type, _num), \ } +#define VMSTATE_2DARRAY(_field, _state, _n1, _n2, _version, _info, _type) { \ + .name = (stringify(_field)), \ + .version_id = (_version), \ + .num = (_n1) * (_n2), \ + .info = &(_info), \ + .size = sizeof(_type), \ + .flags = VMS_ARRAY, \ + .offset = vmstate_offset_2darray(_state, _field, _type, _n1, _n2), \ +} + #define VMSTATE_ARRAY_TEST(_field, _state, _num, _test, _info, _type) {\ .name = (stringify(_field)), \ .field_exists = (_test), \ @@ -554,15 +569,27 @@ extern const VMStateInfo vmstate_info_bitmap; #define VMSTATE_UINT16_ARRAY_V(_f, _s, _n, _v) \ VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint16, uint16_t) +#define VMSTATE_UINT16_2DARRAY_V(_f, _s, _n1, _n2, _v) \ + VMSTATE_2DARRAY(_f, _s, _n1, _n2, _v, vmstate_info_uint16, uint16_t) + #define VMSTATE_UINT16_ARRAY(_f, _s, _n) \ VMSTATE_UINT16_ARRAY_V(_f, _s, _n, 0) +#define VMSTATE_UINT16_2DARRAY(_f, _s, _n1, _n2) \ + VMSTATE_UINT16_2DARRAY_V(_f, _s, _n1, _n2, 0) + +#define VMSTATE_UINT8_2DARRAY_V(_f, _s, _n1, _n2, _v) \ + VMSTATE_2DARRAY(_f, _s, _n1, _n2, _v, vmstate_info_uint8, uint8_t) + #define VMSTATE_UINT8_ARRAY_V(_f, _s, _n, _v) \ VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint8, uint8_t) #define VMSTATE_UINT8_ARRAY(_f, _s, _n) \ VMSTATE_UINT8_ARRAY_V(_f, _s, _n, 0) +#define VMSTATE_UINT8_2DARRAY(_f, _s, _n1, _n2) \ + VMSTATE_UINT8_2DARRAY_V(_f, _s, _n1, _n2, 0) + #define VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v) \ VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint32, uint32_t) -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/3] vmstate: Add support for two dimensional arrays 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 1/3] vmstate: Add support for two dimensional arrays Peter Maydell @ 2013-03-22 19:20 ` Igor Mitsyanko 0 siblings, 0 replies; 6+ messages in thread From: Igor Mitsyanko @ 2013-03-22 19:20 UTC (permalink / raw) To: Peter Maydell; +Cc: qemu-devel, Gerd Hoffmann, Andreas, patches [-- Attachment #1: Type: text/plain, Size: 3818 bytes --] On Mar 22, 2013 10:02 PM, "Peter Maydell" <peter.maydell@linaro.org> wrote: > > Add support for migrating two dimensional arrays, by defining > a set of new macros VMSTATE_*_2DARRAY paralleling the existing > VMSTATE_*_ARRAY macros. 2D arrays are handled the same for actual > state serialization; the only difference is that the type check > has to change for a 2D array. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > include/migration/vmstate.h | 27 +++++++++++++++++++++++++++ > 1 file changed, 27 insertions(+) > Reviewed-by: Igor Mitsyanko <i.mitsyanko@gmail.com> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index 6666d27..24bc537 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -161,6 +161,7 @@ extern const VMStateInfo vmstate_info_buffer; > extern const VMStateInfo vmstate_info_unused_buffer; > extern const VMStateInfo vmstate_info_bitmap; > > +#define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0) > #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0) > #define type_check_pointer(t1,t2) ((t1**)0 - (t2*)0) > > @@ -176,6 +177,10 @@ extern const VMStateInfo vmstate_info_bitmap; > (offsetof(_state, _field) + \ > type_check_array(_type, typeof_field(_state, _field), _num)) > > +#define vmstate_offset_2darray(_state, _field, _type, _n1, _n2) \ > + (offsetof(_state, _field) + \ > + type_check_2darray(_type, typeof_field(_state, _field), _n1, _n2)) > + > #define vmstate_offset_sub_array(_state, _field, _type, _start) \ > (offsetof(_state, _field[_start])) > > @@ -221,6 +226,16 @@ extern const VMStateInfo vmstate_info_bitmap; > .offset = vmstate_offset_array(_state, _field, _type, _num), \ > } > > +#define VMSTATE_2DARRAY(_field, _state, _n1, _n2, _version, _info, _type) { \ > + .name = (stringify(_field)), \ > + .version_id = (_version), \ > + .num = (_n1) * (_n2), \ > + .info = &(_info), \ > + .size = sizeof(_type), \ > + .flags = VMS_ARRAY, \ > + .offset = vmstate_offset_2darray(_state, _field, _type, _n1, _n2), \ > +} > + > #define VMSTATE_ARRAY_TEST(_field, _state, _num, _test, _info, _type) {\ > .name = (stringify(_field)), \ > .field_exists = (_test), \ > @@ -554,15 +569,27 @@ extern const VMStateInfo vmstate_info_bitmap; > #define VMSTATE_UINT16_ARRAY_V(_f, _s, _n, _v) \ > VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint16, uint16_t) > > +#define VMSTATE_UINT16_2DARRAY_V(_f, _s, _n1, _n2, _v) \ > + VMSTATE_2DARRAY(_f, _s, _n1, _n2, _v, vmstate_info_uint16, uint16_t) > + > #define VMSTATE_UINT16_ARRAY(_f, _s, _n) \ > VMSTATE_UINT16_ARRAY_V(_f, _s, _n, 0) > > +#define VMSTATE_UINT16_2DARRAY(_f, _s, _n1, _n2) \ > + VMSTATE_UINT16_2DARRAY_V(_f, _s, _n1, _n2, 0) > + > +#define VMSTATE_UINT8_2DARRAY_V(_f, _s, _n1, _n2, _v) \ > + VMSTATE_2DARRAY(_f, _s, _n1, _n2, _v, vmstate_info_uint8, uint8_t) > + > #define VMSTATE_UINT8_ARRAY_V(_f, _s, _n, _v) \ > VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint8, uint8_t) > > #define VMSTATE_UINT8_ARRAY(_f, _s, _n) \ > VMSTATE_UINT8_ARRAY_V(_f, _s, _n, 0) > > +#define VMSTATE_UINT8_2DARRAY(_f, _s, _n1, _n2) \ > + VMSTATE_UINT8_2DARRAY_V(_f, _s, _n1, _n2, 0) > + > #define VMSTATE_UINT32_ARRAY_V(_f, _s, _n, _v) \ > VMSTATE_ARRAY(_f, _s, _n, _v, vmstate_info_uint32, uint32_t) > > -- > 1.7.9.5 > [-- Attachment #2: Type: text/html, Size: 4926 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v3 2/3] arm_gic: Fix sizes of state fields in preparation for vmstate support 2013-03-22 18:02 [Qemu-devel] [PATCH v3 0/3] arm_gic: convert to vmstate Peter Maydell 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 1/3] vmstate: Add support for two dimensional arrays Peter Maydell @ 2013-03-22 18:02 ` Peter Maydell 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 3/3] hw/arm_gic_common: Use vmstate struct rather than save/load functions Peter Maydell 2 siblings, 0 replies; 6+ messages in thread From: Peter Maydell @ 2013-03-22 18:02 UTC (permalink / raw) To: qemu-devel; +Cc: Igor Mitsyanko, Gerd Hoffmann, Andreas Färber, patches In preparation for switching to vmstate for migration support, fix the sizes of various GIC state fields. In particular, we replace all the bitfields (which VMState can't deal with) with straightforward uint8_t values which we do bit operations on. (The bitfields made more sense when NCPU was set differently in different situations, but we now always model at the architectural limit of 8.) Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Igor Mitsyanko <i.mitsyanko@gmail.com> Reviewed-by: Andreas Färber <afaerber@suse.de> --- hw/arm_gic_common.c | 4 ++-- hw/arm_gic_internal.h | 42 +++++++++++++++++++++--------------------- hw/armv7m_nvic.c | 4 ++-- 3 files changed, 25 insertions(+), 25 deletions(-) diff --git a/hw/arm_gic_common.c b/hw/arm_gic_common.c index f2dc8bf..f95bec3 100644 --- a/hw/arm_gic_common.c +++ b/hw/arm_gic_common.c @@ -149,7 +149,7 @@ static void arm_gic_common_reset(DeviceState *dev) s->current_pending[i] = 1023; s->running_irq[i] = 1023; s->running_priority[i] = 0x100; - s->cpu_enabled[i] = 0; + s->cpu_enabled[i] = false; } for (i = 0; i < 16; i++) { GIC_SET_ENABLED(i, ALL_CPU_MASK); @@ -161,7 +161,7 @@ static void arm_gic_common_reset(DeviceState *dev) s->irq_target[i] = 1; } } - s->enabled = 0; + s->enabled = false; } static Property arm_gic_common_properties[] = { diff --git a/hw/arm_gic_internal.h b/hw/arm_gic_internal.h index 3e1928b..99a3bc3 100644 --- a/hw/arm_gic_internal.h +++ b/hw/arm_gic_internal.h @@ -45,14 +45,14 @@ #define GIC_SET_ACTIVE(irq, cm) s->irq_state[irq].active |= (cm) #define GIC_CLEAR_ACTIVE(irq, cm) s->irq_state[irq].active &= ~(cm) #define GIC_TEST_ACTIVE(irq, cm) ((s->irq_state[irq].active & (cm)) != 0) -#define GIC_SET_MODEL(irq) s->irq_state[irq].model = 1 -#define GIC_CLEAR_MODEL(irq) s->irq_state[irq].model = 0 +#define GIC_SET_MODEL(irq) s->irq_state[irq].model = true +#define GIC_CLEAR_MODEL(irq) s->irq_state[irq].model = false #define GIC_TEST_MODEL(irq) s->irq_state[irq].model #define GIC_SET_LEVEL(irq, cm) s->irq_state[irq].level = (cm) #define GIC_CLEAR_LEVEL(irq, cm) s->irq_state[irq].level &= ~(cm) #define GIC_TEST_LEVEL(irq, cm) ((s->irq_state[irq].level & (cm)) != 0) -#define GIC_SET_TRIGGER(irq) s->irq_state[irq].trigger = 1 -#define GIC_CLEAR_TRIGGER(irq) s->irq_state[irq].trigger = 0 +#define GIC_SET_TRIGGER(irq) s->irq_state[irq].trigger = true +#define GIC_CLEAR_TRIGGER(irq) s->irq_state[irq].trigger = false #define GIC_TEST_TRIGGER(irq) s->irq_state[irq].trigger #define GIC_GET_PRIORITY(irq, cpu) (((irq) < GIC_INTERNAL) ? \ s->priority1[irq][cpu] : \ @@ -61,30 +61,30 @@ typedef struct gic_irq_state { /* The enable bits are only banked for per-cpu interrupts. */ - unsigned enabled:NCPU; - unsigned pending:NCPU; - unsigned active:NCPU; - unsigned level:NCPU; - unsigned model:1; /* 0 = N:N, 1 = 1:N */ - unsigned trigger:1; /* nonzero = edge triggered. */ + uint8_t enabled; + uint8_t pending; + uint8_t active; + uint8_t level; + bool model; /* 0 = N:N, 1 = 1:N */ + bool trigger; /* nonzero = edge triggered. */ } gic_irq_state; typedef struct GICState { SysBusDevice busdev; qemu_irq parent_irq[NCPU]; - int enabled; - int cpu_enabled[NCPU]; + bool enabled; + bool cpu_enabled[NCPU]; gic_irq_state irq_state[GIC_MAXIRQ]; - int irq_target[GIC_MAXIRQ]; - int priority1[GIC_INTERNAL][NCPU]; - int priority2[GIC_MAXIRQ - GIC_INTERNAL]; - int last_active[GIC_MAXIRQ][NCPU]; - - int priority_mask[NCPU]; - int running_irq[NCPU]; - int running_priority[NCPU]; - int current_pending[NCPU]; + uint8_t irq_target[GIC_MAXIRQ]; + uint8_t priority1[GIC_INTERNAL][NCPU]; + uint8_t priority2[GIC_MAXIRQ - GIC_INTERNAL]; + uint16_t last_active[GIC_MAXIRQ][NCPU]; + + uint16_t priority_mask[NCPU]; + uint16_t running_irq[NCPU]; + uint16_t running_priority[NCPU]; + uint16_t current_pending[NCPU]; uint32_t num_cpu; diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c index d198cfd..2351200 100644 --- a/hw/armv7m_nvic.c +++ b/hw/armv7m_nvic.c @@ -458,10 +458,10 @@ static void armv7m_nvic_reset(DeviceState *dev) * as enabled by default, and with a priority mask which allows * all interrupts through. */ - s->gic.cpu_enabled[0] = 1; + s->gic.cpu_enabled[0] = true; s->gic.priority_mask[0] = 0x100; /* The NVIC as a whole is always enabled. */ - s->gic.enabled = 1; + s->gic.enabled = true; systick_reset(s); } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH v3 3/3] hw/arm_gic_common: Use vmstate struct rather than save/load functions 2013-03-22 18:02 [Qemu-devel] [PATCH v3 0/3] arm_gic: convert to vmstate Peter Maydell 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 1/3] vmstate: Add support for two dimensional arrays Peter Maydell 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 2/3] arm_gic: Fix sizes of state fields in preparation for vmstate support Peter Maydell @ 2013-03-22 18:02 ` Peter Maydell 2013-03-22 19:35 ` Igor Mitsyanko 2 siblings, 1 reply; 6+ messages in thread From: Peter Maydell @ 2013-03-22 18:02 UTC (permalink / raw) To: qemu-devel; +Cc: Igor Mitsyanko, Gerd Hoffmann, Andreas Färber, patches Update the GIC save/restore to use vmstate rather than hand-rolled save/load functions. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- hw/arm_gic_common.c | 108 +++++++++++++++++++-------------------------------- 1 file changed, 41 insertions(+), 67 deletions(-) diff --git a/hw/arm_gic_common.c b/hw/arm_gic_common.c index f95bec3..71594f1 100644 --- a/hw/arm_gic_common.c +++ b/hw/arm_gic_common.c @@ -20,90 +20,65 @@ #include "hw/arm_gic_internal.h" -static void gic_save(QEMUFile *f, void *opaque) +static void gic_pre_save(void *opaque) { GICState *s = (GICState *)opaque; ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s); - int i; - int j; if (c->pre_save) { c->pre_save(s); } - - qemu_put_be32(f, s->enabled); - for (i = 0; i < s->num_cpu; i++) { - qemu_put_be32(f, s->cpu_enabled[i]); - for (j = 0; j < GIC_INTERNAL; j++) { - qemu_put_be32(f, s->priority1[j][i]); - } - for (j = 0; j < s->num_irq; j++) { - qemu_put_be32(f, s->last_active[j][i]); - } - qemu_put_be32(f, s->priority_mask[i]); - qemu_put_be32(f, s->running_irq[i]); - qemu_put_be32(f, s->running_priority[i]); - qemu_put_be32(f, s->current_pending[i]); - } - for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) { - qemu_put_be32(f, s->priority2[i]); - } - for (i = 0; i < s->num_irq; i++) { - qemu_put_be32(f, s->irq_target[i]); - qemu_put_byte(f, s->irq_state[i].enabled); - qemu_put_byte(f, s->irq_state[i].pending); - qemu_put_byte(f, s->irq_state[i].active); - qemu_put_byte(f, s->irq_state[i].level); - qemu_put_byte(f, s->irq_state[i].model); - qemu_put_byte(f, s->irq_state[i].trigger); - } } -static int gic_load(QEMUFile *f, void *opaque, int version_id) +static int gic_post_load(void *opaque, int version_id) { GICState *s = (GICState *)opaque; ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s); - int i; - int j; - - if (version_id != 3) { - return -EINVAL; - } - - s->enabled = qemu_get_be32(f); - for (i = 0; i < s->num_cpu; i++) { - s->cpu_enabled[i] = qemu_get_be32(f); - for (j = 0; j < GIC_INTERNAL; j++) { - s->priority1[j][i] = qemu_get_be32(f); - } - for (j = 0; j < s->num_irq; j++) { - s->last_active[j][i] = qemu_get_be32(f); - } - s->priority_mask[i] = qemu_get_be32(f); - s->running_irq[i] = qemu_get_be32(f); - s->running_priority[i] = qemu_get_be32(f); - s->current_pending[i] = qemu_get_be32(f); - } - for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) { - s->priority2[i] = qemu_get_be32(f); - } - for (i = 0; i < s->num_irq; i++) { - s->irq_target[i] = qemu_get_be32(f); - s->irq_state[i].enabled = qemu_get_byte(f); - s->irq_state[i].pending = qemu_get_byte(f); - s->irq_state[i].active = qemu_get_byte(f); - s->irq_state[i].level = qemu_get_byte(f); - s->irq_state[i].model = qemu_get_byte(f); - s->irq_state[i].trigger = qemu_get_byte(f); - } if (c->post_load) { c->post_load(s); } - return 0; } +static const VMStateDescription vmstate_gic_irq_state = { + .name = "arm_gic_irq_state", + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT8(enabled, gic_irq_state), + VMSTATE_UINT8(pending, gic_irq_state), + VMSTATE_UINT8(active, gic_irq_state), + VMSTATE_UINT8(level, gic_irq_state), + VMSTATE_BOOL(model, gic_irq_state), + VMSTATE_BOOL(trigger, gic_irq_state), + VMSTATE_END_OF_LIST() + } +}; + +static const VMStateDescription vmstate_gic = { + .name = "arm_gic", + .version_id = 4, + .minimum_version_id = 4, + .pre_save = gic_pre_save, + .post_load = gic_post_load, + .fields = (VMStateField[]) { + VMSTATE_BOOL(enabled, GICState), + VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, NCPU), + VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1, + vmstate_gic_irq_state, gic_irq_state), + VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ), + VMSTATE_UINT8_2DARRAY(priority1, GICState, GIC_INTERNAL, NCPU), + VMSTATE_UINT8_ARRAY(priority2, GICState, GIC_MAXIRQ - GIC_INTERNAL), + VMSTATE_UINT16_2DARRAY(last_active, GICState, GIC_MAXIRQ, NCPU), + VMSTATE_UINT16_ARRAY(priority_mask, GICState, NCPU), + VMSTATE_UINT16_ARRAY(running_irq, GICState, NCPU), + VMSTATE_UINT16_ARRAY(running_priority, GICState, NCPU), + VMSTATE_UINT16_ARRAY(current_pending, GICState, NCPU), + VMSTATE_END_OF_LIST() + } +}; + static void arm_gic_common_realize(DeviceState *dev, Error **errp) { GICState *s = ARM_GIC_COMMON(dev); @@ -131,8 +106,6 @@ static void arm_gic_common_realize(DeviceState *dev, Error **errp) num_irq); return; } - - register_savevm(NULL, "arm_gic", -1, 3, gic_save, gic_load, s); } static void arm_gic_common_reset(DeviceState *dev) @@ -182,6 +155,7 @@ static void arm_gic_common_class_init(ObjectClass *klass, void *data) dc->reset = arm_gic_common_reset; dc->realize = arm_gic_common_realize; dc->props = arm_gic_common_properties; + dc->vmsd = &vmstate_gic; dc->no_user = 1; } -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH v3 3/3] hw/arm_gic_common: Use vmstate struct rather than save/load functions 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 3/3] hw/arm_gic_common: Use vmstate struct rather than save/load functions Peter Maydell @ 2013-03-22 19:35 ` Igor Mitsyanko 0 siblings, 0 replies; 6+ messages in thread From: Igor Mitsyanko @ 2013-03-22 19:35 UTC (permalink / raw) To: Peter Maydell; +Cc: qemu-devel, Gerd Hoffmann, Andreas, patches [-- Attachment #1: Type: text/plain, Size: 6277 bytes --] On Mar 22, 2013 10:02 PM, "Peter Maydell" <peter.maydell@linaro.org> wrote: > > Update the GIC save/restore to use vmstate rather than hand-rolled > save/load functions. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > hw/arm_gic_common.c | 108 +++++++++++++++++++-------------------------------- > 1 file changed, 41 insertions(+), 67 deletions(-) > > diff --git a/hw/arm_gic_common.c b/hw/arm_gic_common.c > index f95bec3..71594f1 100644 > --- a/hw/arm_gic_common.c > +++ b/hw/arm_gic_common.c > @@ -20,90 +20,65 @@ > > #include "hw/arm_gic_internal.h" > > -static void gic_save(QEMUFile *f, void *opaque) > +static void gic_pre_save(void *opaque) > { > GICState *s = (GICState *)opaque; > ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s); > - int i; > - int j; > > if (c->pre_save) { > c->pre_save(s); > } > - > - qemu_put_be32(f, s->enabled); > - for (i = 0; i < s->num_cpu; i++) { > - qemu_put_be32(f, s->cpu_enabled[i]); > - for (j = 0; j < GIC_INTERNAL; j++) { > - qemu_put_be32(f, s->priority1[j][i]); > - } > - for (j = 0; j < s->num_irq; j++) { > - qemu_put_be32(f, s->last_active[j][i]); > - } > - qemu_put_be32(f, s->priority_mask[i]); > - qemu_put_be32(f, s->running_irq[i]); > - qemu_put_be32(f, s->running_priority[i]); > - qemu_put_be32(f, s->current_pending[i]); > - } > - for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) { > - qemu_put_be32(f, s->priority2[i]); > - } > - for (i = 0; i < s->num_irq; i++) { > - qemu_put_be32(f, s->irq_target[i]); > - qemu_put_byte(f, s->irq_state[i].enabled); > - qemu_put_byte(f, s->irq_state[i].pending); > - qemu_put_byte(f, s->irq_state[i].active); > - qemu_put_byte(f, s->irq_state[i].level); > - qemu_put_byte(f, s->irq_state[i].model); > - qemu_put_byte(f, s->irq_state[i].trigger); > - } > } > > -static int gic_load(QEMUFile *f, void *opaque, int version_id) > +static int gic_post_load(void *opaque, int version_id) > { > GICState *s = (GICState *)opaque; > ARMGICCommonClass *c = ARM_GIC_COMMON_GET_CLASS(s); > - int i; > - int j; > - > - if (version_id != 3) { > - return -EINVAL; > - } > - > - s->enabled = qemu_get_be32(f); > - for (i = 0; i < s->num_cpu; i++) { > - s->cpu_enabled[i] = qemu_get_be32(f); > - for (j = 0; j < GIC_INTERNAL; j++) { > - s->priority1[j][i] = qemu_get_be32(f); > - } > - for (j = 0; j < s->num_irq; j++) { > - s->last_active[j][i] = qemu_get_be32(f); > - } > - s->priority_mask[i] = qemu_get_be32(f); > - s->running_irq[i] = qemu_get_be32(f); > - s->running_priority[i] = qemu_get_be32(f); > - s->current_pending[i] = qemu_get_be32(f); > - } > - for (i = 0; i < s->num_irq - GIC_INTERNAL; i++) { > - s->priority2[i] = qemu_get_be32(f); > - } > - for (i = 0; i < s->num_irq; i++) { > - s->irq_target[i] = qemu_get_be32(f); > - s->irq_state[i].enabled = qemu_get_byte(f); > - s->irq_state[i].pending = qemu_get_byte(f); > - s->irq_state[i].active = qemu_get_byte(f); > - s->irq_state[i].level = qemu_get_byte(f); > - s->irq_state[i].model = qemu_get_byte(f); > - s->irq_state[i].trigger = qemu_get_byte(f); > - } > > if (c->post_load) { > c->post_load(s); > } > - > return 0; > } > > +static const VMStateDescription vmstate_gic_irq_state = { > + .name = "arm_gic_irq_state", > + .version_id = 1, > + .minimum_version_id = 1, > + .fields = (VMStateField[]) { > + VMSTATE_UINT8(enabled, gic_irq_state), > + VMSTATE_UINT8(pending, gic_irq_state), > + VMSTATE_UINT8(active, gic_irq_state), > + VMSTATE_UINT8(level, gic_irq_state), > + VMSTATE_BOOL(model, gic_irq_state), > + VMSTATE_BOOL(trigger, gic_irq_state), > + VMSTATE_END_OF_LIST() > + } > +}; > + > +static const VMStateDescription vmstate_gic = { > + .name = "arm_gic", > + .version_id = 4, > + .minimum_version_id = 4, > + .pre_save = gic_pre_save, > + .post_load = gic_post_load, I've just found out that if.minimum_version_id_old is not set and you`re trying to load an older versioned VM snapshot, vmstate_load_state will call load_state_old callback. And this will cause segfault because its not initialized in gic VMstateDescription. Its not mandatory to initialize it, many devices in QEMU dont set minimum_version_id_old, so I guess its a savevm.c bug? Reviewed-by: Igor Mitsyanko <i.mitsyanko@gmail.com> > + .fields = (VMStateField[]) { > + VMSTATE_BOOL(enabled, GICState), > + VMSTATE_BOOL_ARRAY(cpu_enabled, GICState, NCPU), > + VMSTATE_STRUCT_ARRAY(irq_state, GICState, GIC_MAXIRQ, 1, > + vmstate_gic_irq_state, gic_irq_state), > + VMSTATE_UINT8_ARRAY(irq_target, GICState, GIC_MAXIRQ), > + VMSTATE_UINT8_2DARRAY(priority1, GICState, GIC_INTERNAL, NCPU), > + VMSTATE_UINT8_ARRAY(priority2, GICState, GIC_MAXIRQ - GIC_INTERNAL), > + VMSTATE_UINT16_2DARRAY(last_active, GICState, GIC_MAXIRQ, NCPU), > + VMSTATE_UINT16_ARRAY(priority_mask, GICState, NCPU), > + VMSTATE_UINT16_ARRAY(running_irq, GICState, NCPU), > + VMSTATE_UINT16_ARRAY(running_priority, GICState, NCPU), > + VMSTATE_UINT16_ARRAY(current_pending, GICState, NCPU), > + VMSTATE_END_OF_LIST() > + } > +}; > + > static void arm_gic_common_realize(DeviceState *dev, Error **errp) > { > GICState *s = ARM_GIC_COMMON(dev); > @@ -131,8 +106,6 @@ static void arm_gic_common_realize(DeviceState *dev, Error **errp) > num_irq); > return; > } > - > - register_savevm(NULL, "arm_gic", -1, 3, gic_save, gic_load, s); > } > > static void arm_gic_common_reset(DeviceState *dev) > @@ -182,6 +155,7 @@ static void arm_gic_common_class_init(ObjectClass *klass, void *data) > dc->reset = arm_gic_common_reset; > dc->realize = arm_gic_common_realize; > dc->props = arm_gic_common_properties; > + dc->vmsd = &vmstate_gic; > dc->no_user = 1; > } > > -- > 1.7.9.5 > [-- Attachment #2: Type: text/html, Size: 8042 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-03-22 19:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-22 18:02 [Qemu-devel] [PATCH v3 0/3] arm_gic: convert to vmstate Peter Maydell 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 1/3] vmstate: Add support for two dimensional arrays Peter Maydell 2013-03-22 19:20 ` Igor Mitsyanko 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 2/3] arm_gic: Fix sizes of state fields in preparation for vmstate support Peter Maydell 2013-03-22 18:02 ` [Qemu-devel] [PATCH v3 3/3] hw/arm_gic_common: Use vmstate struct rather than save/load functions Peter Maydell 2013-03-22 19:35 ` Igor Mitsyanko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).