[Qemu-devel] [PATCH 0/4] Use rtc_clock uniformly for ARM

[Qemu-devel] [PATCH 0/4] Use rtc_clock uniformly for ARM

[Paolo Bonzini]

This series uses rtc_clock uniformly in device models that provide RTC
functionality.  This will let users choose the desired semantics for
the clock.

This is most important with qtest, where "-rtc clock=vm" will provide
determinism and let you run tests that fake execution for large time
periods.  However, for consistency I'm switching also the two RTC
models that always used the vm_clock, m48t59 and pl031.  m48t59 is
not ARM so I'm sending it separately.

Patch 3 fixes an unrelated bug in the pl031 migration code.

Paolo Bonzini (4):
  rtc: add -rtc clock=rt
  arm: switch real-time clocks to rtc_clock
  pl031: rearm alarm timer upon load
  pl031: switch clock base to rtc_clock

 hw/omap1.c      |   10 +++---
 hw/pl031.c      |   76 ++++++++++++++++++++++++++++++++++++-------------------
 hw/pxa2xx.c     |   28 ++++++++++----------
 hw/strongarm.c  |   10 +++---
 hw/twl92230.c   |    9 +++---
 qemu-options.hx |    7 +++--
 vl.c            |    2 +
 7 files changed, 85 insertions(+), 57 deletions(-)

-- 
1.7.7.1

[Qemu-devel] [PATCH 1/4] rtc: add -rtc clock=rt

[Paolo Bonzini]

This will let people use backwards-compatible semantics for devices that
will be affected by the following patch.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-options.hx |    7 ++++---
 vl.c            |    2 ++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/qemu-options.hx b/qemu-options.hx
index 6295cde..da311f0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2360,7 +2360,7 @@ DEF("localtime", 0, QEMU_OPTION_localtime, "", QEMU_ARCH_ALL)
 DEF("startdate", HAS_ARG, QEMU_OPTION_startdate, "", QEMU_ARCH_ALL)
 
 DEF("rtc", HAS_ARG, QEMU_OPTION_rtc, \
-    "-rtc [base=utc|localtime|date][,clock=host|vm][,driftfix=none|slew]\n" \
+    "-rtc [base=utc|localtime|date][,clock=host|rt|vm][,driftfix=none|slew]\n" \
     "                set the RTC base and clock, enable drift fix for clock ticks (x86 only)\n",
     QEMU_ARCH_ALL)
 
@@ -2376,8 +2376,9 @@ format @code{2006-06-17T16:01:21} or @code{2006-06-17}. The default base is UTC.
 By default the RTC is driven by the host system time. This allows to use the
 RTC as accurate reference clock inside the guest, specifically if the host
 time is smoothly following an accurate external reference clock, e.g. via NTP.
-If you want to isolate the guest time from the host, even prevent it from
-progressing during suspension, you can set @option{clock} to @code{vm} instead.
+If you want to isolate the guest time from the host, you can set @option{clock}
+to @code{rt} instead.  To even prevent it from progressing during suspension,
+you can set it to @code{vm}.
 
 Enable @option{driftfix} (i386 targets only) if you experience time drift problems,
 specifically with Windows' ACPI HAL. This option will try to figure out how
diff --git a/vl.c b/vl.c
index ba55b35..6ad67a6 100644
--- a/vl.c
+++ b/vl.c
@@ -537,6 +537,8 @@ static void configure_rtc(QemuOpts *opts)
     if (value) {
         if (!strcmp(value, "host")) {
             rtc_clock = host_clock;
+        } else if (!strcmp(value, "rt")) {
+            rtc_clock = rt_clock;
         } else if (!strcmp(value, "vm")) {
             rtc_clock = vm_clock;
         } else {
-- 
1.7.7.1

[Qemu-devel] [PATCH 2/4] arm: switch real-time clocks to rtc_clock

[Paolo Bonzini]

This lets the user specify the desired semantics.  By default, the RTC
will follow adjustments from the host's NTP client.  "-rtc clock=vm" will
improve determinism with both icount and qtest.  Finally, the previous
behavior is available with "-rtc clock=rt".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/omap1.c     |   10 +++++-----
 hw/pxa2xx.c    |   28 ++++++++++++++--------------
 hw/strongarm.c |   10 +++++-----
 hw/twl92230.c  |    9 +++++----
 4 files changed, 29 insertions(+), 28 deletions(-)

diff --git a/hw/omap1.c b/hw/omap1.c
index 1aa5f23..e9b19a2 100644
--- a/hw/omap1.c
+++ b/hw/omap1.c
@@ -2888,7 +2888,7 @@ static void omap_rtc_reset(struct omap_rtc_s *s)
     s->pm_am = 0;
     s->auto_comp = 0;
     s->round = 0;
-    s->tick = qemu_get_clock_ms(rt_clock);
+    s->tick = qemu_get_clock_ms(rtc_clock);
     memset(&s->alarm_tm, 0, sizeof(s->alarm_tm));
     s->alarm_tm.tm_mday = 0x01;
     s->status = 1 << 7;
@@ -2909,7 +2909,7 @@ static struct omap_rtc_s *omap_rtc_init(MemoryRegion *system_memory,
 
     s->irq = timerirq;
     s->alarm = alarmirq;
-    s->clk = qemu_new_timer_ms(rt_clock, omap_rtc_tick, s);
+    s->clk = qemu_new_timer_ms(rtc_clock, omap_rtc_tick, s);
 
     omap_rtc_reset(s);
 
@@ -3497,9 +3497,9 @@ static void omap_lpg_tick(void *opaque)
     struct omap_lpg_s *s = opaque;
 
     if (s->cycle)
-        qemu_mod_timer(s->tm, qemu_get_clock_ms(rt_clock) + s->period - s->on);
+        qemu_mod_timer(s->tm, qemu_get_clock_ms(rtc_clock) + s->period - s->on);
     else
-        qemu_mod_timer(s->tm, qemu_get_clock_ms(rt_clock) + s->on);
+        qemu_mod_timer(s->tm, qemu_get_clock_ms(rtc_clock) + s->on);
 
     s->cycle = !s->cycle;
     printf("%s: LED is %s\n", __FUNCTION__, s->cycle ? "on" : "off");
@@ -3617,7 +3617,7 @@ static struct omap_lpg_s *omap_lpg_init(MemoryRegion *system_memory,
     struct omap_lpg_s *s = (struct omap_lpg_s *)
             g_malloc0(sizeof(struct omap_lpg_s));
 
-    s->tm = qemu_new_timer_ms(rt_clock, omap_lpg_tick, s);
+    s->tm = qemu_new_timer_ms(rtc_clock, omap_lpg_tick, s);
 
     omap_lpg_reset(s);
 
diff --git a/hw/pxa2xx.c b/hw/pxa2xx.c
index 6ddd500..35816a5 100644
--- a/hw/pxa2xx.c
+++ b/hw/pxa2xx.c
@@ -875,7 +875,7 @@ static inline void pxa2xx_rtc_int_update(PXA2xxRTCState *s)
 
 static void pxa2xx_rtc_hzupdate(PXA2xxRTCState *s)
 {
-    int64_t rt = qemu_get_clock_ms(rt_clock);
+    int64_t rt = qemu_get_clock_ms(rtc_clock);
     s->last_rcnr += ((rt - s->last_hz) << 15) /
             (1000 * ((s->rttr & 0xffff) + 1));
     s->last_rdcr += ((rt - s->last_hz) << 15) /
@@ -885,7 +885,7 @@ static void pxa2xx_rtc_hzupdate(PXA2xxRTCState *s)
 
 static void pxa2xx_rtc_swupdate(PXA2xxRTCState *s)
 {
-    int64_t rt = qemu_get_clock_ms(rt_clock);
+    int64_t rt = qemu_get_clock_ms(rtc_clock);
     if (s->rtsr & (1 << 12))
         s->last_swcr += (rt - s->last_sw) / 10;
     s->last_sw = rt;
@@ -893,7 +893,7 @@ static void pxa2xx_rtc_swupdate(PXA2xxRTCState *s)
 
 static void pxa2xx_rtc_piupdate(PXA2xxRTCState *s)
 {
-    int64_t rt = qemu_get_clock_ms(rt_clock);
+    int64_t rt = qemu_get_clock_ms(rtc_clock);
     if (s->rtsr & (1 << 15))
         s->last_swcr += rt - s->last_pi;
     s->last_pi = rt;
@@ -1019,16 +1019,16 @@ static uint64_t pxa2xx_rtc_read(void *opaque, target_phys_addr_t addr,
     case PIAR:
         return s->piar;
     case RCNR:
-        return s->last_rcnr + ((qemu_get_clock_ms(rt_clock) - s->last_hz) << 15) /
+        return s->last_rcnr + ((qemu_get_clock_ms(rtc_clock) - s->last_hz) << 15) /
                 (1000 * ((s->rttr & 0xffff) + 1));
     case RDCR:
-        return s->last_rdcr + ((qemu_get_clock_ms(rt_clock) - s->last_hz) << 15) /
+        return s->last_rdcr + ((qemu_get_clock_ms(rtc_clock) - s->last_hz) << 15) /
                 (1000 * ((s->rttr & 0xffff) + 1));
     case RYCR:
         return s->last_rycr;
     case SWCR:
         if (s->rtsr & (1 << 12))
-            return s->last_swcr + (qemu_get_clock_ms(rt_clock) - s->last_sw) / 10;
+            return s->last_swcr + (qemu_get_clock_ms(rtc_clock) - s->last_sw) / 10;
         else
             return s->last_swcr;
     default:
@@ -1168,14 +1168,14 @@ static int pxa2xx_rtc_init(SysBusDevice *dev)
     s->last_swcr = (tm.tm_hour << 19) |
             (tm.tm_min << 13) | (tm.tm_sec << 7);
     s->last_rtcpicr = 0;
-    s->last_hz = s->last_sw = s->last_pi = qemu_get_clock_ms(rt_clock);
-
-    s->rtc_hz    = qemu_new_timer_ms(rt_clock, pxa2xx_rtc_hz_tick,    s);
-    s->rtc_rdal1 = qemu_new_timer_ms(rt_clock, pxa2xx_rtc_rdal1_tick, s);
-    s->rtc_rdal2 = qemu_new_timer_ms(rt_clock, pxa2xx_rtc_rdal2_tick, s);
-    s->rtc_swal1 = qemu_new_timer_ms(rt_clock, pxa2xx_rtc_swal1_tick, s);
-    s->rtc_swal2 = qemu_new_timer_ms(rt_clock, pxa2xx_rtc_swal2_tick, s);
-    s->rtc_pi    = qemu_new_timer_ms(rt_clock, pxa2xx_rtc_pi_tick,    s);
+    s->last_hz = s->last_sw = s->last_pi = qemu_get_clock_ms(rtc_clock);
+
+    s->rtc_hz    = qemu_new_timer_ms(rtc_clock, pxa2xx_rtc_hz_tick,    s);
+    s->rtc_rdal1 = qemu_new_timer_ms(rtc_clock, pxa2xx_rtc_rdal1_tick, s);
+    s->rtc_rdal2 = qemu_new_timer_ms(rtc_clock, pxa2xx_rtc_rdal2_tick, s);
+    s->rtc_swal1 = qemu_new_timer_ms(rtc_clock, pxa2xx_rtc_swal1_tick, s);
+    s->rtc_swal2 = qemu_new_timer_ms(rtc_clock, pxa2xx_rtc_swal2_tick, s);
+    s->rtc_pi    = qemu_new_timer_ms(rtc_clock, pxa2xx_rtc_pi_tick,    s);
 
     sysbus_init_irq(dev, &s->rtc_irq);
 
diff --git a/hw/strongarm.c b/hw/strongarm.c
index fe63fd7..0114f32 100644
--- a/hw/strongarm.c
+++ b/hw/strongarm.c
@@ -246,7 +246,7 @@ static inline void strongarm_rtc_int_update(StrongARMRTCState *s)
 
 static void strongarm_rtc_hzupdate(StrongARMRTCState *s)
 {
-    int64_t rt = qemu_get_clock_ms(rt_clock);
+    int64_t rt = qemu_get_clock_ms(rtc_clock);
     s->last_rcnr += ((rt - s->last_hz) << 15) /
             (1000 * ((s->rttr & 0xffff) + 1));
     s->last_hz = rt;
@@ -299,7 +299,7 @@ static uint64_t strongarm_rtc_read(void *opaque, target_phys_addr_t addr,
         return s->rtar;
     case RCNR:
         return s->last_rcnr +
-                ((qemu_get_clock_ms(rt_clock) - s->last_hz) << 15) /
+                ((qemu_get_clock_ms(rtc_clock) - s->last_hz) << 15) /
                 (1000 * ((s->rttr & 0xffff) + 1));
     default:
         printf("%s: Bad register 0x" TARGET_FMT_plx "\n", __func__, addr);
@@ -365,10 +365,10 @@ static int strongarm_rtc_init(SysBusDevice *dev)
     qemu_get_timedate(&tm, 0);
 
     s->last_rcnr = (uint32_t) mktimegm(&tm);
-    s->last_hz = qemu_get_clock_ms(rt_clock);
+    s->last_hz = qemu_get_clock_ms(rtc_clock);
 
-    s->rtc_alarm = qemu_new_timer_ms(rt_clock, strongarm_rtc_alarm_tick, s);
-    s->rtc_hz = qemu_new_timer_ms(rt_clock, strongarm_rtc_hz_tick, s);
+    s->rtc_alarm = qemu_new_timer_ms(rtc_clock, strongarm_rtc_alarm_tick, s);
+    s->rtc_hz = qemu_new_timer_ms(rtc_clock, strongarm_rtc_hz_tick, s);
 
     sysbus_init_irq(dev, &s->rtc_irq);
     sysbus_init_irq(dev, &s->rtc_hz_irq);
diff --git a/hw/twl92230.c b/hw/twl92230.c
index 6416752..4974a6a 100644
--- a/hw/twl92230.c
+++ b/hw/twl92230.c
@@ -22,6 +22,7 @@
 #include "hw.h"
 #include "qemu-timer.h"
 #include "i2c.h"
+#include "sysemu.h"
 #include "console.h"
 
 #define VERBOSE 1
@@ -71,14 +72,14 @@ static inline void menelaus_update(MenelausState *s)
 
 static inline void menelaus_rtc_start(MenelausState *s)
 {
-    s->rtc.next += qemu_get_clock_ms(rt_clock);
+    s->rtc.next += qemu_get_clock_ms(rtc_clock);
     qemu_mod_timer(s->rtc.hz_tm, s->rtc.next);
 }
 
 static inline void menelaus_rtc_stop(MenelausState *s)
 {
     qemu_del_timer(s->rtc.hz_tm);
-    s->rtc.next -= qemu_get_clock_ms(rt_clock);
+    s->rtc.next -= qemu_get_clock_ms(rtc_clock);
     if (s->rtc.next < 1)
         s->rtc.next = 1;
 }
@@ -781,7 +782,7 @@ static void menelaus_pre_save(void *opaque)
 {
     MenelausState *s = opaque;
     /* Should be <= 1000 */
-    s->rtc_next_vmstate =  s->rtc.next - qemu_get_clock_ms(rt_clock);
+    s->rtc_next_vmstate =  s->rtc.next - qemu_get_clock_ms(rtc_clock);
 }
 
 static int menelaus_post_load(void *opaque, int version_id)
@@ -842,7 +843,7 @@ static int twl92230_init(i2c_slave *i2c)
 {
     MenelausState *s = FROM_I2C_SLAVE(MenelausState, i2c);
 
-    s->rtc.hz_tm = qemu_new_timer_ms(rt_clock, menelaus_rtc_hz, s);
+    s->rtc.hz_tm = qemu_new_timer_ms(rtc_clock, menelaus_rtc_hz, s);
     /* Three output pins plus one interrupt pin.  */
     qdev_init_gpio_out(&i2c->qdev, s->out, 4);
 
-- 
1.7.7.1

Re: [Qemu-devel] [PATCH 2/4] arm: switch real-time clocks to rtc_clock

[andrzej zaborowski]

On 20 January 2012 13:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This lets the user specify the desired semantics.  By default, the RTC
> will follow adjustments from the host's NTP client.  "-rtc clock=vm" will
> improve determinism with both icount and qtest.  Finally, the previous
> behavior is available with "-rtc clock=rt".

Generally this makes sense except for the omap1 "lpg" module where the
clock state is not visible to the guest, only to the host.  It
emulates a blinking led.

Cheers

Re: [Qemu-devel] [PATCH 2/4] arm: switch real-time clocks to rtc_clock

[Paolo Bonzini]

On 02/17/2012 08:09 AM, andrzej zaborowski wrote:
>> > This lets the user specify the desired semantics.  By default, the RTC
>> > will follow adjustments from the host's NTP client.  "-rtc clock=vm" will
>> > improve determinism with both icount and qtest.  Finally, the previous
>> > behavior is available with "-rtc clock=rt".
> Generally this makes sense except for the omap1 "lpg" module where the
> clock state is not visible to the guest, only to the host.  It
> emulates a blinking led.

Ok, then I think it's better to switch that one to vm_clock (so that you
could use it as a deterministic debugging aid with -icount, for
example).  What do you think?

Paolo

Re: [Qemu-devel] [PATCH 2/4] arm: switch real-time clocks to rtc_clock

[andrzej zaborowski]

On 17 February 2012 09:05, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 02/17/2012 08:09 AM, andrzej zaborowski wrote:
>>> > This lets the user specify the desired semantics.  By default, the RTC
>>> > will follow adjustments from the host's NTP client.  "-rtc clock=vm" will
>>> > improve determinism with both icount and qtest.  Finally, the previous
>>> > behavior is available with "-rtc clock=rt".
>> Generally this makes sense except for the omap1 "lpg" module where the
>> clock state is not visible to the guest, only to the host.  It
>> emulates a blinking led.
>
> Ok, then I think it's better to switch that one to vm_clock (so that you
> could use it as a deterministic debugging aid with -icount, for
> example).  What do you think?

I was thinking about potential visual effects.  But perhaps you were
right with the first approach, i.e. debugging -> deterministic (which
in this case would ideally also use the frequency derived from the
system clock passed to omap_lpg_init), normal work -> realtime.  I'll
just push the original patch then, if you're ok with that.

Cheers

Re: [Qemu-devel] [PATCH 2/4] arm: switch real-time clocks to rtc_clock

[Paolo Bonzini]

On 02/17/2012 09:24 AM, andrzej zaborowski wrote:
>> > Ok, then I think it's better to switch that one to vm_clock (so that you
>> > could use it as a deterministic debugging aid with -icount, for
>> > example).  What do you think?
> I was thinking about potential visual effects.

vm_clock is the same as rt_clock as long as: 1) you don't use -icount;
2) you don't stop the VM.

Not pulsing the LED when the VM is stopped seems okay, so I think
switching from rt_clock to vm_clock is the right thing to do.

> But perhaps you were
> right with the first approach, i.e. debugging -> deterministic (which
> in this case would ideally also use the frequency derived from the
> system clock passed to omap_lpg_init), normal work -> realtime.

You should get this from -icount if you just use vm_clock.

The overall series doesn't apply anymore, I'll split omap_lpg into its
own patch and resend.

Paolo

[Qemu-devel] [PATCH 3/4] pl031: rearm alarm timer upon load

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pl031.c |   40 +++++++++++++++++++++++++---------------
 1 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/hw/pl031.c b/hw/pl031.c
index 2fb0c8e..1ca4c23 100644
--- a/hw/pl031.c
+++ b/hw/pl031.c
@@ -47,21 +47,6 @@ typedef struct {
     uint32_t is;
 } pl031_state;
 
-static const VMStateDescription vmstate_pl031 = {
-    .name = "pl031",
-    .version_id = 1,
-    .minimum_version_id = 1,
-    .fields = (VMStateField[]) {
-        VMSTATE_UINT32(tick_offset, pl031_state),
-        VMSTATE_UINT32(mr, pl031_state),
-        VMSTATE_UINT32(lr, pl031_state),
-        VMSTATE_UINT32(cr, pl031_state),
-        VMSTATE_UINT32(im, pl031_state),
-        VMSTATE_UINT32(is, pl031_state),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 static const unsigned char pl031_id[] = {
     0x31, 0x10, 0x14, 0x00,         /* Device ID        */
     0x0d, 0xf0, 0x05, 0xb1          /* Cell ID      */
@@ -213,6 +198,31 @@ static int pl031_init(SysBusDevice *dev)
     return 0;
 }
 
+static int pl031_post_load(void *opaque, int version_id)
+{
+    pl031_state *s = opaque;
+
+    pl031_set_alarm(s);
+    return 0;
+}
+
+static const VMStateDescription vmstate_pl031 = {
+    .name = "pl031",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .post_load = pl031_post_load,
+    .fields = (VMStateField[]) {
+        VMSTATE_UINT32(tick_offset, pl031_state),
+        VMSTATE_UINT32(mr, pl031_state),
+        VMSTATE_UINT32(lr, pl031_state),
+        VMSTATE_UINT32(cr, pl031_state),
+        VMSTATE_UINT32(im, pl031_state),
+        VMSTATE_UINT32(is, pl031_state),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+
 static SysBusDeviceInfo pl031_info = {
     .init = pl031_init,
     .qdev.name = "pl031",
-- 
1.7.7.1

[Qemu-devel] [PATCH 4/4] pl031: switch clock base to rtc_clock

[Paolo Bonzini]

This lets the user specify the desired semantics.  By default, the RTC
will follow adjustments from the host's NTP client, and will remain in
sync when the virtual machine is stopped.  The previous behavior, which
provides determinism with both icount and qtest, remains available with
"-rtc clock=vm".

pl031 supports migration, so we need to convert the time
base from rtc_clock to vm_clock and back for backwards compatibility.

The device model is kind of broken, because it stores the offset with
second precision rather than nanosecond.  The alarm timer thus may be
off by up to one second.  Anyway, this is unrelated to this change.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/pl031.c |   38 ++++++++++++++++++++++++++------------
 1 files changed, 26 insertions(+), 12 deletions(-)

diff --git a/hw/pl031.c b/hw/pl031.c
index 1ca4c23..1a4de99 100644
--- a/hw/pl031.c
+++ b/hw/pl031.c
@@ -13,6 +13,7 @@
 
 #include "sysbus.h"
 #include "qemu-timer.h"
+#include "sysemu.h"
 
 //#define DEBUG_PL031
 
@@ -38,6 +39,11 @@ typedef struct {
     QEMUTimer *timer;
     qemu_irq irq;
 
+    /* Needed to preserve the tick_count across migration, even if the
+     * absolute value of the rtc_clock is different on the source and
+     * destination.
+     */
+    uint32_t tick_offset_vmstate;
     uint32_t tick_offset;
 
     uint32_t mr;
@@ -68,27 +74,23 @@ static void pl031_interrupt(void * opaque)
 
 static uint32_t pl031_get_count(pl031_state *s)
 {
-    /* This assumes qemu_get_clock_ns returns the time since the machine was
-       created.  */
-    return s->tick_offset + qemu_get_clock_ns(vm_clock) / get_ticks_per_sec();
+    int64_t now = qemu_get_clock_ns(rtc_clock);
+    return s->tick_offset + now / get_ticks_per_sec();
 }
 
 static void pl031_set_alarm(pl031_state *s)
 {
-    int64_t now;
     uint32_t ticks;
 
-    now = qemu_get_clock_ns(vm_clock);
-    ticks = s->tick_offset + now / get_ticks_per_sec();
-
     /* The timer wraps around.  This subtraction also wraps in the same way,
        and gives correct results when alarm < now_ticks.  */
-    ticks = s->mr - ticks;
+    ticks = s->mr - pl031_get_count(s);
     DPRINTF("Alarm set in %ud ticks\n", ticks);
     if (ticks == 0) {
         qemu_del_timer(s->timer);
         pl031_interrupt(s);
     } else {
+        int64_t now = qemu_get_clock_ns(rtc_clock);
         qemu_mod_timer(s->timer, now + (int64_t)ticks * get_ticks_per_sec());
     }
 }
@@ -190,18 +192,29 @@ static int pl031_init(SysBusDevice *dev)
     sysbus_init_mmio(dev, &s->iomem);
 
     sysbus_init_irq(dev, &s->irq);
-    /* ??? We assume vm_clock is zero at this point.  */
     qemu_get_timedate(&tm, 0);
-    s->tick_offset = mktimegm(&tm);
+    s->tick_offset = mktimegm(&tm) - qemu_get_clock_ns(rtc_clock) / get_ticks_per_sec();
 
-    s->timer = qemu_new_timer_ns(vm_clock, pl031_interrupt, s);
+    s->timer = qemu_new_timer_ns(rtc_clock, pl031_interrupt, s);
     return 0;
 }
 
+static void pl031_pre_save(void *opaque)
+{
+    pl031_state *s = opaque;
+
+    /* tick_offset is base_time - rtc_clock base time.  Instead, we want to
+     * store the base time relative to the vm_clock for backwards-compatibility.  */
+    int64_t delta = qemu_get_clock_ns(rtc_clock) - qemu_get_clock_ns(vm_clock);
+    s->tick_offset_vmstate = s->tick_offset + delta / get_ticks_per_sec();
+}
+
 static int pl031_post_load(void *opaque, int version_id)
 {
     pl031_state *s = opaque;
 
+    int64_t delta = qemu_get_clock_ns(rtc_clock) - qemu_get_clock_ns(vm_clock);
+    s->tick_offset = s->tick_offset_vmstate - delta / get_ticks_per_sec();
     pl031_set_alarm(s);
     return 0;
 }
@@ -210,9 +223,10 @@ static const VMStateDescription vmstate_pl031 = {
     .name = "pl031",
     .version_id = 1,
     .minimum_version_id = 1,
+    .pre_save = pl031_pre_save,
     .post_load = pl031_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_UINT32(tick_offset, pl031_state),
+        VMSTATE_UINT32(tick_offset_vmstate, pl031_state),
         VMSTATE_UINT32(mr, pl031_state),
         VMSTATE_UINT32(lr, pl031_state),
         VMSTATE_UINT32(cr, pl031_state),
-- 
1.7.7.1

Re: [Qemu-devel] [PATCH 4/4] pl031: switch clock base to rtc_clock

[Peter Maydell]

On 20 January 2012 12:06, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This lets the user specify the desired semantics.  By default, the RTC
> will follow adjustments from the host's NTP client, and will remain in
> sync when the virtual machine is stopped.  The previous behavior, which
> provides determinism with both icount and qtest, remains available with
> "-rtc clock=vm".
>
> pl031 supports migration, so we need to convert the time
> base from rtc_clock to vm_clock and back for backwards compatibility.

If it would be cleaner I'm entirely happy for us to do a version
bump and break binary compatibility. (We bump the version on
the CPUState pretty regularly anyhow; save/load for ARM devices
currently is mostly for the benefit of snapshot save/restore rather
than between-qemu-versions migration.)

-- PMM

Re: [Qemu-devel] [PATCH 4/4] pl031: switch clock base to rtc_clock

[Paolo Bonzini]

On 01/30/2012 05:54 PM, Peter Maydell wrote:
>> >  This lets the user specify the desired semantics.  By default, the RTC
>> >  will follow adjustments from the host's NTP client, and will remain in
>> >  sync when the virtual machine is stopped.  The previous behavior, which
>> >  provides determinism with both icount and qtest, remains available with
>> >  "-rtc clock=vm".
>> >
>> >  pl031 supports migration, so we need to convert the time
>> >  base from rtc_clock to vm_clock and back for backwards compatibility.
> If it would be cleaner I'm entirely happy for us to do a version
> bump and break binary compatibility. (We bump the version on
> the CPUState pretty regularly anyhow; save/load for ARM devices
> currently is mostly for the benefit of snapshot save/restore rather
> than between-qemu-versions migration.)

Actually it's not really just backwards compatibility.

rtc_clock may be counting up from a different time base on the source 
and destination machine.  vm_clock is the only time base that is 
migrated, so it is the only one that is safe to use in the data stream.

Perhaps I could transfer the timer deadline relative to the current 
rtc_clock value and convert it again on the destination.  However, it 
would be basically the same code that transfers the value relative to 
the vm_clock; it's just a different offset to apply.

Paolo

Re: [Qemu-devel] [PATCH 0/4] Use rtc_clock uniformly for ARM

[Paolo Bonzini]

On 01/20/2012 01:06 PM, Paolo Bonzini wrote:
> This series uses rtc_clock uniformly in device models that provide RTC
> functionality.  This will let users choose the desired semantics for
> the clock.
>
> This is most important with qtest, where "-rtc clock=vm" will provide
> determinism and let you run tests that fake execution for large time
> periods.  However, for consistency I'm switching also the two RTC
> models that always used the vm_clock, m48t59 and pl031.  m48t59 is
> not ARM so I'm sending it separately.
>
> Patch 3 fixes an unrelated bug in the pl031 migration code.
>
> Paolo Bonzini (4):
>    rtc: add -rtc clock=rt
>    arm: switch real-time clocks to rtc_clock
>    pl031: rearm alarm timer upon load
>    pl031: switch clock base to rtc_clock
>
>   hw/omap1.c      |   10 +++---
>   hw/pl031.c      |   76 ++++++++++++++++++++++++++++++++++++-------------------
>   hw/pxa2xx.c     |   28 ++++++++++----------
>   hw/strongarm.c  |   10 +++---
>   hw/twl92230.c   |    9 +++---
>   qemu-options.hx |    7 +++--
>   vl.c            |    2 +
>   7 files changed, 85 insertions(+), 57 deletions(-)
>

Ping.

Paolo

Re: [Qemu-devel] [PATCH 0/4] Use rtc_clock uniformly for ARM

[Peter Maydell]

On 30 January 2012 16:42, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 01/20/2012 01:06 PM, Paolo Bonzini wrote:
>> This series uses rtc_clock uniformly in device models that provide RTC
>> functionality.  This will let users choose the desired semantics for
>> the clock.
>>
>> This is most important with qtest, where "-rtc clock=vm" will provide
>> determinism and let you run tests that fake execution for large time
>> periods.  However, for consistency I'm switching also the two RTC
>> models that always used the vm_clock, m48t59 and pl031.  m48t59 is
>> not ARM so I'm sending it separately.

> Ping.

My timer-fu is weak so I'm not going to claim to have actually
reviewed these, but they don't look to be obviously wrong.
Are you hoping for these to go in via the arm-devs tree?

You'll need to rebase following the QOM series landing.

-- PMM

Re: [Qemu-devel] [PATCH 0/4] Use rtc_clock uniformly for ARM

[Paolo Bonzini]

On 01/30/2012 06:21 PM, Peter Maydell wrote:
> My timer-fu is weak so I'm not going to claim to have actually
> reviewed these, but they don't look to be obviously wrong.
> Are you hoping for these to go in via the arm-devs tree?

Yes...

> You'll need to rebase following the QOM series landing.

Gah, right. :)  Will wait for the next QOM mass change before reposting, 
then.

Thanks!

Paolo