From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:46983) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNibk-0005ru-1n for qemu-devel@nongnu.org; Tue, 08 Nov 2011 05:08:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RNibi-0004Gn-Ck for qemu-devel@nongnu.org; Tue, 08 Nov 2011 05:08:44 -0500 Received: from mail-fx0-f45.google.com ([209.85.161.45]:35092) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNibi-0004GO-4E for qemu-devel@nongnu.org; Tue, 08 Nov 2011 05:08:42 -0500 Received: by faap15 with SMTP id p15so422901faa.4 for ; Tue, 08 Nov 2011 02:08:40 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <4EB8CD52.1000008@redhat.com> References: <1319540983-4248-1-git-send-email-benoit.canet@gmail.com> <1319540983-4248-5-git-send-email-benoit.canet@gmail.com> <4EB8CD52.1000008@redhat.com> Date: Tue, 8 Nov 2011 11:08:40 +0100 Message-ID: From: =?UTF-8?Q?Beno=C3=AEt_Canet?= Content-Type: multipart/alternative; boundary=f46d042dff2d09508504b1365bed Subject: Re: [Qemu-devel] [PATCH 4/5] integratorcp: convert integratorcm to VMState List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: Peter Maydell , qemu-devel@nongnu.org, quintela@redhat.com --f46d042dff2d09508504b1365bed Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable flash_mapped reflect the bit 2 of a control register. Peter, does this patch look better ? commit 2fa7b11ee2b2532d00056d6bbc928c5162925e1d Author: Beno=C3=AEt Canet Date: Mon Oct 24 14:39:26 2011 +0200 integratorcp: convert integratorcm to VMState Signed-off-by: Benoit Canet diff --git a/hw/integratorcp.c b/hw/integratorcp.c index 9a289b4..4e51579 100644 --- a/hw/integratorcp.c +++ b/hw/integratorcp.c @@ -34,6 +34,42 @@ typedef struct { uint32_t fiq_enabled; } integratorcm_state; +static void integratorcm_do_remap(integratorcm_state *s, int flash); + +static int integratorcm_post_load(void *opaque, int version_id) +{ + integratorcm_state *s =3D opaque; + + if (s->cm_ctrl & 4) { + integratorcm_do_remap(s, 0); + } + /* ??? tlb_flush (cpu_single_env, 1); */ + return 0; +} + +static const VMStateDescription vmstate_integratorcm =3D { + .name =3D "integratorcm", + .version_id =3D 1, + .minimum_version_id =3D 1, + .minimum_version_id_old =3D 1, + .post_load =3D integratorcm_post_load, + .fields =3D (VMStateField[]) { + VMSTATE_UINT32(memsz, integratorcm_state), + VMSTATE_UINT32(cm_osc, integratorcm_state), + VMSTATE_UINT32(cm_ctrl, integratorcm_state), + VMSTATE_UINT32(cm_lock, integratorcm_state), + VMSTATE_UINT32(cm_auxosc, integratorcm_state), + VMSTATE_UINT32(cm_sdram, integratorcm_state), + VMSTATE_UINT32(cm_init, integratorcm_state), + VMSTATE_UINT32(cm_flags, integratorcm_state), + VMSTATE_UINT32(cm_nvflags, integratorcm_state), + VMSTATE_UINT32(int_level, integratorcm_state), + VMSTATE_UINT32(irq_enabled, integratorcm_state), + VMSTATE_UINT32(fiq_enabled, integratorcm_state), + VMSTATE_END_OF_LIST() + } +}; + static uint8_t integrator_spd[128] =3D { 128, 8, 4, 11, 9, 1, 64, 0, 2, 0xa0, 0xa0, 0, 0, 8, 0, 1, 0xe, 4, 0x1c, 1, 2, 0x20, 0xc0, 0, 0, 0, 0, 0x30, 0x28, 0x30, 0x28, 0x4= 0 @@ -547,6 +583,7 @@ static SysBusDeviceInfo core_info =3D { .init =3D integratorcm_init, .qdev.name =3D "integrator_core", .qdev.size =3D sizeof(integratorcm_state), + .qdev.vmsd =3D &vmstate_integratorcm, .qdev.props =3D (Property[]) { DEFINE_PROP_UINT32("memsz", integratorcm_state, memsz, 0), DEFINE_PROP_END_OF_LIST(), On Tue, Nov 8, 2011 at 7:33 AM, Avi Kivity wrote: > On 11/08/2011 04:07 AM, Peter Maydell wrote: > > 2011/10/26 Peter Maydell : > > > On 25 October 2011 12:09, Beno=C3=AEt Canet = wrote: > > >> +static const VMStateDescription vmstate_integratorcm =3D { > > >> + .name =3D "integratorcm", > > >> + .version_id =3D 1, > > >> + .minimum_version_id =3D 1, > > >> + .minimum_version_id_old =3D 1, > > >> + .fields =3D (VMStateField[]) { > > >> + VMSTATE_UINT32(memsz, integratorcm_state), > > >> + VMSTATE_BOOL(flash_mapped, integratorcm_state), > > > > > > This raises a question. flash_mapped here is a flag which just > > > tracks whether the associated MemoryRegion is currently mapped > > > or unmapped. Do we need to do anything special to ensure that > > > the MemoryRegion itself is reset to the correct mapped/unmapped > > > state on restore? > > > > > > I recall discussing this kind of thing with Avi on IRC but I > > > can't remember what the conclusion was. > > > > Avi, ping? I'm still interested in the answer to this question. > > Sorry, missed this. Yes, you need to ensure the memory region mapping > reflects flash_mapped. > > btw, is flash_mapped a real device state (corresponds to a real memory > bit on the device) or just an artefact? It's usually a bad idea to cast > implementation artefacts in vmstate concrete. > > -- > I have a truly marvellous patch that fixes the bug which this > signature is too narrow to contain. > > --f46d042dff2d09508504b1365bed Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

flash_mapped reflect the bit 2 of a control register.

Peter, does this patch look better ?

=
commit 2fa7b11ee2b2532d00056d6bbc928c5162925e1d
Author: Beno=C3=AEt Canet <ben= oit.canet@gmail.com>
Date: =C2=A0 Mon Oct 24 14:39:26 2011= +0200

=C2=A0 =C2=A0 integratorcp: convert integra= torcm to VMState
=C2=A0 =C2=A0=C2=A0
=C2=A0 =C2=A0 Signed-off-by: Benoit Cane= t <benoit.canet@gmail.com&= gt;

diff --git a/hw/integratorcp.c b/hw/integrator= cp.c
index 9a289b4..4e51579 100644
--- a/hw/integratorcp.c
+++ b/hw/integratorcp.c
@@= -34,6 +34,42 @@ typedef struct {
=C2=A0 =C2=A0 =C2=A0uint32_t fi= q_enabled;
=C2=A0} integratorcm_state;
=C2=A0
+static void integratorcm_do_remap(integratorcm_state *s, int flash);
+
+static int integratorcm_post_load(void *opaque, int versi= on_id)
+{
+ =C2=A0 =C2=A0integratorcm_state *s =3D opaq= ue;
+
+ =C2=A0 =C2=A0if (s->cm_ctrl & 4) {
=
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0integratorcm_do_remap(s, 0);
+ =C2=A0 =C2=A0}
+ =C2=A0 =C2=A0/* ??? tlb_flush (cpu_single= _env, 1); */
+ =C2=A0 =C2=A0return 0;
+}
+
+static const VMStateDescription vmstate_integratorcm =3D {
+ =C2=A0 =C2=A0.name =3D "integratorcm",
+ =C2=A0 =C2=A0.version_id =3D 1,
+ =C2=A0 =C2=A0.minimum_ve= rsion_id =3D 1,
+ =C2=A0 =C2=A0.minimum_version_id_old =3D 1,
+ =C2=A0 =C2=A0.post_load =3D integratorcm_post_load,
+ = =C2=A0 =C2=A0.fields =3D (VMStateField[]) {
+ =C2=A0 =C2=A0 =C2= =A0 =C2=A0VMSTATE_UINT32(memsz, integratorcm_state),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(cm_osc, integratorcm_state= ),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(cm_ctrl, integrato= rcm_state),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(cm_lock, = integratorcm_state),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(= cm_auxosc, integratorcm_state),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(cm_sdram, integratorcm_sta= te),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(cm_init, integra= torcm_state),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(cm_flag= s, integratorcm_state),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT= 32(cm_nvflags, integratorcm_state),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(int_level, integratorcm_st= ate),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(irq_enabled, in= tegratorcm_state),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(fi= q_enabled, integratorcm_state),
+ =C2=A0 =C2=A0 =C2=A0 =C2=A0VMST= ATE_END_OF_LIST()
+ =C2=A0 =C2=A0}
+};
+
=C2=A0static uint= 8_t integrator_spd[128] =3D {
=C2=A0 =C2=A0 128, 8, 4, 11, 9, 1, = 64, 0, =C2=A02, 0xa0, 0xa0, 0, 0, 8, 0, 1,
=C2=A0 =C2=A0 0xe, 4, = 0x1c, 1, 2, 0x20, 0xc0, 0, 0, 0, 0, 0x30, 0x28, 0x30, 0x28, 0x40
@@ -547,6 +583,7 @@ static SysBusDeviceInfo core_info =3D {
= =C2=A0 =C2=A0 =C2=A0.init =3D integratorcm_init,
=C2=A0 =C2=A0 = =C2=A0.qdev.name =C2=A0=3D "integrato= r_core",
=C2=A0 =C2=A0 =C2=A0.qdev.size =C2=A0=3D sizeof(int= egratorcm_state),
+ =C2=A0 =C2=A0.qdev.vmsd =3D &vmstate_integratorcm,
=C2= =A0 =C2=A0 =C2=A0.qdev.props =3D (Property[]) {
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0DEFINE_PROP_UINT32("memsz", integratorcm_state, = memsz, 0),
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0DEFINE_PROP_END_OF_L= IST(),


On Tue, Nov 8, 2011 at 7:33 A= M, Avi Kivity <avi@r= edhat.com> wrote:
On 11/08/2011 04:07 AM, Peter Mayde= ll wrote:
> 2011/10/26 Peter Maydell <peter.maydell@linaro.org>:
> > On 25 October 2011 12:09, Beno=C3=AEt Canet <benoit.canet@gmail.com> wrote:
> >> +static const VMStateDescription vmstate_integratorcm =3D { > >> + =C2=A0 =C2=A0.name =3D "integratorcm",
> >> + =C2=A0 =C2=A0.version_id =3D 1,
> >> + =C2=A0 =C2=A0.minimum_version_id =3D 1,
> >> + =C2=A0 =C2=A0.minimum_version_id_old =3D 1,
> >> + =C2=A0 =C2=A0.fields =3D (VMStateField[]) {
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_UINT32(memsz, integrator= cm_state),
> >> + =C2=A0 =C2=A0 =C2=A0 =C2=A0VMSTATE_BOOL(flash_mapped, integ= ratorcm_state),
> >
> > This raises a question. flash_mapped here is a flag which just > > tracks whether the associated MemoryRegion is currently mapped > > or unmapped. Do we need to do anything special to ensure that
> > the MemoryRegion itself is reset to the correct mapped/unmapped > > state on restore?
> >
> > I recall discussing this kind of thing with Avi on IRC but I
> > can't remember what the conclusion was.
>
> Avi, ping? I'm still interested in the answer to this question.
Sorry, missed this. Yes, you need to ensure the memory region m= apping
reflects flash_mapped.

btw, is flash_mapped a real device state (corresponds to a real memory
bit on the device) or just an artefact? =C2=A0It's usually a bad idea t= o cast
implementation artefacts in vmstate concrete.

--
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.


--f46d042dff2d09508504b1365bed--