From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57535) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RnsG0-0001Y1-NL for qemu-devel@nongnu.org; Thu, 19 Jan 2012 08:42:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RnsFy-0006h5-UE for qemu-devel@nongnu.org; Thu, 19 Jan 2012 08:42:24 -0500 Received: from cantor2.suse.de ([195.135.220.15]:34940 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RnsFy-0006h1-71 for qemu-devel@nongnu.org; Thu, 19 Jan 2012 08:42:22 -0500 Message-ID: <4F181D47.4050402@suse.de> Date: Thu, 19 Jan 2012 14:40:23 +0100 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1324893824-13558-1-git-send-email-i.mitsyanko@samsung.com> <1325086342-25653-1-git-send-email-i.mitsyanko@samsung.com> <1325086342-25653-2-git-send-email-i.mitsyanko@samsung.com> In-Reply-To: <1325086342-25653-2-git-send-email-i.mitsyanko@samsung.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH V3 1/5] vmstate: introduce get_bufsize entry in VMStateField List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: quintela@redhat.com Cc: peter.maydell@linaro.org, Mitsyanko Igor , e.voevodin@samsung.com, qemu-devel@nongnu.org, kyungmin.park@samsung.com, d.solodkiy@samsung.com, m.kozlov@samsung.com, jehyung.lee@samsung.com Am 28.12.2011 16:32, schrieb Mitsyanko Igor: > New get_bufsize field in VMStateField is supposed to help us easily add= save/restore > support of dynamically allocated buffers in device's states. > There are some cases when information about size of dynamically allocat= ed buffer is > already presented in specific device's state structure, but in such a f= orm that > can not be used with existing VMStateField interface. Currently, we eit= her can get size from > another variable in device's state as it is with VMSTATE_VBUFFER_* macr= os, or we can > also multiply value kept in a variable by a constant with VMSTATE_BUFFE= R_MULTIPLY > macro. If we need to perform any other action, we're forced to add addi= tional > variable with size information to device state structure with the only = intention > to use it in VMStateDescription. This approach is not very pretty. Addi= ng extra > flags to VMStateFlags enum for every other possible operation with size= field > seems redundant, and still it would't cover cases when we need to perfo= rm a set of > operations to get size value. > With get_bufsize callback we can calculate size of dynamic array in whi= chever > way we need. We don't need .size_offset field anymore, so we can remove= it from > VMState Field structure to compensate for extra memory consuption becau= se of > get_bufsize addition. Macros VMSTATE_VBUFFER* are modified to use new c= allback > instead of .size_offset. Macro VMSTATE_BUFFER_MULTIPLY and VMFlag VMS_M= ULTIPLY > are removed completely as they are now redundant. >=20 > Signed-off-by: Mitsyanko Igor Ping! Juan, what do you think? Andreas > --- > hw/g364fb.c | 7 ++++++- > hw/hw.h | 41 +++++++---------------------------------- > hw/m48t59.c | 7 ++++++- > hw/mac_nvram.c | 8 +++++++- > hw/onenand.c | 7 ++++++- > savevm.c | 10 ++-------- > 6 files changed, 34 insertions(+), 46 deletions(-) >=20 > diff --git a/hw/g364fb.c b/hw/g364fb.c > index 34fb08c..1ab36c2 100644 > --- a/hw/g364fb.c > +++ b/hw/g364fb.c > @@ -495,6 +495,11 @@ static int g364fb_post_load(void *opaque, int vers= ion_id) > return 0; > } > =20 > +static int g364fb_get_vramsize(void *opaque, int version_id) > +{ > + return ((G364State *)opaque)->vram_size; > +} > + > static const VMStateDescription vmstate_g364fb =3D { > .name =3D "g364fb", > .version_id =3D 1, > @@ -502,7 +507,7 @@ static const VMStateDescription vmstate_g364fb =3D = { > .minimum_version_id_old =3D 1, > .post_load =3D g364fb_post_load, > .fields =3D (VMStateField[]) { > - VMSTATE_VBUFFER_UINT32(vram, G364State, 1, NULL, 0, vram_size)= , > + VMSTATE_VBUFFER(vram, G364State, 1, NULL, 0, g364fb_get_vramsi= ze), > VMSTATE_BUFFER_UNSAFE(color_palette, G364State, 0, 256 * 3), > VMSTATE_BUFFER_UNSAFE(cursor_palette, G364State, 0, 9), > VMSTATE_UINT16_ARRAY(cursor, G364State, 512), > diff --git a/hw/hw.h b/hw/hw.h > index efa04d1..a2a43b6 100644 > --- a/hw/hw.h > +++ b/hw/hw.h > @@ -303,7 +303,6 @@ enum VMStateFlags { > VMS_ARRAY_OF_POINTER =3D 0x040, > VMS_VARRAY_UINT16 =3D 0x080, /* Array with size in uint16_t fi= eld */ > VMS_VBUFFER =3D 0x100, /* Buffer with size in int32_t fi= eld */ > - VMS_MULTIPLY =3D 0x200, /* multiply "size" field by field= _size */ > VMS_VARRAY_UINT8 =3D 0x400, /* Array with size in uint8_t fie= ld*/ > VMS_VARRAY_UINT32 =3D 0x800, /* Array with size in uint32_t fi= eld*/ > }; > @@ -315,12 +314,12 @@ typedef struct { > size_t start; > int num; > size_t num_offset; > - size_t size_offset; > const VMStateInfo *info; > enum VMStateFlags flags; > const VMStateDescription *vmsd; > int version_id; > bool (*field_exists)(void *opaque, int version_id); > + int (*get_bufsize)(void *opaque, int version_id); > } VMStateField; > =20 > typedef struct VMStateSubsection { > @@ -584,34 +583,11 @@ extern const VMStateInfo vmstate_info_unused_buff= er; > .offset =3D vmstate_offset_buffer(_state, _field) + _start, = \ > } > =20 > -#define VMSTATE_BUFFER_MULTIPLY(_field, _state, _version, _test, _star= t, _field_size, _multiply) { \ > +#define VMSTATE_VBUFFER(_field, _state, _version, _test, _start, _get_= bufsize) { \ > .name =3D (stringify(_field)), = \ > .version_id =3D (_version), = \ > .field_exists =3D (_test), = \ > - .size_offset =3D vmstate_offset_value(_state, _field_size, uint32= _t),\ > - .size =3D (_multiply), = \ > - .info =3D &vmstate_info_buffer, = \ > - .flags =3D VMS_VBUFFER|VMS_MULTIPLY, = \ > - .offset =3D offsetof(_state, _field), = \ > - .start =3D (_start), = \ > -} > - > -#define VMSTATE_VBUFFER(_field, _state, _version, _test, _start, _fiel= d_size) { \ > - .name =3D (stringify(_field)), = \ > - .version_id =3D (_version), = \ > - .field_exists =3D (_test), = \ > - .size_offset =3D vmstate_offset_value(_state, _field_size, int32_= t),\ > - .info =3D &vmstate_info_buffer, = \ > - .flags =3D VMS_VBUFFER|VMS_POINTER, = \ > - .offset =3D offsetof(_state, _field), = \ > - .start =3D (_start), = \ > -} > - > -#define VMSTATE_VBUFFER_UINT32(_field, _state, _version, _test, _start= , _field_size) { \ > - .name =3D (stringify(_field)), = \ > - .version_id =3D (_version), = \ > - .field_exists =3D (_test), = \ > - .size_offset =3D vmstate_offset_value(_state, _field_size, uint32= _t),\ > + .get_bufsize =3D (_get_bufsize), = \ > .info =3D &vmstate_info_buffer, = \ > .flags =3D VMS_VBUFFER|VMS_POINTER, = \ > .offset =3D offsetof(_state, _field), = \ > @@ -891,14 +867,11 @@ extern const VMStateDescription vmstate_hid_ptr_d= evice; > #define VMSTATE_BUFFER_START_MIDDLE(_f, _s, _start) \ > VMSTATE_STATIC_BUFFER(_f, _s, 0, NULL, _start, sizeof(typeof_field= (_s, _f))) > =20 > -#define VMSTATE_PARTIAL_VBUFFER(_f, _s, _size) = \ > - VMSTATE_VBUFFER(_f, _s, 0, NULL, 0, _size) > - > -#define VMSTATE_PARTIAL_VBUFFER_UINT32(_f, _s, _size) = \ > - VMSTATE_VBUFFER_UINT32(_f, _s, 0, NULL, 0, _size) > +#define VMSTATE_PARTIAL_VBUFFER(_f, _s, _get_bufsize) = \ > + VMSTATE_VBUFFER(_f, _s, 0, NULL, 0, _get_bufsize) > =20 > -#define VMSTATE_SUB_VBUFFER(_f, _s, _start, _size) = \ > - VMSTATE_VBUFFER(_f, _s, 0, NULL, _start, _size) > +#define VMSTATE_SUB_VBUFFER(_f, _s, _start, _get_bufsize) = \ > + VMSTATE_VBUFFER(_f, _s, 0, NULL, _start, _get_bufsize) > =20 > #define VMSTATE_BUFFER_TEST(_f, _s, _test) = \ > VMSTATE_STATIC_BUFFER(_f, _s, 0, _test, 0, sizeof(typeof_field(_s,= _f))) > diff --git a/hw/m48t59.c b/hw/m48t59.c > index c043996..4e4c9f3 100644 > --- a/hw/m48t59.c > +++ b/hw/m48t59.c > @@ -582,6 +582,11 @@ static const MemoryRegionOps nvram_ops =3D { > .endianness =3D DEVICE_NATIVE_ENDIAN, > }; > =20 > +static int m48t59_get_bufsize(void *opaque, int version_id) > +{ > + return ((M48t59State *)opaque)->size; > +} > + > static const VMStateDescription vmstate_m48t59 =3D { > .name =3D "m48t59", > .version_id =3D 1, > @@ -590,7 +595,7 @@ static const VMStateDescription vmstate_m48t59 =3D = { > .fields =3D (VMStateField[]) { > VMSTATE_UINT8(lock, M48t59State), > VMSTATE_UINT16(addr, M48t59State), > - VMSTATE_VBUFFER_UINT32(buffer, M48t59State, 0, NULL, 0, size), > + VMSTATE_VBUFFER(buffer, M48t59State, 0, NULL, 0, m48t59_get_bu= fsize), > VMSTATE_END_OF_LIST() > } > }; > diff --git a/hw/mac_nvram.c b/hw/mac_nvram.c > index ed0a2b7..f4367f8 100644 > --- a/hw/mac_nvram.c > +++ b/hw/mac_nvram.c > @@ -100,13 +100,19 @@ static const MemoryRegionOps macio_nvram_ops =3D = { > .endianness =3D DEVICE_NATIVE_ENDIAN, > }; > =20 > +static int macio_nvram_get_datasize(void *opaque, int version_id) > +{ > + return ((MacIONVRAMState *)opaque)->size; > +} > + > static const VMStateDescription vmstate_macio_nvram =3D { > .name =3D "macio_nvram", > .version_id =3D 1, > .minimum_version_id =3D 1, > .minimum_version_id_old =3D 1, > .fields =3D (VMStateField[]) { > - VMSTATE_VBUFFER_UINT32(data, MacIONVRAMState, 0, NULL, 0, size= ), > + VMSTATE_VBUFFER(data, MacIONVRAMState, 0, NULL, 0, > + macio_nvram_get_datasize), > VMSTATE_END_OF_LIST() > } > }; > diff --git a/hw/onenand.c b/hw/onenand.c > index a9d8d67..3e2016b 100644 > --- a/hw/onenand.c > +++ b/hw/onenand.c > @@ -160,6 +160,11 @@ static int onenand_post_load(void *opaque, int ver= sion_id) > return 0; > } > =20 > +static int onenand_get_blockwpsize(void *opaque, int version_id) > +{ > + return ((OneNANDState *)opaque)->blocks; > +} > + > static const VMStateDescription vmstate_onenand =3D { > .name =3D "onenand", > .version_id =3D 1, > @@ -181,7 +186,7 @@ static const VMStateDescription vmstate_onenand =3D= { > VMSTATE_UINT16(intstatus, OneNANDState), > VMSTATE_UINT16(wpstatus, OneNANDState), > VMSTATE_INT32(secs_cur, OneNANDState), > - VMSTATE_PARTIAL_VBUFFER(blockwp, OneNANDState, blocks), > + VMSTATE_PARTIAL_VBUFFER(blockwp, OneNANDState, onenand_get_blo= ckwpsize), > VMSTATE_UINT8(ecc.cp, OneNANDState), > VMSTATE_UINT16_ARRAY(ecc.lp, OneNANDState, 2), > VMSTATE_UINT16(ecc.count, OneNANDState), > diff --git a/savevm.c b/savevm.c > index f153c25..831c50a 100644 > --- a/savevm.c > +++ b/savevm.c > @@ -1412,10 +1412,7 @@ int vmstate_load_state(QEMUFile *f, const VMStat= eDescription *vmsd, > int size =3D field->size; > =20 > if (field->flags & VMS_VBUFFER) { > - size =3D *(int32_t *)(opaque+field->size_offset); > - if (field->flags & VMS_MULTIPLY) { > - size *=3D field->size; > - } > + size =3D field->get_bufsize(opaque, version_id); > } > if (field->flags & VMS_ARRAY) { > n_elems =3D field->num; > @@ -1476,10 +1473,7 @@ void vmstate_save_state(QEMUFile *f, const VMSta= teDescription *vmsd, > int size =3D field->size; > =20 > if (field->flags & VMS_VBUFFER) { > - size =3D *(int32_t *)(opaque+field->size_offset); > - if (field->flags & VMS_MULTIPLY) { > - size *=3D field->size; > - } > + size =3D field->get_bufsize(opaque, vmsd->version_id); > } > if (field->flags & VMS_ARRAY) { > n_elems =3D field->num; --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg