[Qemu-devel] [PATCH 0/3] rtc port to VMState (and fix rtc-td hack)

[Qemu-devel] [PATCH 0/3] rtc port to VMState (and fix rtc-td hack)

[Juan Quintela]

now:
- minor indentation cleanup
- integrate rtc-td-hack properly in normal rtc state
- port to VMState

>From the fix rtc-td-hack that didn't change from last sent.
v2:

Only call rtc_coalesced_timer_update() if rtc_td_hack is true.
Patch was failing to load its own images.

v1:

There hasn't still released a version with the rtc_td hack.
Do we want to release 0.11 with this hack or change to a more maintenable mode,
i.e. just upgrade the version to 3 and save always the new values.
We should also propably add rtd_hack to the state, and make sure that value is the
same in origing and destination of th emigration.
Another problem is what we do with kvm, they have already done releases
with the "mc146818rtc-td" savevm section.

Notice that rtc-td-hack should really, really be enabled by defaulet.  It is
needed to have windows guest running correctly under load.

What do you think?  What should we do?

Later, Juan.

PD. Once that I was looking here, this driver has several features that are not
    used anywhere else.
- rtc_mm_init() is defined and exported but not used
- rtc_init_sqw() is defined, but only called from rtc_init() with sqw_irq = NULL
- sqw_irq is only assigned in rtc_init_sqw() and always to NULL value.

Is there a reason why we have the functionality that is not used anywhere?

Juan Quintela (3):
  mc145818rtc: fix saving of rtc-td hack properly upgrading the version
    number
  mc146818rtc: port rtc to vmstate
  mc146818rtc: fix indentation

 hw/mc146818rtc.c |  117 +++++++++++++++++------------------------------------
 1 files changed, 38 insertions(+), 79 deletions(-)

[Qemu-devel] [PATCH 1/3] mc145818rtc: fix saving of rtc-td hack properly upgrading the version number

[Juan Quintela]

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

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 0b945d5..a9d849b 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -76,11 +76,9 @@ struct RTCState {
     int64_t next_periodic_time;
     /* second update */
     int64_t next_second_time;
-#ifdef TARGET_I386
     uint32_t irq_coalesced;
     uint32_t period;
     QEMUTimer *coalesced_timer;
-#endif
     QEMUTimer *second_timer;
     QEMUTimer *second_timer2;
 };
@@ -524,13 +522,15 @@ static void rtc_save(QEMUFile *f, void *opaque)
     qemu_put_be64(f, s->next_second_time);
     qemu_put_timer(f, s->second_timer);
     qemu_put_timer(f, s->second_timer2);
+    qemu_put_be32(f, s->irq_coalesced);
+    qemu_put_be32(f, s->period);
 }

 static int rtc_load(QEMUFile *f, void *opaque, int version_id)
 {
     RTCState *s = opaque;

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

     qemu_get_buffer(f, s->cmos_data, 128);
@@ -550,31 +550,18 @@ static int rtc_load(QEMUFile *f, void *opaque, int version_id)
     s->next_second_time=qemu_get_be64(f);
     qemu_get_timer(f, s->second_timer);
     qemu_get_timer(f, s->second_timer2);
-    return 0;
-}

+    if (version_id >= 2) {
+        s->irq_coalesced = qemu_get_be32(f);
+        s->period = qemu_get_be32(f);
 #ifdef TARGET_I386
-static void rtc_save_td(QEMUFile *f, void *opaque)
-{
-    RTCState *s = opaque;
-
-    qemu_put_be32(f, s->irq_coalesced);
-    qemu_put_be32(f, s->period);
-}
-
-static int rtc_load_td(QEMUFile *f, void *opaque, int version_id)
-{
-    RTCState *s = opaque;
-
-    if (version_id != 1)
-        return -EINVAL;
-
-    s->irq_coalesced = qemu_get_be32(f);
-    s->period = qemu_get_be32(f);
-    rtc_coalesced_timer_update(s);
+        if (rtc_td_hack) {
+            rtc_coalesced_timer_update(s);
+        }
+#endif
+    }
     return 0;
 }
-#endif

 static void rtc_reset(void *opaque)
 {
@@ -622,11 +609,7 @@ static int rtc_initfn(ISADevice *dev)
     register_ioport_write(base, 2, 1, cmos_ioport_write, s);
     register_ioport_read(base, 2, 1, cmos_ioport_read, s);

-    register_savevm("mc146818rtc", base, 1, rtc_save, rtc_load, s);
-#ifdef TARGET_I386
-    if (rtc_td_hack)
-        register_savevm("mc146818rtc-td", base, 1, rtc_save_td, rtc_load_td, s);
-#endif
+    register_savevm("mc146818rtc", base, 2, rtc_save, rtc_load, s);
     qemu_register_reset(rtc_reset, s);
     return 0;
 }
@@ -758,11 +741,7 @@ RTCState *rtc_mm_init(target_phys_addr_t base, int it_shift, qemu_irq irq,
     io_memory = cpu_register_io_memory(rtc_mm_read, rtc_mm_write, s);
     cpu_register_physical_memory(base, 2 << it_shift, io_memory);

-    register_savevm("mc146818rtc", base, 1, rtc_save, rtc_load, s);
-#ifdef TARGET_I386
-    if (rtc_td_hack)
-        register_savevm("mc146818rtc-td", base, 1, rtc_save_td, rtc_load_td, s);
-#endif
+    register_savevm("mc146818rtc", base, 2, rtc_save, rtc_load, s);
     qemu_register_reset(rtc_reset, s);
     return s;
 }
-- 
1.6.2.5

[Qemu-devel] [PATCH 2/3] mc146818rtc: port rtc to vmstate

[Juan Quintela]

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

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index a9d849b..4086592 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -501,59 +501,11 @@ static void rtc_set_date_from_host(RTCState *s)
     rtc_set_memory(s, REG_IBM_PS2_CENTURY_BYTE, val);
 }

-static void rtc_save(QEMUFile *f, void *opaque)
+static int rtc_post_load(void *opaque, int version_id)
 {
     RTCState *s = opaque;

-    qemu_put_buffer(f, s->cmos_data, 128);
-    qemu_put_8s(f, &s->cmos_index);
-
-    qemu_put_be32(f, s->current_tm.tm_sec);
-    qemu_put_be32(f, s->current_tm.tm_min);
-    qemu_put_be32(f, s->current_tm.tm_hour);
-    qemu_put_be32(f, s->current_tm.tm_wday);
-    qemu_put_be32(f, s->current_tm.tm_mday);
-    qemu_put_be32(f, s->current_tm.tm_mon);
-    qemu_put_be32(f, s->current_tm.tm_year);
-
-    qemu_put_timer(f, s->periodic_timer);
-    qemu_put_be64(f, s->next_periodic_time);
-
-    qemu_put_be64(f, s->next_second_time);
-    qemu_put_timer(f, s->second_timer);
-    qemu_put_timer(f, s->second_timer2);
-    qemu_put_be32(f, s->irq_coalesced);
-    qemu_put_be32(f, s->period);
-}
-
-static int rtc_load(QEMUFile *f, void *opaque, int version_id)
-{
-    RTCState *s = opaque;
-
-    if (version_id < 1 || version_id > 2)
-        return -EINVAL;
-
-    qemu_get_buffer(f, s->cmos_data, 128);
-    qemu_get_8s(f, &s->cmos_index);
-
-    s->current_tm.tm_sec=qemu_get_be32(f);
-    s->current_tm.tm_min=qemu_get_be32(f);
-    s->current_tm.tm_hour=qemu_get_be32(f);
-    s->current_tm.tm_wday=qemu_get_be32(f);
-    s->current_tm.tm_mday=qemu_get_be32(f);
-    s->current_tm.tm_mon=qemu_get_be32(f);
-    s->current_tm.tm_year=qemu_get_be32(f);
-
-    qemu_get_timer(f, s->periodic_timer);
-    s->next_periodic_time=qemu_get_be64(f);
-
-    s->next_second_time=qemu_get_be64(f);
-    qemu_get_timer(f, s->second_timer);
-    qemu_get_timer(f, s->second_timer2);
-
     if (version_id >= 2) {
-        s->irq_coalesced = qemu_get_be32(f);
-        s->period = qemu_get_be32(f);
 #ifdef TARGET_I386
         if (rtc_td_hack) {
             rtc_coalesced_timer_update(s);
@@ -563,6 +515,33 @@ static int rtc_load(QEMUFile *f, void *opaque, int version_id)
     return 0;
 }

+static const VMStateDescription vmstate_rtc = {
+    .name = "mc146818rtc",
+    .version_id = 2,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .post_load = rtc_post_load,
+    .fields      = (VMStateField []) {
+        VMSTATE_BUFFER(cmos_data, RTCState),
+        VMSTATE_UINT8(cmos_index, RTCState),
+        VMSTATE_INT32(current_tm.tm_sec, RTCState),
+        VMSTATE_INT32(current_tm.tm_min, RTCState),
+        VMSTATE_INT32(current_tm.tm_hour, RTCState),
+        VMSTATE_INT32(current_tm.tm_wday, RTCState),
+        VMSTATE_INT32(current_tm.tm_mday, RTCState),
+        VMSTATE_INT32(current_tm.tm_mon, RTCState),
+        VMSTATE_INT32(current_tm.tm_year, RTCState),
+        VMSTATE_TIMER(periodic_timer, RTCState),
+        VMSTATE_INT64(next_periodic_time, RTCState),
+        VMSTATE_INT64(next_second_time, RTCState),
+        VMSTATE_TIMER(second_timer, RTCState),
+        VMSTATE_TIMER(second_timer2, RTCState),
+        VMSTATE_UINT32_V(irq_coalesced, RTCState, 2),
+        VMSTATE_UINT32_V(period, RTCState, 2),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
 static void rtc_reset(void *opaque)
 {
     RTCState *s = opaque;
@@ -609,7 +588,7 @@ static int rtc_initfn(ISADevice *dev)
     register_ioport_write(base, 2, 1, cmos_ioport_write, s);
     register_ioport_read(base, 2, 1, cmos_ioport_read, s);

-    register_savevm("mc146818rtc", base, 2, rtc_save, rtc_load, s);
+    vmstate_register(base, &vmstate_rtc, s);
     qemu_register_reset(rtc_reset, s);
     return 0;
 }
@@ -741,7 +720,7 @@ RTCState *rtc_mm_init(target_phys_addr_t base, int it_shift, qemu_irq irq,
     io_memory = cpu_register_io_memory(rtc_mm_read, rtc_mm_write, s);
     cpu_register_physical_memory(base, 2 << it_shift, io_memory);

-    register_savevm("mc146818rtc", base, 2, rtc_save, rtc_load, s);
+    vmstate_register(base, &vmstate_rtc, s);
     qemu_register_reset(rtc_reset, s);
     return s;
 }
-- 
1.6.2.5

[Qemu-devel] [PATCH 3/3] mc146818rtc: fix indentation

[Juan Quintela]

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

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 4086592..8b8630f 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -83,7 +83,8 @@ struct RTCState {
     QEMUTimer *second_timer2;
 };

-static void rtc_irq_raise(qemu_irq irq) {
+static void rtc_irq_raise(qemu_irq irq)
+{
     /* When HPET is operating in legacy mode, RTC interrupts are disabled
      * We block qemu_irq_raise, but not qemu_irq_lower, in case legacy
      * mode is established while interrupt is raised. We want it to
-- 
1.6.2.5

Re: [Qemu-devel] [PATCH 2/3] mc146818rtc: port rtc to vmstate

[Anthony Liguori]

Juan Quintela wrote:
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hw/mc146818rtc.c |   81 ++++++++++++++++++++----------------------------------
>  1 files changed, 30 insertions(+), 51 deletions(-)
>
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index a9d849b..4086592 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -501,59 +501,11 @@ static void rtc_set_date_from_host(RTCState *s)
>      rtc_set_memory(s, REG_IBM_PS2_CENTURY_BYTE, val);
>  }
>
> -static void rtc_save(QEMUFile *f, void *opaque)
> +static int rtc_post_load(void *opaque, int version_id)
>  {
>      RTCState *s = opaque;
>   

Breaks the build because this is unused...

>  #ifdef TARGET_I386
>          if (rtc_td_hack) {
>              rtc_coalesced_timer_update(s);
>   

In target mips-softmmu.

Regards,

Anthony Liguori