Patchwork [2/4] arm: switch real-time clocks to rtc_clock

login
register
mail settings
Submitter Paolo Bonzini
Date Jan. 20, 2012, 12:06 p.m.
Message ID <1327061206-9404-3-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/137017/
State New
Headers show

Comments

Paolo Bonzini - Jan. 20, 2012, 12:06 p.m.
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(-)
andrzej zaborowski - Feb. 17, 2012, 7:09 a.m.
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
Paolo Bonzini - Feb. 17, 2012, 8:05 a.m.
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
andrzej zaborowski - Feb. 17, 2012, 8:24 a.m.
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
Paolo Bonzini - Feb. 17, 2012, 8:31 a.m.
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

Patch

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);