[Qemu-devel] [PATCH 00/20] Port audio to vmstate

[Qemu-devel] [PATCH 00/20] Port audio to vmstate

[Juan Quintela]

Hi

THis patch series port audio to vmstate.
- fix compliation with DEBUG_PLIVE and DEBUG_AC97
- unfold IO_READ_PROTO/IO_WRITE_PROTO and remove them
- audio.c: the easiest thing to port (nothing inside)
- sb16: port to vmstate, testing shows that it didn't survive migrations
  very well (it happened before already).  Guest got this messages:
   I got lots of underruns on the guest reported by alsa
  on monitor I get lots of:
   sb16: warning: command 0x42,2 is not truly understood yet
- es1370: the best working with migration.
- adlib: I am not able to get sound out of it on any recent Fedora :(
- cs4231a: it uses irq9, and that don't fly with acpi using that irq on qemu
  I disabled dma_running before loading state.  malc can you take a look here?
  I changed it to be able to use dma_running state field for state.
- gus: no module anymore on Fedora
- ac97: from Anthony sugestion, remove the use of active array, it can be
        recalculated.  All my testing (muted, not muted, ..., shows that
        transmited array is the same one than the recalculated one).
  ac97 don't work always with migration.

How did I test:
- start a linux guest
- inside there play a song (ogg123 foo.ogg)
- in the middle of the song do savevm foo
- later restart qemu with -loadvm foo option

What did I expect?:
- I expected after ending the loadvm to hear the contination of the song.

Actual results:
- es1370: the one working better
- sb16: lots of underruns, very low sound quality.
- ac97: mixed results.  The 1st 5-10 seconds always sound perfect
   Then, between 10 and 30 seconds, sometimes it lots syncs and start playing at
   full speed, basiaclly no sound ouput. No sound after this is ever generated.
   If it is able to generate sound for 30-40 seconds, then sound works correctly.
  I tried to debug this, enabled all the audio DEBUG*, but I didn't find what is
  happening.
  Notice that this happens when I launch from the same savevm image.  Some times
  it works well, some times it stops working at 5 seconds, sometimes it stops
  working at 10 seconds, and so on.
  My theory is that we are not saving all the state that we need, but I have been
  unable to found anything obviosu here.  malc, do you have any suggestion?

This took a lot because the ac97 testing, I thouguht the ac97 problems were due to
my changes.  It just happened that the 1st time that I loaded without my changes
it just worked :(  Problem can be reproduced as easily without any of my changes.
More work needs to be done to be sure that ac97 migrates correctly, but that is
independent of this patch :)

Later, Juan.

Juan Quintela (20):
  audio: fix compilation of DEBUG_PLIVE
  audio: port to vmstate
  sb16: remove IO_READ_PROTO
  sb16: remove IO_WRITE_PROTO
  sb16: port to vmstate
  es1370: remove IO_READ_PROTO
  es1370: remove IO_WRITE_PROTO
  es1370: port to vmstate
  adlib: remove IO_READ_PROTO
  adlib: remove IO_WRITE_PROTO
  c4231a: remove IO_READ_PROTO
  c4231a: remove IO_WRITE_PROTO
  c4231a: port to vmstate
  gus: remove IO_READ_PROTO
  gus: remove IO_WRITE_PROTO
  gus: port to vmstate
  ac97: sizeof returns unsigned long
  ac97: recalculate active after loadvm
  ac97: up savevm version and remove active from state
  ac97: port to vmstate

 audio/audio.c          |   26 +++-----
 audio/audio_template.h |    6 +-
 hw/ac97.c              |  113 +++++++++++++-----------------
 hw/adlib.c             |    9 +--
 hw/cs4231a.c           |   68 ++++++++---------
 hw/es1370.c            |   94 +++++++++++--------------
 hw/gus.c               |   60 ++++++----------
 hw/sb16.c              |  185 +++++++++++++++++------------------------------
 8 files changed, 225 insertions(+), 336 deletions(-)

[Qemu-devel] [PATCH 01/20] audio: fix compilation of DEBUG_PLIVE

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 audio/audio_template.h |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/audio/audio_template.h b/audio/audio_template.h
index 14c648c..19cdb7f 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -445,9 +445,9 @@ SW *glue (AUD_open_, TYPE) (
                SW_NAME (sw), sw->info.freq, sw->info.bits, sw->info.nchannels);
         dolog ("New %s freq %d, bits %d, channels %d\n",
                name,
-               freq,
-               (fmt == AUD_FMT_S16 || fmt == AUD_FMT_U16) ? 16 : 8,
-               nchannels);
+               as->freq,
+               (as->fmt == AUD_FMT_S16 || as->fmt == AUD_FMT_U16) ? 16 : 8,
+               as->nchannels);
 #endif

         if (live) {
-- 
1.6.2.5

[Qemu-devel] [PATCH 02/20] audio: port to vmstate

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 audio/audio.c |   26 +++++++++-----------------
 1 files changed, 9 insertions(+), 17 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 80a717b..a5305c4 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1784,23 +1784,15 @@ static void audio_atexit (void)
     }
 }

-static void audio_save (QEMUFile *f, void *opaque)
-{
-    (void) f;
-    (void) opaque;
-}
-
-static int audio_load (QEMUFile *f, void *opaque, int version_id)
-{
-    (void) f;
-    (void) opaque;
-
-    if (version_id != 1) {
-        return -EINVAL;
+static const VMStateDescription vmstate_audio = {
+    .name = "audio",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_END_OF_LIST()
     }
-
-    return 0;
-}
+};

 static void audio_init (void)
 {
@@ -1900,7 +1892,7 @@ static void audio_init (void)
     }

     QLIST_INIT (&s->card_head);
-    register_savevm ("audio", 0, 1, audio_save, audio_load, s);
+    vmstate_register (0, &vmstate_audio, s);
 }

 void AUD_register_card (const char *name, QEMUSoundCard *card)
-- 
1.6.2.5

[Qemu-devel] [PATCH 03/20] sb16: remove IO_READ_PROTO

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/sb16.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/sb16.c b/hw/sb16.c
index 8654b7d..f4f96d1 100644
--- a/hw/sb16.c
+++ b/hw/sb16.c
@@ -40,8 +40,6 @@
 #define ldebug(...)
 #endif

-#define IO_READ_PROTO(name)                             \
-    uint32_t name (void *opaque, uint32_t nport)
 #define IO_WRITE_PROTO(name)                                    \
     void name (void *opaque, uint32_t nport, uint32_t val)

@@ -961,7 +959,7 @@ static IO_WRITE_PROTO (dsp_write)
     }
 }

-static IO_READ_PROTO (dsp_read)
+static uint32_t dsp_read (void *opaque, uint32_t nport)
 {
     SB16State *s = opaque;
     int iport, retval, ack = 0;
@@ -1129,7 +1127,7 @@ static IO_WRITE_PROTO (mixer_write_indexw)
     mixer_write_datab (opaque, nport, (val >> 8) & 0xff);
 }

-static IO_READ_PROTO (mixer_read)
+static uint32_t mixer_read (void *opaque, uint32_t nport)
 {
     SB16State *s = opaque;

-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 03/20] sb16: remove IO_READ_PROTO

[malc]

On Thu, 22 Oct 2009, Juan Quintela wrote:

No.

[..snip..]

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH 03/20] sb16: remove IO_READ_PROTO

[Anthony Liguori]

malc wrote:
> On Thu, 22 Oct 2009, Juan Quintela wrote:
>
> No.
>   

I was going to say, "Because..."

But seeing as the commit message was completely empty, "No." is a pretty 
reasonable response.

Juan, can you explain the justification for removing this a bit more?

Regards,

Anthony Liguori

[Qemu-devel] [PATCH 04/20] sb16: remove IO_WRITE_PROTO

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/sb16.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/hw/sb16.c b/hw/sb16.c
index f4f96d1..e461970 100644
--- a/hw/sb16.c
+++ b/hw/sb16.c
@@ -40,9 +40,6 @@
 #define ldebug(...)
 #endif

-#define IO_WRITE_PROTO(name)                                    \
-    void name (void *opaque, uint32_t nport, uint32_t val)
-
 static const char e3[] = "COPYRIGHT (C) CREATIVE TECHNOLOGY LTD, 1992.";

 typedef struct SB16State {
@@ -874,7 +871,7 @@ static void reset (SB16State *s)
     legacy_reset (s);
 }

-static IO_WRITE_PROTO (dsp_write)
+static void dsp_write (void *opaque, uint32_t nport, uint32_t val)
 {
     SB16State *s = opaque;
     int iport;
@@ -1058,14 +1055,14 @@ static void reset_mixer (SB16State *s)
     }
 }

-static IO_WRITE_PROTO (mixer_write_indexb)
+static void mixer_write_indexb (void *opaque, uint32_t nport, uint32_t val)
 {
     SB16State *s = opaque;
     (void) nport;
     s->mixer_nreg = val;
 }

-static IO_WRITE_PROTO (mixer_write_datab)
+static void mixer_write_datab (void *opaque, uint32_t nport, uint32_t val)
 {
     SB16State *s = opaque;

@@ -1121,7 +1118,7 @@ static IO_WRITE_PROTO (mixer_write_datab)
     s->mixer_regs[s->mixer_nreg] = val;
 }

-static IO_WRITE_PROTO (mixer_write_indexw)
+static void mixer_write_indexw (void *opaque, uint32_t nport, uint32_t val)
 {
     mixer_write_indexb (opaque, nport, val & 0xff);
     mixer_write_datab (opaque, nport, (val >> 8) & 0xff);
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 04/20] sb16: remove IO_WRITE_PROTO

[malc]

On Thu, 22 Oct 2009, Juan Quintela wrote:

No.

[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 05/20] sb16: port to vmstate

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/sb16.c |  168 ++++++++++++++++++++++---------------------------------------
 1 files changed, 61 insertions(+), 107 deletions(-)

diff --git a/hw/sb16.c b/hw/sb16.c
index e461970..d66a6e3 100644
--- a/hw/sb16.c
+++ b/hw/sb16.c
@@ -1246,115 +1246,10 @@ static void SB_audio_callback (void *opaque, int free)
     s->audio_free = free;
 }

-static void SB_save (QEMUFile *f, void *opaque)
+static int sb16_post_load (void *opaque, int version_id)
 {
     SB16State *s = opaque;

-    qemu_put_be32 (f, s->irq);
-    qemu_put_be32 (f, s->dma);
-    qemu_put_be32 (f, s->hdma);
-    qemu_put_be32 (f, s->port);
-    qemu_put_be32 (f, s->ver);
-    qemu_put_be32 (f, s->in_index);
-    qemu_put_be32 (f, s->out_data_len);
-    qemu_put_be32 (f, s->fmt_stereo);
-    qemu_put_be32 (f, s->fmt_signed);
-    qemu_put_be32 (f, s->fmt_bits);
-    qemu_put_be32s (f, &s->fmt);
-    qemu_put_be32 (f, s->dma_auto);
-    qemu_put_be32 (f, s->block_size);
-    qemu_put_be32 (f, s->fifo);
-    qemu_put_be32 (f, s->freq);
-    qemu_put_be32 (f, s->time_const);
-    qemu_put_be32 (f, s->speaker);
-    qemu_put_be32 (f, s->needed_bytes);
-    qemu_put_be32 (f, s->cmd);
-    qemu_put_be32 (f, s->use_hdma);
-    qemu_put_be32 (f, s->highspeed);
-    qemu_put_be32 (f, s->can_write);
-    qemu_put_be32 (f, s->v2x6);
-
-    qemu_put_8s (f, &s->csp_param);
-    qemu_put_8s (f, &s->csp_value);
-    qemu_put_8s (f, &s->csp_mode);
-    qemu_put_8s (f, &s->csp_param);
-    qemu_put_buffer (f, s->csp_regs, 256);
-    qemu_put_8s (f, &s->csp_index);
-    qemu_put_buffer (f, s->csp_reg83, 4);
-    qemu_put_be32 (f, s->csp_reg83r);
-    qemu_put_be32 (f, s->csp_reg83w);
-
-    qemu_put_buffer (f, s->in2_data, sizeof (s->in2_data));
-    qemu_put_buffer (f, s->out_data, sizeof (s->out_data));
-    qemu_put_8s (f, &s->test_reg);
-    qemu_put_8s (f, &s->last_read_byte);
-
-    qemu_put_be32 (f, s->nzero);
-    qemu_put_be32 (f, s->left_till_irq);
-    qemu_put_be32 (f, s->dma_running);
-    qemu_put_be32 (f, s->bytes_per_second);
-    qemu_put_be32 (f, s->align);
-
-    qemu_put_be32 (f, s->mixer_nreg);
-    qemu_put_buffer (f, s->mixer_regs, 256);
-}
-
-static int SB_load (QEMUFile *f, void *opaque, int version_id)
-{
-    SB16State *s = opaque;
-
-    if (version_id != 1) {
-        return -EINVAL;
-    }
-
-    s->irq=qemu_get_be32 (f);
-    s->dma=qemu_get_be32 (f);
-    s->hdma=qemu_get_be32 (f);
-    s->port=qemu_get_be32 (f);
-    s->ver=qemu_get_be32 (f);
-    s->in_index=qemu_get_be32 (f);
-    s->out_data_len=qemu_get_be32 (f);
-    s->fmt_stereo=qemu_get_be32 (f);
-    s->fmt_signed=qemu_get_be32 (f);
-    s->fmt_bits=qemu_get_be32 (f);
-    qemu_get_be32s (f, &s->fmt);
-    s->dma_auto=qemu_get_be32 (f);
-    s->block_size=qemu_get_be32 (f);
-    s->fifo=qemu_get_be32 (f);
-    s->freq=qemu_get_be32 (f);
-    s->time_const=qemu_get_be32 (f);
-    s->speaker=qemu_get_be32 (f);
-    s->needed_bytes=qemu_get_be32 (f);
-    s->cmd=qemu_get_be32 (f);
-    s->use_hdma=qemu_get_be32 (f);
-    s->highspeed=qemu_get_be32 (f);
-    s->can_write=qemu_get_be32 (f);
-    s->v2x6=qemu_get_be32 (f);
-
-    qemu_get_8s (f, &s->csp_param);
-    qemu_get_8s (f, &s->csp_value);
-    qemu_get_8s (f, &s->csp_mode);
-    qemu_get_8s (f, &s->csp_param);
-    qemu_get_buffer (f, s->csp_regs, 256);
-    qemu_get_8s (f, &s->csp_index);
-    qemu_get_buffer (f, s->csp_reg83, 4);
-    s->csp_reg83r=qemu_get_be32 (f);
-    s->csp_reg83w=qemu_get_be32 (f);
-
-    qemu_get_buffer (f, s->in2_data, sizeof (s->in2_data));
-    qemu_get_buffer (f, s->out_data, sizeof (s->out_data));
-    qemu_get_8s (f, &s->test_reg);
-    qemu_get_8s (f, &s->last_read_byte);
-
-    s->nzero=qemu_get_be32 (f);
-    s->left_till_irq=qemu_get_be32 (f);
-    s->dma_running=qemu_get_be32 (f);
-    s->bytes_per_second=qemu_get_be32 (f);
-    s->align=qemu_get_be32 (f);
-
-    s->mixer_nreg=qemu_get_be32 (f);
-    qemu_get_buffer (f, s->mixer_regs, 256);
-
     if (s->voice) {
         AUD_close_out (&s->card, s->voice);
         s->voice = NULL;
@@ -1387,6 +1282,65 @@ static int SB_load (QEMUFile *f, void *opaque, int version_id)
     return 0;
 }

+static const VMStateDescription vmstate_sb16 = {
+    .name = "sb16",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .post_load = sb16_post_load,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT32(irq, SB16State),
+        VMSTATE_UINT32(dma, SB16State),
+        VMSTATE_UINT32(hdma, SB16State),
+        VMSTATE_UINT32(port, SB16State),
+        VMSTATE_UINT32(ver, SB16State),
+        VMSTATE_INT32(in_index, SB16State),
+        VMSTATE_INT32(out_data_len, SB16State),
+        VMSTATE_INT32(fmt_stereo, SB16State),
+        VMSTATE_INT32(fmt_signed, SB16State),
+        VMSTATE_INT32(fmt_bits, SB16State),
+        VMSTATE_UINT32(fmt, SB16State),
+        VMSTATE_INT32(dma_auto, SB16State),
+        VMSTATE_INT32(block_size, SB16State),
+        VMSTATE_INT32(fifo, SB16State),
+        VMSTATE_INT32(freq, SB16State),
+        VMSTATE_INT32(time_const, SB16State),
+        VMSTATE_INT32(speaker, SB16State),
+        VMSTATE_INT32(needed_bytes, SB16State),
+        VMSTATE_INT32(cmd, SB16State),
+        VMSTATE_INT32(use_hdma, SB16State),
+        VMSTATE_INT32(highspeed, SB16State),
+        VMSTATE_INT32(can_write, SB16State),
+        VMSTATE_INT32(v2x6, SB16State),
+
+        VMSTATE_UINT8(csp_param, SB16State),
+        VMSTATE_UINT8(csp_value, SB16State),
+        VMSTATE_UINT8(csp_mode, SB16State),
+        VMSTATE_UINT8(csp_param, SB16State),
+        VMSTATE_BUFFER(csp_regs, SB16State),
+        VMSTATE_UINT8(csp_index, SB16State),
+        VMSTATE_BUFFER(csp_reg83, SB16State),
+        VMSTATE_INT32(csp_reg83r, SB16State),
+        VMSTATE_INT32(csp_reg83w, SB16State),
+
+        VMSTATE_BUFFER(in2_data, SB16State),
+        VMSTATE_BUFFER(out_data, SB16State),
+        VMSTATE_UINT8(test_reg, SB16State),
+        VMSTATE_UINT8(last_read_byte, SB16State),
+
+        VMSTATE_INT32(nzero, SB16State),
+        VMSTATE_INT32(left_till_irq, SB16State),
+        VMSTATE_INT32(dma_running, SB16State),
+        VMSTATE_INT32(bytes_per_second, SB16State),
+        VMSTATE_INT32(align, SB16State),
+
+        VMSTATE_INT32(mixer_nreg, SB16State),
+        VMSTATE_BUFFER(mixer_regs, SB16State),
+
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static int sb16_initfn (ISADevice *dev)
 {
     static const uint8_t dsp_write_ports[] = {0x6, 0xc};
@@ -1429,7 +1383,7 @@ static int sb16_initfn (ISADevice *dev)
     DMA_register_channel (s->dma, SB_read_DMA, s);
     s->can_write = 1;

-    register_savevm ("sb16", 0, 1, SB_save, SB_load, s);
+    vmstate_register (0, &vmstate_sb16, s);
     AUD_register_card ("sb16", &s->card);
     return 0;
 }
-- 
1.6.2.5

[Qemu-devel] [PATCH 06/20] es1370: remove IO_READ_PROTO

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/es1370.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/hw/es1370.c b/hw/es1370.c
index 9071a48..fe98267 100644
--- a/hw/es1370.c
+++ b/hw/es1370.c
@@ -156,8 +156,6 @@ static const unsigned dac1_samplerate[] = { 5512, 11025, 22050, 44100 };
 #define DAC2_CHANNEL 1
 #define ADC_CHANNEL 2

-#define IO_READ_PROTO(n) \
-static uint32_t n (void *opaque, uint32_t addr)
 #define IO_WRITE_PROTO(n) \
 static void n (void *opaque, uint32_t addr, uint32_t val)

@@ -613,7 +611,7 @@ IO_WRITE_PROTO (es1370_writel)
     }
 }

-IO_READ_PROTO (es1370_readb)
+static uint32_t es1370_readb (void *opaque, uint32_t addr)
 {
     ES1370State *s = opaque;
     uint32_t val;
@@ -648,7 +646,7 @@ IO_READ_PROTO (es1370_readb)
     return val;
 }

-IO_READ_PROTO (es1370_readw)
+static uint32_t es1370_readw (void *opaque, uint32_t addr)
 {
     ES1370State *s = opaque;
     struct chan *d = &s->chan[0];
@@ -690,7 +688,7 @@ IO_READ_PROTO (es1370_readw)
     return val;
 }

-IO_READ_PROTO (es1370_readl)
+static uint32_t es1370_readl (void *opaque, uint32_t addr)
 {
     ES1370State *s = opaque;
     uint32_t val;
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 06/20] es1370: remove IO_READ_PROTO

[malc]

On Thu, 22 Oct 2009, Juan Quintela wrote:

No.
[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 07/20] es1370: remove IO_WRITE_PROTO

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/es1370.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/es1370.c b/hw/es1370.c
index fe98267..6f0995a 100644
--- a/hw/es1370.c
+++ b/hw/es1370.c
@@ -156,9 +156,6 @@ static const unsigned dac1_samplerate[] = { 5512, 11025, 22050, 44100 };
 #define DAC2_CHANNEL 1
 #define ADC_CHANNEL 2

-#define IO_WRITE_PROTO(n) \
-static void n (void *opaque, uint32_t addr, uint32_t val)
-
 static void es1370_dac1_callback (void *opaque, int free);
 static void es1370_dac2_callback (void *opaque, int free);
 static void es1370_adc_callback (void *opaque, int avail);
@@ -470,7 +467,7 @@ static inline uint32_t es1370_fixup (ES1370State *s, uint32_t addr)
     return addr;
 }

-IO_WRITE_PROTO (es1370_writeb)
+static void es1370_writeb (void *opaque, uint32_t addr, uint32_t val)
 {
     ES1370State *s = opaque;
     uint32_t shift, mask;
@@ -508,7 +505,7 @@ IO_WRITE_PROTO (es1370_writeb)
     }
 }

-IO_WRITE_PROTO (es1370_writew)
+static void es1370_writew (void *opaque, uint32_t addr, uint32_t val)
 {
     ES1370State *s = opaque;
     addr = es1370_fixup (s, addr);
@@ -545,7 +542,7 @@ IO_WRITE_PROTO (es1370_writew)
     }
 }

-IO_WRITE_PROTO (es1370_writel)
+static void es1370_writel (void *opaque, uint32_t addr, uint32_t val)
 {
     ES1370State *s = opaque;
     struct chan *d = &s->chan[0];
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 07/20] es1370: remove IO_WRITE_PROTO

[malc]

+On Thu, 22 Oct 2009, Juan Quintela wrote:

No.
[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 08/20] es1370: port to vmstate

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/es1370.c |   77 +++++++++++++++++++++++++++-------------------------------
 1 files changed, 36 insertions(+), 41 deletions(-)

diff --git a/hw/es1370.c b/hw/es1370.c
index 6f0995a..532baae 100644
--- a/hw/es1370.c
+++ b/hw/es1370.c
@@ -919,48 +919,28 @@ static void es1370_map (PCIDevice *pci_dev, int region_num,
     register_ioport_read (addr, 0x40, 4, es1370_readl, s);
 }

-static void es1370_save (QEMUFile *f, void *opaque)
-{
-    ES1370State *s = opaque;
-    size_t i;
-
-    pci_device_save (&s->dev, f);
-    for (i = 0; i < NB_CHANNELS; ++i) {
-        struct chan *d = &s->chan[i];
-        qemu_put_be32s (f, &d->shift);
-        qemu_put_be32s (f, &d->leftover);
-        qemu_put_be32s (f, &d->scount);
-        qemu_put_be32s (f, &d->frame_addr);
-        qemu_put_be32s (f, &d->frame_cnt);
+static const VMStateDescription vmstate_es1370_channel = {
+    .name = "es1370_channel",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT32(shift, struct chan),
+        VMSTATE_UINT32(leftover, struct chan),
+        VMSTATE_UINT32(scount, struct chan),
+        VMSTATE_UINT32(frame_addr, struct chan),
+        VMSTATE_UINT32(frame_cnt, struct chan),
+        VMSTATE_END_OF_LIST()
     }
-    qemu_put_be32s (f, &s->ctl);
-    qemu_put_be32s (f, &s->status);
-    qemu_put_be32s (f, &s->mempage);
-    qemu_put_be32s (f, &s->codec);
-    qemu_put_be32s (f, &s->sctl);
-}
+};

-static int es1370_load (QEMUFile *f, void *opaque, int version_id)
+static int es1370_post_load (void *opaque, int version_id)
 {
-    int ret;
     uint32_t ctl, sctl;
     ES1370State *s = opaque;
     size_t i;

-    if (version_id != 2)
-        return -EINVAL;
-
-    ret = pci_device_load (&s->dev, f);
-    if (ret)
-        return ret;
-
     for (i = 0; i < NB_CHANNELS; ++i) {
-        struct chan *d = &s->chan[i];
-        qemu_get_be32s (f, &d->shift);
-        qemu_get_be32s (f, &d->leftover);
-        qemu_get_be32s (f, &d->scount);
-        qemu_get_be32s (f, &d->frame_addr);
-        qemu_get_be32s (f, &d->frame_cnt);
         if (i == ADC_CHANNEL) {
             if (s->adc_voice) {
                 AUD_close_in (&s->card, s->adc_voice);
@@ -975,18 +955,33 @@ static int es1370_load (QEMUFile *f, void *opaque, int version_id)
         }
     }

-    qemu_get_be32s (f, &ctl);
-    qemu_get_be32s (f, &s->status);
-    qemu_get_be32s (f, &s->mempage);
-    qemu_get_be32s (f, &s->codec);
-    qemu_get_be32s (f, &sctl);
-
+    ctl = s->ctl;
+    sctl = s->sctl;
     s->ctl = 0;
     s->sctl = 0;
     es1370_update_voices (s, ctl, sctl);
     return 0;
 }

+static const VMStateDescription vmstate_es1370 = {
+    .name = "es1370",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
+    .post_load = es1370_post_load,
+    .fields      = (VMStateField []) {
+        VMSTATE_PCI_DEVICE(dev, ES1370State),
+        VMSTATE_STRUCT_ARRAY(chan, ES1370State, NB_CHANNELS, 2,
+                             vmstate_es1370_channel, struct chan),
+        VMSTATE_UINT32(ctl, ES1370State),
+        VMSTATE_UINT32(status, ES1370State),
+        VMSTATE_UINT32(mempage, ES1370State),
+        VMSTATE_UINT32(codec, ES1370State),
+        VMSTATE_UINT32(sctl, ES1370State),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void es1370_on_reset (void *opaque)
 {
     ES1370State *s = opaque;
@@ -1023,7 +1018,7 @@ static int es1370_initfn (PCIDevice *dev)
     c[0x3f] = 0x80;

     pci_register_bar (&s->dev, 0, 256, PCI_ADDRESS_SPACE_IO, es1370_map);
-    register_savevm ("es1370", 0, 2, es1370_save, es1370_load, s);
+    vmstate_register (0, &vmstate_es1370, s);
     qemu_register_reset (es1370_on_reset, s);

     AUD_register_card ("es1370", &s->card);
-- 
1.6.2.5

[Qemu-devel] [PATCH 09/20] adlib: remove IO_READ_PROTO

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/adlib.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/hw/adlib.c b/hw/adlib.c
index 1d8092b..3221fea 100644
--- a/hw/adlib.c
+++ b/hw/adlib.c
@@ -51,8 +51,6 @@ void YMF262UpdateOneQEMU (int which, INT16 *dst, int length);
 #define SHIFT 1
 #endif

-#define IO_READ_PROTO(name) \
-    uint32_t name (void *opaque, uint32_t nport)
 #define IO_WRITE_PROTO(name) \
     void name (void *opaque, uint32_t nport, uint32_t val)

@@ -133,7 +131,7 @@ static IO_WRITE_PROTO (adlib_write)
 #endif
 }

-static IO_READ_PROTO (adlib_read)
+static uint32_t adlib_read (void *opaque, uint32_t nport)
 {
     AdlibState *s = opaque;
     uint8_t data;
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 09/20] adlib: remove IO_READ_PROTO

[malc]

On Thu, 22 Oct 2009, Juan Quintela wrote:

No.
[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 10/20] adlib: remove IO_WRITE_PROTO

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/adlib.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/hw/adlib.c b/hw/adlib.c
index 3221fea..fa3c6c0 100644
--- a/hw/adlib.c
+++ b/hw/adlib.c
@@ -51,9 +51,6 @@ void YMF262UpdateOneQEMU (int which, INT16 *dst, int length);
 #define SHIFT 1
 #endif

-#define IO_WRITE_PROTO(name) \
-    void name (void *opaque, uint32_t nport, uint32_t val)
-
 static struct {
     int port;
     int freq;
@@ -113,7 +110,7 @@ static void adlib_kill_timers (AdlibState *s)
     }
 }

-static IO_WRITE_PROTO (adlib_write)
+static void adlib_write (void *opaque, uint32_t nport, uint32_t val)
 {
     AdlibState *s = opaque;
     int a = nport & 3;
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 10/20] adlib: remove IO_WRITE_PROTO

[malc]

On Thu, 22 Oct 2009, Juan Quintela wrote:

No.
[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 11/20] c4231a: remove IO_READ_PROTO

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/cs4231a.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/hw/cs4231a.c b/hw/cs4231a.c
index e03c5d2..8a91b29 100644
--- a/hw/cs4231a.c
+++ b/hw/cs4231a.c
@@ -74,9 +74,6 @@ typedef struct CSState {
     int16_t *tab;
 } CSState;

-#define IO_READ_PROTO(name)                             \
-    static uint32_t name (void *opaque, uint32_t addr)
-
 #define IO_WRITE_PROTO(name)                                            \
     static void name (void *opaque, uint32_t addr, uint32_t val)

@@ -353,7 +350,7 @@ static void cs_reset_voices (CSState *s, uint32_t val)
     }
 }

-IO_READ_PROTO (cs_read)
+static uint32_t cs_read (void *opaque, uint32_t addr)
 {
     CSState *s = opaque;
     uint32_t saddr, iaddr, ret;
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 11/20] c4231a: remove IO_READ_PROTO

[malc]

On Thu, 22 Oct 2009, Juan Quintela wrote:

No.
[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 12/20] c4231a: remove IO_WRITE_PROTO

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/cs4231a.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/hw/cs4231a.c b/hw/cs4231a.c
index 8a91b29..a077edc 100644
--- a/hw/cs4231a.c
+++ b/hw/cs4231a.c
@@ -74,9 +74,6 @@ typedef struct CSState {
     int16_t *tab;
 } CSState;

-#define IO_WRITE_PROTO(name)                                            \
-    static void name (void *opaque, uint32_t addr, uint32_t val)
-
 #define GET_SADDR(addr) (addr & 3)

 #define MODE2 (1 << 6)
@@ -387,7 +384,7 @@ static uint32_t cs_read (void *opaque, uint32_t addr)
     return ret;
 }

-IO_WRITE_PROTO (cs_write)
+static void cs_write (void *opaque, uint32_t addr, uint32_t val)
 {
     CSState *s = opaque;
     uint32_t saddr, iaddr;
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 12/20] c4231a: remove IO_WRITE_PROTO

[malc]

On Thu, 22 Oct 2009, Juan Quintela wrote:

No.
[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 13/20] c4231a: port to vmstate

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/cs4231a.c |   58 ++++++++++++++++++++++++++++++----------------------------
 1 files changed, 30 insertions(+), 28 deletions(-)

diff --git a/hw/cs4231a.c b/hw/cs4231a.c
index a077edc..e9468bc 100644
--- a/hw/cs4231a.c
+++ b/hw/cs4231a.c
@@ -590,45 +590,47 @@ static int cs_dma_read (void *opaque, int nchan, int dma_pos, int dma_len)
     return dma_pos;
 }

-static void cs_save (QEMUFile *f, void *opaque)
+static int cs4231a_pre_load (void *opaque)
 {
     CSState *s = opaque;
-    unsigned int i;
-    uint32_t val;

-    for (i = 0; i < CS_REGS; i++)
-        qemu_put_be32s (f, &s->regs[i]);
-
-    qemu_put_buffer (f, s->dregs, CS_DREGS);
-    val = s->dma_running; qemu_put_be32s (f, &val);
-    val = s->audio_free;  qemu_put_be32s (f, &val);
-    val = s->transferred; qemu_put_be32s (f, &val);
-    val = s->aci_counter; qemu_put_be32s (f, &val);
+    if (s->dma_running) {
+        DMA_release_DREQ (s->dma);
+        AUD_set_active_out (s->voice, 0);
+    }
+    s->dma_running = 0;
+    return 0;
 }

-static int cs_load (QEMUFile *f, void *opaque, int version_id)
+static int cs4231a_post_load (void *opaque, int version_id)
 {
     CSState *s = opaque;
-    unsigned int i;
-    uint32_t val, dma_running;
-
-    if (version_id > 1)
-        return -EINVAL;

-    for (i = 0; i < CS_REGS; i++)
-        qemu_get_be32s (f, &s->regs[i]);
-
-    qemu_get_buffer (f, s->dregs, CS_DREGS);
-
-    qemu_get_be32s (f, &dma_running);
-    qemu_get_be32s (f, &val); s->audio_free  = val;
-    qemu_get_be32s (f, &val); s->transferred = val;
-    qemu_get_be32s (f, &val); s->aci_counter = val;
-    if (dma_running && (s->dregs[Interface_Configuration] & PEN))
+    if (s->dma_running && (s->dregs[Interface_Configuration] & PEN)) {
+        s->dma_running = 0;
         cs_reset_voices (s, s->dregs[FS_And_Playback_Data_Format]);
+    }
     return 0;
 }

+static const VMStateDescription vmstate_cs4231a = {
+    .name = "cs4231a",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .pre_load = cs4231a_pre_load,
+    .post_load = cs4231a_post_load,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT32_ARRAY(regs, CSState, CS_REGS),
+        VMSTATE_BUFFER(dregs, CSState),
+        VMSTATE_INT32(dma_running, CSState),
+        VMSTATE_INT32(audio_free, CSState),
+        VMSTATE_INT32(transferred, CSState),
+        VMSTATE_INT32(aci_counter, CSState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static int cs4231a_initfn (ISADevice *dev)
 {
     CSState *s = DO_UPCAST (CSState, dev, dev);
@@ -643,7 +645,7 @@ static int cs4231a_initfn (ISADevice *dev)

     DMA_register_channel (s->dma, cs_dma_read, s);

-    register_savevm ("cs4231a", 0, 1, cs_save, cs_load, s);
+    vmstate_register (0, &vmstate_cs4231a, s);
     qemu_register_reset (cs_reset, s);
     cs_reset (s);

-- 
1.6.2.5

[Qemu-devel] [PATCH 14/20] gus: remove IO_READ_PROTO

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/gus.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/gus.c b/hw/gus.c
index c6b98b3..101e645 100644
--- a/hw/gus.c
+++ b/hw/gus.c
@@ -41,8 +41,6 @@
 #define GUS_ENDIANNESS 0
 #endif

-#define IO_READ_PROTO(name) \
-    static uint32_t name (void *opaque, uint32_t nport)
 #define IO_WRITE_PROTO(name) \
     static void name (void *opaque, uint32_t nport, uint32_t val)

@@ -61,14 +59,14 @@ typedef struct GUSState {
     qemu_irq pic;
 } GUSState;

-IO_READ_PROTO (gus_readb)
+static uint32_t gus_readb (void *opaque, uint32_t nport)
 {
     GUSState *s = opaque;

     return gus_read (&s->emu, nport, 1);
 }

-IO_READ_PROTO (gus_readw)
+static uint32_t gus_readw (void *opaque, uint32_t nport)
 {
     GUSState *s = opaque;

-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 14/20] gus: remove IO_READ_PROTO

[malc]

On Thu, 22 Oct 2009, Juan Quintela wrote:

No.
[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 15/20] gus: remove IO_WRITE_PROTO

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/gus.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/gus.c b/hw/gus.c
index 101e645..d9c47b1 100644
--- a/hw/gus.c
+++ b/hw/gus.c
@@ -41,9 +41,6 @@
 #define GUS_ENDIANNESS 0
 #endif

-#define IO_WRITE_PROTO(name) \
-    static void name (void *opaque, uint32_t nport, uint32_t val)
-
 typedef struct GUSState {
     ISADevice dev;
     GUSEmuState emu;
@@ -73,14 +70,14 @@ static uint32_t gus_readw (void *opaque, uint32_t nport)
     return gus_read (&s->emu, nport, 2);
 }

-IO_WRITE_PROTO (gus_writeb)
+static void gus_writeb (void *opaque, uint32_t nport, uint32_t val)
 {
     GUSState *s = opaque;

     gus_write (&s->emu, nport, 1, val);
 }

-IO_WRITE_PROTO (gus_writew)
+static void gus_writew (void *opaque, uint32_t nport, uint32_t val)
 {
     GUSState *s = opaque;

-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 15/20] gus: remove IO_WRITE_PROTO

[malc]

On Thu, 22 Oct 2009, Juan Quintela wrote:

No.
[..snip..]

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH 16/20] gus: port to vmstate

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/gus.c |   47 +++++++++++++++++------------------------------
 1 files changed, 17 insertions(+), 30 deletions(-)

diff --git a/hw/gus.c b/hw/gus.c
index d9c47b1..50c2523 100644
--- a/hw/gus.c
+++ b/hw/gus.c
@@ -210,35 +210,22 @@ static int GUS_read_DMA (void *opaque, int nchan, int dma_pos, int dma_len)
     return dma_len;
 }

-static void GUS_save (QEMUFile *f, void *opaque)
-{
-    GUSState *s = opaque;
-
-    qemu_put_be32 (f, s->pos);
-    qemu_put_be32 (f, s->left);
-    qemu_put_be32 (f, s->shift);
-    qemu_put_be32 (f, s->irqs);
-    qemu_put_be32 (f, s->samples);
-    qemu_put_be64 (f, s->last_ticks);
-    qemu_put_buffer (f, s->himem, sizeof (s->himem));
-}
-
-static int GUS_load (QEMUFile *f, void *opaque, int version_id)
-{
-    GUSState *s = opaque;
-
-    if (version_id != 2)
-        return -EINVAL;
-
-    s->pos = qemu_get_be32 (f);
-    s->left = qemu_get_be32 (f);
-    s->shift = qemu_get_be32 (f);
-    s->irqs = qemu_get_be32 (f);
-    s->samples = qemu_get_be32 (f);
-    s->last_ticks = qemu_get_be64 (f);
-    qemu_get_buffer (f, s->himem, sizeof (s->himem));
-    return 0;
-}
+static const VMStateDescription vmstate_gus = {
+    .name = "gus",
+    .version_id = 2,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
+    .fields      = (VMStateField []) {
+        VMSTATE_INT32(pos, GUSState),
+        VMSTATE_INT32(left, GUSState),
+        VMSTATE_INT32(shift, GUSState),
+        VMSTATE_INT32(irqs, GUSState),
+        VMSTATE_INT32(samples, GUSState),
+        VMSTATE_INT64(last_ticks, GUSState),
+        VMSTATE_BUFFER(himem, GUSState),
+        VMSTATE_END_OF_LIST()
+    }
+};

 static int gus_initfn (ISADevice *dev)
 {
@@ -295,7 +282,7 @@ static int gus_initfn (ISADevice *dev)

     AUD_set_active_out (s->voice, 1);

-    register_savevm ("gus", 0, 2, GUS_save, GUS_load, s);
+    vmstate_register (0, &vmstate_gus, s);
     return 0;
 }

-- 
1.6.2.5

[Qemu-devel] [PATCH 17/20] ac97: sizeof returns unsigned long

[Juan Quintela]

This change makes DEBUG_AC97 to compile again

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ac97.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ac97.c b/hw/ac97.c
index 610ca60..e4ecbe9 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -323,7 +323,7 @@ static void reset_bm_regs (AC97LinkState *s, AC97BusMasterRegs *r)
 static void mixer_store (AC97LinkState *s, uint32_t i, uint16_t v)
 {
     if (i + 2 > sizeof (s->mixer_data)) {
-        dolog ("mixer_store: index %d out of bounds %d\n",
+        dolog ("mixer_store: index %d out of bounds %lu\n",
                i, sizeof (s->mixer_data));
         return;
     }
@@ -337,7 +337,7 @@ static uint16_t mixer_load (AC97LinkState *s, uint32_t i)
     uint16_t val = 0xffff;

     if (i + 2 > sizeof (s->mixer_data)) {
-        dolog ("mixer_store: index %d out of bounds %d\n",
+        dolog ("mixer_store: index %d out of bounds %lu\n",
                i, sizeof (s->mixer_data));
     }
     else {
-- 
1.6.2.5

[Qemu-devel] [PATCH 18/20] ac97: recalculate active after loadvm

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ac97.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/ac97.c b/hw/ac97.c
index e4ecbe9..b1a6395 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -1242,6 +1242,9 @@ static int ac97_load (QEMUFile *f, void *opaque, int version_id)
     V_ (AC97_Line_In_Volume_Mute, AUD_MIXER_LINE_IN);
 #undef V_
 #endif
+    active[PI_INDEX] = !!(s->bm_regs[PI_INDEX].cr & CR_RPBM);
+    active[PO_INDEX] = !!(s->bm_regs[PO_INDEX].cr & CR_RPBM);
+    active[MC_INDEX] = !!(s->bm_regs[MC_INDEX].cr & CR_RPBM);
     reset_voices (s, active);

     s->bup_flag = 0;
-- 
1.6.2.5

[Qemu-devel] [PATCH 19/20] ac97: up savevm version and remove active from state

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ac97.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/ac97.c b/hw/ac97.c
index b1a6395..8064fc3 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -1170,7 +1170,6 @@ static void po_callback (void *opaque, int free)
 static void ac97_save (QEMUFile *f, void *opaque)
 {
     size_t i;
-    uint8_t active[LAST_INDEX];
     AC97LinkState *s = opaque;

     pci_device_save (&s->dev, f);
@@ -1193,11 +1192,6 @@ static void ac97_save (QEMUFile *f, void *opaque)
         qemu_put_be32s (f, &r->bd.ctl_len);
     }
     qemu_put_buffer (f, s->mixer_data, sizeof (s->mixer_data));
-
-    active[PI_INDEX] = AUD_is_active_in (s->voice_pi) ? 1 : 0;
-    active[PO_INDEX] = AUD_is_active_out (s->voice_po) ? 1 : 0;
-    active[MC_INDEX] = AUD_is_active_in (s->voice_mc) ? 1 : 0;
-    qemu_put_buffer (f, active, sizeof (active));
 }

 static int ac97_load (QEMUFile *f, void *opaque, int version_id)
@@ -1207,7 +1201,7 @@ static int ac97_load (QEMUFile *f, void *opaque, int version_id)
     uint8_t active[LAST_INDEX];
     AC97LinkState *s = opaque;

-    if (version_id != 2)
+    if (version_id < 2 || version_id > 3)
         return -EINVAL;

     ret = pci_device_load (&s->dev, f);
@@ -1232,7 +1226,8 @@ static int ac97_load (QEMUFile *f, void *opaque, int version_id)
         qemu_get_be32s (f, &r->bd.ctl_len);
     }
     qemu_get_buffer (f, s->mixer_data, sizeof (s->mixer_data));
-    qemu_get_buffer (f, active, sizeof (active));
+    if (version_id < 3)
+        qemu_get_buffer (f, active, sizeof (active));

 #ifdef USE_MIXER
     record_select (s, mixer_load (s, AC97_Record_Select));
@@ -1336,7 +1331,7 @@ static int ac97_initfn (PCIDevice *dev)

     pci_register_bar (&s->dev, 0, 256 * 4, PCI_ADDRESS_SPACE_IO, ac97_map);
     pci_register_bar (&s->dev, 1, 64 * 4, PCI_ADDRESS_SPACE_IO, ac97_map);
-    register_savevm ("ac97", 0, 2, ac97_save, ac97_load, s);
+    register_savevm ("ac97", 0, 3, ac97_save, ac97_load, s);
     qemu_register_reset (ac97_on_reset, s);
     AUD_register_card ("ac97", &s->card);
     ac97_on_reset (s);
-- 
1.6.2.5

[Qemu-devel] [PATCH 20/20] ac97: port to vmstate

[Juan Quintela]

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/ac97.c |  101 ++++++++++++++++++++++++++----------------------------------
 1 files changed, 44 insertions(+), 57 deletions(-)

diff --git a/hw/ac97.c b/hw/ac97.c
index 8064fc3..99a43f8 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -1167,68 +1167,31 @@ static void po_callback (void *opaque, int free)
     transfer_audio (opaque, PO_INDEX, free);
 }

-static void ac97_save (QEMUFile *f, void *opaque)
-{
-    size_t i;
-    AC97LinkState *s = opaque;
-
-    pci_device_save (&s->dev, f);
-
-    qemu_put_be32s (f, &s->glob_cnt);
-    qemu_put_be32s (f, &s->glob_sta);
-    qemu_put_be32s (f, &s->cas);
-
-    for (i = 0; i < ARRAY_SIZE (s->bm_regs); ++i) {
-        AC97BusMasterRegs *r = &s->bm_regs[i];
-        qemu_put_be32s (f, &r->bdbar);
-        qemu_put_8s (f, &r->civ);
-        qemu_put_8s (f, &r->lvi);
-        qemu_put_be16s (f, &r->sr);
-        qemu_put_be16s (f, &r->picb);
-        qemu_put_8s (f, &r->piv);
-        qemu_put_8s (f, &r->cr);
-        qemu_put_be32s (f, &r->bd_valid);
-        qemu_put_be32s (f, &r->bd.addr);
-        qemu_put_be32s (f, &r->bd.ctl_len);
+static const VMStateDescription vmstate_ac97_bm_regs = {
+    .name = "ac97_bm_regs",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_UINT32(bdbar, AC97BusMasterRegs),
+        VMSTATE_UINT8(civ, AC97BusMasterRegs),
+        VMSTATE_UINT8(lvi, AC97BusMasterRegs),
+        VMSTATE_UINT16(sr, AC97BusMasterRegs),
+        VMSTATE_UINT16(picb, AC97BusMasterRegs),
+        VMSTATE_UINT8(piv, AC97BusMasterRegs),
+        VMSTATE_UINT8(cr, AC97BusMasterRegs),
+        VMSTATE_UINT32(bd_valid, AC97BusMasterRegs),
+        VMSTATE_UINT32(bd.addr, AC97BusMasterRegs),
+        VMSTATE_UINT32(bd.ctl_len, AC97BusMasterRegs),
+        VMSTATE_END_OF_LIST()
     }
-    qemu_put_buffer (f, s->mixer_data, sizeof (s->mixer_data));
-}
+};

-static int ac97_load (QEMUFile *f, void *opaque, int version_id)
+static int ac97_post_load (void *opaque, int version_id)
 {
-    int ret;
-    size_t i;
     uint8_t active[LAST_INDEX];
     AC97LinkState *s = opaque;

-    if (version_id < 2 || version_id > 3)
-        return -EINVAL;
-
-    ret = pci_device_load (&s->dev, f);
-    if (ret)
-        return ret;
-
-    qemu_get_be32s (f, &s->glob_cnt);
-    qemu_get_be32s (f, &s->glob_sta);
-    qemu_get_be32s (f, &s->cas);
-
-    for (i = 0; i < ARRAY_SIZE (s->bm_regs); ++i) {
-        AC97BusMasterRegs *r = &s->bm_regs[i];
-        qemu_get_be32s (f, &r->bdbar);
-        qemu_get_8s (f, &r->civ);
-        qemu_get_8s (f, &r->lvi);
-        qemu_get_be16s (f, &r->sr);
-        qemu_get_be16s (f, &r->picb);
-        qemu_get_8s (f, &r->piv);
-        qemu_get_8s (f, &r->cr);
-        qemu_get_be32s (f, &r->bd_valid);
-        qemu_get_be32s (f, &r->bd.addr);
-        qemu_get_be32s (f, &r->bd.ctl_len);
-    }
-    qemu_get_buffer (f, s->mixer_data, sizeof (s->mixer_data));
-    if (version_id < 3)
-        qemu_get_buffer (f, active, sizeof (active));
-
 #ifdef USE_MIXER
     record_select (s, mixer_load (s, AC97_Record_Select));
 #define V_(a, b) set_volume (s, a, b, mixer_load (s, a))
@@ -1247,6 +1210,30 @@ static int ac97_load (QEMUFile *f, void *opaque, int version_id)
     return 0;
 }

+static bool is_version_2 (void *opaque, int version_id)
+{
+    return version_id == 2;
+}
+
+static const VMStateDescription vmstate_ac97 = {
+    .name = "ac97",
+    .version_id = 3,
+    .minimum_version_id = 2,
+    .minimum_version_id_old = 2,
+    .post_load = ac97_post_load,
+    .fields      = (VMStateField []) {
+        VMSTATE_PCI_DEVICE(dev, AC97LinkState),
+        VMSTATE_UINT32(glob_cnt, AC97LinkState),
+        VMSTATE_UINT32(glob_sta, AC97LinkState),
+        VMSTATE_UINT32(cas, AC97LinkState),
+        VMSTATE_STRUCT_ARRAY(bm_regs, AC97LinkState, 3, 1,
+                             vmstate_ac97_bm_regs, AC97BusMasterRegs),
+        VMSTATE_BUFFER(mixer_data, AC97LinkState),
+        VMSTATE_UNUSED_TEST(is_version_2, 4),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void ac97_map (PCIDevice *pci_dev, int region_num,
                       uint32_t addr, uint32_t size, int type)
 {
@@ -1331,7 +1318,7 @@ static int ac97_initfn (PCIDevice *dev)

     pci_register_bar (&s->dev, 0, 256 * 4, PCI_ADDRESS_SPACE_IO, ac97_map);
     pci_register_bar (&s->dev, 1, 64 * 4, PCI_ADDRESS_SPACE_IO, ac97_map);
-    register_savevm ("ac97", 0, 3, ac97_save, ac97_load, s);
+    vmstate_register (0, &vmstate_ac97, s);
     qemu_register_reset (ac97_on_reset, s);
     AUD_register_card ("ac97", &s->card);
     ac97_on_reset (s);
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 00/20] Port audio to vmstate

[malc]

On Thu, 22 Oct 2009, Juan Quintela wrote:

> Hi
> 
> THis patch series port audio to vmstate.
> - fix compliation with DEBUG_PLIVE and DEBUG_AC97
> - unfold IO_READ_PROTO/IO_WRITE_PROTO and remove them
> - audio.c: the easiest thing to port (nothing inside)
> - sb16: port to vmstate, testing shows that it didn't survive migrations
>   very well (it happened before already).  Guest got this messages:
>    I got lots of underruns on the guest reported by alsa
>   on monitor I get lots of:
>    sb16: warning: command 0x42,2 is not truly understood yet

That's an attempt to do something or other with ADC (which is not
implmented for SB16) if my memory serves me.

> - es1370: the best working with migration.
> - adlib: I am not able to get sound out of it on any recent Fedora :(

It's an FM chip, trying to play PCM with it just not gonna fly.

> - cs4231a: it uses irq9, and that don't fly with acpi using that irq on qemu

-no-acpi

>   I disabled dma_running before loading state.  malc can you take a look here?

Can you please elaborate on what is exactly the problem.

>   I changed it to be able to use dma_running state field for state.

> - gus: no module anymore on Fedora

Use DOS.

> - ac97: from Anthony sugestion, remove the use of active array, it can be
>         recalculated.  All my testing (muted, not muted, ..., shows that
>         transmited array is the same one than the recalculated one).

Good.

>   ac97 don't work always with migration.
> 
> How did I test:
> - start a linux guest
> - inside there play a song (ogg123 foo.ogg)
> - in the middle of the song do savevm foo
> - later restart qemu with -loadvm foo option
> 
> What did I expect?:
> - I expected after ending the loadvm to hear the contination of the song.
> 
> Actual results:
> - es1370: the one working better
> - sb16: lots of underruns, very low sound quality.
> - ac97: mixed results.  The 1st 5-10 seconds always sound perfect
>    Then, between 10 and 30 seconds, sometimes it lots syncs and start playing at
>    full speed, basiaclly no sound ouput. No sound after this is ever generated.
>    If it is able to generate sound for 30-40 seconds, then sound works correctly.
>   I tried to debug this, enabled all the audio DEBUG*, but I didn't find what is
>   happening.
>   Notice that this happens when I launch from the same savevm image.  Some times
>   it works well, some times it stops working at 5 seconds, sometimes it stops
>   working at 10 seconds, and so on.
>   My theory is that we are not saving all the state that we need, but I have been
>   unable to found anything obviosu here.  malc, do you have any suggestion?

a. See how it behaves when you use OSS on the guest instead
   (might need clocking=48000 module parameter)

b. http://mpxplay.sourceforge.net/ is the DOS player which knows about
   i810 audio, see how it fares there
> 
> This took a lot because the ac97 testing, I thouguht the ac97
> problems were due to my changes.  It just happened that the 1st time
> that I loaded without my changes it just worked :( Problem can be
> reproduced as easily without any of my changes.  More work needs to
> be done to be sure that ac97 migrates correctly, but that is
> independent of this patch :)
> 

I don't like vmstate, but if you are dead bent on adding it to audio,
knock yourself out, IO_READ/WRITE_PROTO will stay. Can't help with
migration, GUS/Adlib are better tested with DOS and as for cs4231a i
don't quite understand what you are asking.

-- 
mailto:av1474@comtv.ru

[Qemu-devel] Re: [PATCH 00/20] Port audio to vmstate

[Juan Quintela]

malc <av1474@comtv.ru> wrote:
> On Thu, 22 Oct 2009, Juan Quintela wrote:
>

Hi

>> - es1370: the best working with migration.
>> - adlib: I am not able to get sound out of it on any recent Fedora :(
>
> It's an FM chip, trying to play PCM with it just not gonna fly.

That could expain it :)

>>   I disabled dma_running before loading state.  malc can you take a look here?

> Can you please elaborate on what is exactly the problem.

What we have here:

On save_state, we save a state field:
    val = s->dma_running; qemu_put_be32s (f, &val);

Now on load state, we read it to one local variable:
    qemu_get_be32s (f, &dma_running);

My understanding of the code here is that you should never assign
directly s->dma_running, that you should set/clean it through
  cs_reset_voices()
(setting this value has _side-effects_

So far so good.

Now, the whole point of vmstate is:
- state is described declaratively
- and load/save are "kind" of inverse functions. One save
  the value of one field to the state, and the other takes the value
  from the state and put it back in the field.
- Later (post_load), we do any side-effect that we need to do with the
  new loaded state.

Here we can't do this.  How would I do it in any other driver, i.e. (no
audio).  Easy, you just declare dma_running_vmstate in the state, and
in post_load(), you just call cs_reset_voices depending on that value.

You already ruled out that solution on ac97.  Didn't even try here.

No problem, I just read it on s->dma_running(), and on post_load I do:

if (s->dma_running && (s->dregs[Interface_Configuration] & PEN)) {
      s->dma_running = 0;
      cs_reset_voices (s, s->dregs[FS_And_Playback_Data_Format]);
}

And call it a day :).

But then I thought of what happens if you call loadvm from the monitor
while s->dma_running = 1.

And decided that the proper thing to do is to do:

in pre_load()
 - we set dma_running to 0 if if it 1, but do it like in
   cs_reset_voices()

        if (s->dma_running) {
            DMA_release_DREQ (s->dma);
            AUD_set_active_out (s->voice, 0);
        }
        s->dma_running = 0;

And now, in post_load, we know that s->dma_runing is what was comes from
migration, and we can set it to this value.  Do you think that my
explanation is clear?

Discussing this with Anthony on irc, he thinks that dma_running is the
same that  (s->dregs[Interface_Configuration] & PEN), and that we could
remove it from the state, but I am not sure, nad decided not to go that
route (because I am not sure, not because it is not the same.  I don't know).

>> - ac97: from Anthony sugestion, remove the use of active array, it can be
>>         recalculated.  All my testing (muted, not muted, ..., shows that
>>         transmited array is the same one than the recalculated one).
>
> Good.
>
>>   ac97 don't work always with migration.
>> 
>> How did I test:
>> - start a linux guest
>> - inside there play a song (ogg123 foo.ogg)
>> - in the middle of the song do savevm foo
>> - later restart qemu with -loadvm foo option
>> 
>> What did I expect?:
>> - I expected after ending the loadvm to hear the contination of the song.
>> 
>> Actual results:
>> - es1370: the one working better
>> - sb16: lots of underruns, very low sound quality.
>> - ac97: mixed results.  The 1st 5-10 seconds always sound perfect
>>    Then, between 10 and 30 seconds, sometimes it lots syncs and start playing at
>>    full speed, basiaclly no sound ouput. No sound after this is ever generated.
>>    If it is able to generate sound for 30-40 seconds, then sound works correctly.
>>   I tried to debug this, enabled all the audio DEBUG*, but I didn't find what is
>>   happening.
>>   Notice that this happens when I launch from the same savevm image.  Some times
>>   it works well, some times it stops working at 5 seconds, sometimes it stops
>>   working at 10 seconds, and so on.
>>   My theory is that we are not saving all the state that we need, but I have been
>>   unable to found anything obviosu here.  malc, do you have any suggestion?
>
> a. See how it behaves when you use OSS on the guest instead
>    (might need clocking=48000 module parameter)
>
> b. http://mpxplay.sourceforge.net/ is the DOS player which knows about
>    i810 audio, see how it fares there
>> 
>> This took a lot because the ac97 testing, I thouguht the ac97
>> problems were due to my changes.  It just happened that the 1st time
>> that I loaded without my changes it just worked :( Problem can be
>> reproduced as easily without any of my changes.  More work needs to
>> be done to be sure that ac97 migrates correctly, but that is
>> independent of this patch :)
>> 
>
> I don't like vmstate, but if you are dead bent on adding it to audio,
> knock yourself out, IO_READ/WRITE_PROTO will stay. Can't help with
> migration, GUS/Adlib are better tested with DOS and as for cs4231a i
> don't quite understand what you are asking.

ok, will integrate the vmstate changes, and leave the
IO_READ/WRITE_PROTO until there are consensous about that :)

Later, Juan.

[Qemu-devel] Re: [PATCH 00/20] Port audio to vmstate

[malc]

On Fri, 23 Oct 2009, Juan Quintela wrote:

> malc <av1474@comtv.ru> wrote:
> > On Thu, 22 Oct 2009, Juan Quintela wrote:
> >
> 
> Hi
> 
> >> - es1370: the best working with migration.
> >> - adlib: I am not able to get sound out of it on any recent Fedora :(
> >
> > It's an FM chip, trying to play PCM with it just not gonna fly.
> 
> That could expain it :)
> 
> >>   I disabled dma_running before loading state.  malc can you take a look here?
> 
> > Can you please elaborate on what is exactly the problem.
> 
> What we have here:
> 
> On save_state, we save a state field:
>     val = s->dma_running; qemu_put_be32s (f, &val);
> 
> Now on load state, we read it to one local variable:
>     qemu_get_be32s (f, &dma_running);
> 
> My understanding of the code here is that you should never assign
> directly s->dma_running, that you should set/clean it through
>   cs_reset_voices()
> (setting this value has _side-effects_

Okay, i've looked at the code dma_running is basically a flag which is
sortof redundant but for the lack of better interface tells us whether
we have DREQ asserted, nothing more nothing less.

> 
> So far so good.
> 
> Now, the whole point of vmstate is:
> - state is described declaratively
> - and load/save are "kind" of inverse functions. One save
>   the value of one field to the state, and the other takes the value
>   from the state and put it back in the field.
> - Later (post_load), we do any side-effect that we need to do with the
>   new loaded state.
> 
> Here we can't do this.  How would I do it in any other driver, i.e. (no
> audio).  Easy, you just declare dma_running_vmstate in the state, and
> in post_load(), you just call cs_reset_voices depending on that value.
> 
> You already ruled out that solution on ac97.  Didn't even try here.
> 
> No problem, I just read it on s->dma_running(), and on post_load I do:
> 
> if (s->dma_running && (s->dregs[Interface_Configuration] & PEN)) {
>       s->dma_running = 0;
>       cs_reset_voices (s, s->dregs[FS_And_Playback_Data_Format]);
> }
> 
> And call it a day :).
> 
> But then I thought of what happens if you call loadvm from the monitor
> while s->dma_running = 1.
> 
> And decided that the proper thing to do is to do:
> 
> in pre_load()
>  - we set dma_running to 0 if if it 1, but do it like in
>    cs_reset_voices()
> 
>         if (s->dma_running) {
>             DMA_release_DREQ (s->dma);
>             AUD_set_active_out (s->voice, 0);
>         }
>         s->dma_running = 0;
> 
> And now, in post_load, we know that s->dma_runing is what was comes from
> migration, and we can set it to this value.  Do you think that my
> explanation is clear?

Sorry, i haven't slept in quite a while, so nothing makes any sense
currently, i'll try to do a mental exercise of running it solely
inside my brain sometime later.

> Discussing this with Anthony on irc, he thinks that dma_running is the
> same that  (s->dregs[Interface_Configuration] & PEN), and that we could
> remove it from the state, but I am not sure, nad decided not to go that
> route (because I am not sure, not because it is not the same.  I don't know).

It's not quite the same as Iconf & PEN (see for instance PPIO
handling, or lack thereof)

[..snip..]

-- 
mailto:av1474@comtv.ru