qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer
@ 2012-08-01 16:41 Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 01/10] RTC: Remove the logic to update time format when DM bit changed Paolo Bonzini
                   ` (11 more replies)
  0 siblings, 12 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

The current RTC emulation has two timers firing every second, one
on each edge of the UIP bit.  This will prevent CPUs from staying at
deep C-states.  Intel's measurements from previous submissions show the
C6 residency reduced by 6% when running 64 idle guests.

The following patches remove the two timers.  The patches update the RTC
clock only when the guest tries to read it, and only set timers when
update or alarm is clear.  Hence, a guest will typically fire the RTC
timer only twice, respectively one second after it starts and at the
next midnight.

The patches are mostly the work of Yang Zhang.  My contribution was
limited to reorganizing them for better bisectability, and cleaning
up the computation of UIP.

A qtest for this is not as reliable as a test that actually runs code
in a VM.  A qtest is more deterministic, and the "wiggling" introduced
by running code in the VM is much more likely to find bugs.  I'll post
the unit test separately.  Because the patches also improve the quality
of the emulation, this test fails without the patches.

The first four patches are simple preparatory changes.

The fifth patch removes the timers, and replaces them with a single
timer that is fired every second until UF and AF.  The update logic is
moved to the reading of the registers, and so is UIP.

The sixth patch implements support for divider reset, which helps testing
the RTC because it places it in a known state.  The seventh patch avoids
firing the timer every second until the next alarm.

The next two patches clean up the state of the RTC to eliminate a useless
duplication, and the tenth completes migration support.  Still, backwards
migration is broken because the algorithms in the new device model are
pretty much completely different.  Downstreams that care should include
both device models and pick the old one for old machine types.

v1->v2: annotate versions correctly in the vmstate, added new
        patches to remove current_tm

Paolo Bonzini (4):
  vmstate: add VMSTATE_TIMER_V
  RTC: Do not fire timer periodically to catch next alarm
  RTC: Get and set time without going through s->current_tm
  RTC: Remove the current_tm field

Yang Zhang (6):
  RTC: Remove the logic to update time format when DM bit changed
  RTC: Rename rtc_timer_update
  RTC: Update interrupt state when interrupts are masked/unmasked
  RTC: Update the RTC clock only when reading it
  RTC: Add divider reset support
  RTC: Allow to migrate from old QEMU

 hw/mc146818rtc.c      |  540 ++++++++++++++++++++++++++++++++++---------------
 hw/mc146818rtc_regs.h |    1 +
 vmstate.h             |    5 +-
 3 files changed, 377 insertions(+), 169 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 01/10] RTC: Remove the logic to update time format when DM bit changed
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
@ 2012-08-01 16:41 ` Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 02/10] RTC: Rename rtc_timer_update Paolo Bonzini
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

From: Yang Zhang <yang.z.zhang@intel.com>

Changing the DM (binary/BCD) and 24/12 control bit doesn't affect the internal
registers. It only indicates what format is used for those registers.

Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c |   10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 3777f85..6e5b2f0 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -220,15 +220,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
                     rtc_set_time(s);
                 }
             }
-            if (((s->cmos_data[RTC_REG_B] ^ data) & (REG_B_DM | REG_B_24H)) &&
-                !(data & REG_B_SET)) {
-                /* If the time format has changed and not in set mode,
-                   update the registers immediately. */
-                s->cmos_data[RTC_REG_B] = data;
-                rtc_copy_date(s);
-            } else {
-                s->cmos_data[RTC_REG_B] = data;
-            }
+            s->cmos_data[RTC_REG_B] = data;
             rtc_timer_update(s, qemu_get_clock_ns(rtc_clock));
             break;
         case RTC_REG_C:
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 02/10] RTC: Rename rtc_timer_update
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 01/10] RTC: Remove the logic to update time format when DM bit changed Paolo Bonzini
@ 2012-08-01 16:41 ` Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 03/10] RTC: Update interrupt state when interrupts are masked/unmasked Paolo Bonzini
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

From: Yang Zhang <yang.z.zhang@intel.com>

Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 6e5b2f0..b99f4d3 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -110,7 +110,7 @@ static void rtc_coalesced_timer(void *opaque)
 }
 #endif
 
-static void rtc_timer_update(RTCState *s, int64_t current_time)
+static void periodic_timer_update(RTCState *s, int64_t current_time)
 {
     int period_code, period;
     int64_t cur_clock, next_irq_clock;
@@ -148,7 +148,7 @@ static void rtc_periodic_timer(void *opaque)
 {
     RTCState *s = opaque;
 
-    rtc_timer_update(s, s->next_periodic_time);
+    periodic_timer_update(s, s->next_periodic_time);
     s->cmos_data[RTC_REG_C] |= REG_C_PF;
     if (s->cmos_data[RTC_REG_B] & REG_B_PIE) {
         s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
@@ -207,7 +207,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
             /* UIP bit is read only */
             s->cmos_data[RTC_REG_A] = (data & ~REG_A_UIP) |
                 (s->cmos_data[RTC_REG_A] & REG_A_UIP);
-            rtc_timer_update(s, qemu_get_clock_ns(rtc_clock));
+            periodic_timer_update(s, qemu_get_clock_ns(rtc_clock));
             break;
         case RTC_REG_B:
             if (data & REG_B_SET) {
@@ -221,7 +221,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
                 }
             }
             s->cmos_data[RTC_REG_B] = data;
-            rtc_timer_update(s, qemu_get_clock_ns(rtc_clock));
+            periodic_timer_update(s, qemu_get_clock_ns(rtc_clock));
             break;
         case RTC_REG_C:
         case RTC_REG_D:
@@ -550,7 +550,7 @@ static void rtc_notify_clock_reset(Notifier *notifier, void *data)
     rtc_set_date_from_host(&s->dev);
     s->next_second_time = now + (get_ticks_per_sec() * 99) / 100;
     qemu_mod_timer(s->second_timer2, s->next_second_time);
-    rtc_timer_update(s, now);
+    periodic_timer_update(s, now);
 #ifdef TARGET_I386
     if (s->lost_tick_policy == LOST_TICK_SLEW) {
         rtc_coalesced_timer_update(s);
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 03/10] RTC: Update interrupt state when interrupts are masked/unmasked
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 01/10] RTC: Remove the logic to update time format when DM bit changed Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 02/10] RTC: Rename rtc_timer_update Paolo Bonzini
@ 2012-08-01 16:41 ` Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 06/10] vmstate: add VMSTATE_TIMER_V Paolo Bonzini
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

From: Yang Zhang <yang.z.zhang@intel.com>

If an interrupt flag is already set when the interrupt becomes enabled,
raise an interrupt immediately, and vice versa if interrupts become
disabled.

Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c      |    9 +++++++++
 hw/mc146818rtc_regs.h |    1 +
 2 files changed, 10 insertions(+)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index b99f4d3..293f174 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -220,6 +220,15 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
                     rtc_set_time(s);
                 }
             }
+            /* if an interrupt flag is already set when the interrupt
+             * becomes enabled, raise an interrupt immediately.  */
+            if (data & s->cmos_data[RTC_REG_C] & REG_C_MASK) {
+                s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
+                qemu_irq_raise(s->irq);
+            } else {
+                s->cmos_data[RTC_REG_C] &= ~REG_C_IRQF;
+                qemu_irq_lower(s->irq);
+            }
             s->cmos_data[RTC_REG_B] = data;
             periodic_timer_update(s, qemu_get_clock_ns(rtc_clock));
             break;
diff --git a/hw/mc146818rtc_regs.h b/hw/mc146818rtc_regs.h
index 3ab3770..fc10076 100644
--- a/hw/mc146818rtc_regs.h
+++ b/hw/mc146818rtc_regs.h
@@ -58,5 +58,6 @@
 #define REG_C_IRQF 0x80
 #define REG_C_PF   0x40
 #define REG_C_AF   0x20
+#define REG_C_MASK 0x70
 
 #endif
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 06/10] vmstate: add VMSTATE_TIMER_V
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
                   ` (2 preceding siblings ...)
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 03/10] RTC: Update interrupt state when interrupts are masked/unmasked Paolo Bonzini
@ 2012-08-01 16:41 ` Paolo Bonzini
  2012-08-02  8:56   ` Juan Quintela
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 05/10] RTC: Update the RTC clock only when reading it Paolo Bonzini
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

Also, for consistency with other occurrences, implement VMSTATE_TIMER
as a special case of VMSTATE_TIMER_V rather than VMSTATE_TIMER_TEST.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vmstate.h |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/vmstate.h b/vmstate.h
index 5bd2b76..092f21d 100644
--- a/vmstate.h
+++ b/vmstate.h
@@ -503,8 +503,11 @@ extern const VMStateInfo vmstate_info_unused_buffer;
 #define VMSTATE_TIMER_TEST(_f, _s, _test)                             \
     VMSTATE_POINTER_TEST(_f, _s, _test, vmstate_info_timer, QEMUTimer *)
 
+#define VMSTATE_TIMER_V(_f, _s, _v)                                   \
+    VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)
+
 #define VMSTATE_TIMER(_f, _s)                                         \
-    VMSTATE_TIMER_TEST(_f, _s, NULL)
+    VMSTATE_POINTER(_f, _s, 0, vmstate_info_timer, QEMUTimer *)
 
 #define VMSTATE_TIMER_ARRAY(_f, _s, _n)                              \
     VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *)
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 05/10] RTC: Update the RTC clock only when reading it
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
                   ` (3 preceding siblings ...)
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 06/10] vmstate: add VMSTATE_TIMER_V Paolo Bonzini
@ 2012-08-01 16:41 ` Paolo Bonzini
  2012-08-01 19:48   ` Anthony Liguori
  2012-08-02  9:09   ` Juan Quintela
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 06/10] RTC: Add divider reset support Paolo Bonzini
                   ` (6 subsequent siblings)
  11 siblings, 2 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

From: Yang Zhang <yang.z.zhang@intel.com>

Calculate guest RTC based on the time of the last update, instead of
using timers.  The formula is

    (base_rtc + guest_time_now - guest_time_last_update + offset)

Base_rtc is the RTC value when the RTC was last updated.
Guest_time_now is the guest time when the access happens.
Guest_time_last_update was the guest time when the RTC was last updated.
Offset is used when divider reset happens or the set bit is toggled.

The timer is kept in order to signal interrupts, but it only needs to
run when either UF or AF is cleared.  When the bits are both set, the
timer does not run.

UIP is now synthesized when reading register A.  If the timer is not set,
or if there is more than one second before it (as is the case at the
end of this series), the leading edge of UIP is computed and the rising
edge occurs 220us later.  If the update timer occurs within one second,
however, the rising edge of the AF and UF bits should coincide withe
the falling edge of UIP.  We do not know exactly when this will happen
because there could be delays in the servicing of the timer.  Hence, in
this case reading register A only computes for the rising edge of UIP,
and latches the bit until the timer is fired and clears it.

Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c |  321 +++++++++++++++++++++++++++++++-----------------------
 1 file changed, 186 insertions(+), 135 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 293f174..2c34a82 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -45,6 +45,8 @@
 # define DPRINTF_C(format, ...)      do { } while (0)
 #endif
 
+#define NSEC_PER_SEC    1000000000LL
+
 #define RTC_REINJECT_ON_ACK_COUNT 20
 
 typedef struct RTCState {
@@ -54,27 +56,40 @@ typedef struct RTCState {
     uint8_t cmos_index;
     struct tm current_tm;
     int32_t base_year;
+    uint64_t base_rtc;
+    uint64_t last_update;
+    int64_t offset;
     qemu_irq irq;
     qemu_irq sqw_irq;
     int it_shift;
     /* periodic timer */
     QEMUTimer *periodic_timer;
     int64_t next_periodic_time;
-    /* second update */
-    int64_t next_second_time;
+    /* update-ended timer */
+    QEMUTimer *update_timer;
     uint16_t irq_reinject_on_ack_count;
     uint32_t irq_coalesced;
     uint32_t period;
     QEMUTimer *coalesced_timer;
-    QEMUTimer *second_timer;
-    QEMUTimer *second_timer2;
     Notifier clock_reset_notifier;
     LostTickPolicy lost_tick_policy;
     Notifier suspend_notifier;
 } RTCState;
 
 static void rtc_set_time(RTCState *s);
-static void rtc_copy_date(RTCState *s);
+static void rtc_update_time(RTCState *s);
+static void rtc_set_cmos(RTCState *s);
+static inline int rtc_from_bcd(RTCState *s, int a);
+
+static uint64_t get_guest_rtc_ns(RTCState *s)
+{
+    uint64_t guest_rtc;
+    uint64_t guest_clock = qemu_get_clock_ns(rtc_clock);
+
+    guest_rtc = s->base_rtc * NSEC_PER_SEC
+                 + guest_clock - s->last_update + s->offset;
+    return guest_rtc;
+}
 
 #ifdef TARGET_I386
 static void rtc_coalesced_timer_update(RTCState *s)
@@ -110,6 +125,7 @@ static void rtc_coalesced_timer(void *opaque)
 }
 #endif
 
+/* handle periodic timer */
 static void periodic_timer_update(RTCState *s, int64_t current_time)
 {
     int period_code, period;
@@ -175,6 +191,100 @@ static void rtc_periodic_timer(void *opaque)
     }
 }
 
+/* handle update-ended timer */
+static void check_update_timer(RTCState *s)
+{
+    uint64_t next_update_time;
+    uint64_t guest_nsec;
+
+    /* From the data sheet: setting the SET bit does not prevent
+     * interrupts from occurring!  However, it will prevent an
+     * alarm interrupt from occurring, because the time of day is
+     * not updated.
+     */
+    if ((s->cmos_data[RTC_REG_C] & REG_C_UF) &&
+        (s->cmos_data[RTC_REG_B] & REG_B_SET)) {
+        qemu_del_timer(s->update_timer);
+        return;
+    }
+    if ((s->cmos_data[RTC_REG_C] & REG_C_UF) &&
+        (s->cmos_data[RTC_REG_C] & REG_C_AF)) {
+        qemu_del_timer(s->update_timer);
+        return;
+    }
+
+    guest_nsec = get_guest_rtc_ns(s) % NSEC_PER_SEC;
+    /* reprogram to next second */
+    next_update_time = qemu_get_clock_ns(rtc_clock)
+        + NSEC_PER_SEC - guest_nsec;
+    if (next_update_time != qemu_timer_expire_time_ns(s->update_timer)) {
+        qemu_mod_timer(s->update_timer, next_update_time);
+    }
+}
+
+static inline uint8_t convert_hour(RTCState *s, uint8_t hour)
+{
+    if (!(s->cmos_data[RTC_REG_B] & REG_B_24H)) {
+        hour %= 12;
+        if (s->cmos_data[RTC_HOURS] & 0x80) {
+            hour += 12;
+        }
+    }
+    return hour;
+}
+
+static uint32_t check_alarm(RTCState *s)
+{
+    uint8_t alarm_hour, alarm_min, alarm_sec;
+    uint8_t cur_hour, cur_min, cur_sec;
+
+    alarm_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS_ALARM]);
+    alarm_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES_ALARM]);
+    alarm_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS_ALARM]);
+    alarm_hour = convert_hour(s, alarm_hour);
+
+    cur_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS]);
+    cur_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES]);
+    cur_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS]);
+    cur_hour = convert_hour(s, cur_hour);
+
+    if (((s->cmos_data[RTC_SECONDS_ALARM] & 0xc0) == 0xc0
+                || alarm_sec == cur_sec) &&
+            ((s->cmos_data[RTC_MINUTES_ALARM] & 0xc0) == 0xc0
+             || alarm_min == cur_min) &&
+            ((s->cmos_data[RTC_HOURS_ALARM] & 0xc0) == 0xc0
+             || alarm_hour == cur_hour)) {
+        return 1;
+    }
+    return 0;
+
+}
+
+static void rtc_update_timer(void *opaque)
+{
+    RTCState *s = opaque;
+    int32_t irqs = REG_C_UF;
+    int32_t new_irqs;
+
+    /* UIP might have been latched, update time and clear it.  */
+    rtc_update_time(s);
+    s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
+
+    if (check_alarm(s)) {
+        irqs |= REG_C_AF;
+        if (s->cmos_data[RTC_REG_B] & REG_B_AIE) {
+            qemu_system_wakeup_request(QEMU_WAKEUP_REASON_RTC);
+        }
+    }
+    new_irqs = irqs & ~s->cmos_data[RTC_REG_C];
+    s->cmos_data[RTC_REG_C] |= irqs;
+    if ((new_irqs & s->cmos_data[RTC_REG_B]) != 0) {
+        s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
+        qemu_irq_raise(s->irq);
+    }
+    check_update_timer(s);
+}
+
 static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
 {
     RTCState *s = opaque;
@@ -189,6 +299,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
         case RTC_MINUTES_ALARM:
         case RTC_HOURS_ALARM:
             s->cmos_data[s->cmos_index] = data;
+            check_update_timer(s);
             break;
         case RTC_SECONDS:
         case RTC_MINUTES:
@@ -201,6 +312,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
             /* if in set mode, do not update the time */
             if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
                 rtc_set_time(s);
+                check_update_timer(s);
             }
             break;
         case RTC_REG_A:
@@ -208,15 +320,21 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
             s->cmos_data[RTC_REG_A] = (data & ~REG_A_UIP) |
                 (s->cmos_data[RTC_REG_A] & REG_A_UIP);
             periodic_timer_update(s, qemu_get_clock_ns(rtc_clock));
+            check_update_timer(s);
             break;
         case RTC_REG_B:
             if (data & REG_B_SET) {
+                /* update cmos to when the rtc was stopping */
+                if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
+                    rtc_update_time(s);
+                }
                 /* set mode: reset UIP mode */
                 s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
                 data &= ~REG_B_UIE;
             } else {
                 /* if disabling set mode, update the time */
                 if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
+                    s->offset = get_guest_rtc_ns(s) % NSEC_PER_SEC;
                     rtc_set_time(s);
                 }
             }
@@ -231,6 +349,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
             }
             s->cmos_data[RTC_REG_B] = data;
             periodic_timer_update(s, qemu_get_clock_ns(rtc_clock));
+            check_update_timer(s);
             break;
         case RTC_REG_C:
         case RTC_REG_D:
@@ -279,10 +398,13 @@ static void rtc_set_time(RTCState *s)
     tm->tm_mon = rtc_from_bcd(s, s->cmos_data[RTC_MONTH]) - 1;
     tm->tm_year = rtc_from_bcd(s, s->cmos_data[RTC_YEAR]) + s->base_year - 1900;
 
+    s->base_rtc = mktimegm(tm);
+    s->last_update = qemu_get_clock_ns(rtc_clock);
+
     rtc_change_mon_event(tm);
 }
 
-static void rtc_copy_date(RTCState *s)
+static void rtc_set_cmos(RTCState *s)
 {
     const struct tm *tm = &s->current_tm;
     int year;
@@ -308,122 +430,41 @@ static void rtc_copy_date(RTCState *s)
     s->cmos_data[RTC_YEAR] = rtc_to_bcd(s, year);
 }
 
-/* month is between 0 and 11. */
-static int get_days_in_month(int month, int year)
+static void rtc_update_time(RTCState *s)
 {
-    static const int days_tab[12] = {
-        31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
-    };
-    int d;
-    if ((unsigned )month >= 12)
-        return 31;
-    d = days_tab[month];
-    if (month == 1) {
-        if ((year % 4) == 0 && ((year % 100) != 0 || (year % 400) == 0))
-            d++;
-    }
-    return d;
+    struct tm *ret;
+    time_t guest_sec;
+    int64_t guest_nsec;
+
+    guest_nsec = get_guest_rtc_ns(s);
+    guest_sec = guest_nsec / NSEC_PER_SEC;
+    ret = gmtime(&guest_sec);
+    s->current_tm = *ret;
+    rtc_set_cmos(s);
 }
 
-/* update 'tm' to the next second */
-static void rtc_next_second(struct tm *tm)
+static int update_in_progress(RTCState *s)
 {
-    int days_in_month;
-
-    tm->tm_sec++;
-    if ((unsigned)tm->tm_sec >= 60) {
-        tm->tm_sec = 0;
-        tm->tm_min++;
-        if ((unsigned)tm->tm_min >= 60) {
-            tm->tm_min = 0;
-            tm->tm_hour++;
-            if ((unsigned)tm->tm_hour >= 24) {
-                tm->tm_hour = 0;
-                /* next day */
-                tm->tm_wday++;
-                if ((unsigned)tm->tm_wday >= 7)
-                    tm->tm_wday = 0;
-                days_in_month = get_days_in_month(tm->tm_mon,
-                                                  tm->tm_year + 1900);
-                tm->tm_mday++;
-                if (tm->tm_mday < 1) {
-                    tm->tm_mday = 1;
-                } else if (tm->tm_mday > days_in_month) {
-                    tm->tm_mday = 1;
-                    tm->tm_mon++;
-                    if (tm->tm_mon >= 12) {
-                        tm->tm_mon = 0;
-                        tm->tm_year++;
-                    }
-                }
-            }
-        }
-    }
-}
-
-
-static void rtc_update_second(void *opaque)
-{
-    RTCState *s = opaque;
-    int64_t delay;
-
-    /* if the oscillator is not in normal operation, we do not update */
-    if ((s->cmos_data[RTC_REG_A] & 0x70) != 0x20) {
-        s->next_second_time += get_ticks_per_sec();
-        qemu_mod_timer(s->second_timer, s->next_second_time);
-    } else {
-        rtc_next_second(&s->current_tm);
-
-        if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
-            /* update in progress bit */
-            s->cmos_data[RTC_REG_A] |= REG_A_UIP;
-        }
-        /* should be 244 us = 8 / 32768 seconds, but currently the
-           timers do not have the necessary resolution. */
-        delay = (get_ticks_per_sec() * 1) / 100;
-        if (delay < 1)
-            delay = 1;
-        qemu_mod_timer(s->second_timer2,
-                       s->next_second_time + delay);
-    }
-}
-
-static void rtc_update_second2(void *opaque)
-{
-    RTCState *s = opaque;
+    int64_t guest_nsec;
 
-    if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
-        rtc_copy_date(s);
+    if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
+        return 0;
     }
-
-    /* check alarm */
-    if (((s->cmos_data[RTC_SECONDS_ALARM] & 0xc0) == 0xc0 ||
-         rtc_from_bcd(s, s->cmos_data[RTC_SECONDS_ALARM]) == s->current_tm.tm_sec) &&
-        ((s->cmos_data[RTC_MINUTES_ALARM] & 0xc0) == 0xc0 ||
-         rtc_from_bcd(s, s->cmos_data[RTC_MINUTES_ALARM]) == s->current_tm.tm_min) &&
-        ((s->cmos_data[RTC_HOURS_ALARM] & 0xc0) == 0xc0 ||
-         rtc_from_bcd(s, s->cmos_data[RTC_HOURS_ALARM]) == s->current_tm.tm_hour)) {
-
-        s->cmos_data[RTC_REG_C] |= REG_C_AF;
-        if (s->cmos_data[RTC_REG_B] & REG_B_AIE) {
-            qemu_system_wakeup_request(QEMU_WAKEUP_REASON_RTC);
-            qemu_irq_raise(s->irq);
-            s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
+    if (qemu_timer_pending(s->update_timer)) {
+        int64_t next_update_time = qemu_timer_expire_time_ns(s->update_timer);
+        /* Latch UIP until the timer expires.  */
+        if (qemu_get_clock_ns(rtc_clock) >= (next_update_time - 244000)) {
+            s->cmos_data[RTC_REG_A] |= REG_A_UIP;
+            return 1;
         }
     }
 
-    /* update ended interrupt */
-    s->cmos_data[RTC_REG_C] |= REG_C_UF;
-    if (s->cmos_data[RTC_REG_B] & REG_B_UIE) {
-        s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
-        qemu_irq_raise(s->irq);
+    guest_nsec = get_guest_rtc_ns(s);
+    /* UIP bit will be set at last 244us of every second. */
+    if ((guest_nsec % NSEC_PER_SEC) >= (NSEC_PER_SEC - 244000)) {
+        return 1;
     }
-
-    /* clear update in progress bit */
-    s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
-
-    s->next_second_time += get_ticks_per_sec();
-    qemu_mod_timer(s->second_timer, s->next_second_time);
+    return 0;
 }
 
 static uint32_t cmos_ioport_read(void *opaque, uint32_t addr)
@@ -441,15 +482,28 @@ static uint32_t cmos_ioport_read(void *opaque, uint32_t addr)
         case RTC_DAY_OF_MONTH:
         case RTC_MONTH:
         case RTC_YEAR:
+            /* if not in set mode, calibrate cmos before
+             * reading*/
+            if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
+                rtc_update_time(s);
+            }
             ret = s->cmos_data[s->cmos_index];
             break;
         case RTC_REG_A:
+            if (update_in_progress(s)) {
+                s->cmos_data[s->cmos_index] |= REG_A_UIP;
+            } else {
+                s->cmos_data[s->cmos_index] &= ~REG_A_UIP;
+            }
             ret = s->cmos_data[s->cmos_index];
             break;
         case RTC_REG_C:
             ret = s->cmos_data[s->cmos_index];
             qemu_irq_lower(s->irq);
             s->cmos_data[RTC_REG_C] = 0x00;
+            if (ret & (REG_C_UF | REG_C_AF)) {
+                check_update_timer(s);
+            }
 #ifdef TARGET_I386
             if(s->irq_coalesced &&
                     (s->cmos_data[RTC_REG_B] & REG_B_PIE) &&
@@ -484,13 +538,6 @@ void rtc_set_memory(ISADevice *dev, int addr, int val)
         s->cmos_data[addr] = val;
 }
 
-void rtc_set_date(ISADevice *dev, const struct tm *tm)
-{
-    RTCState *s = DO_UPCAST(RTCState, dev, dev);
-    s->current_tm = *tm;
-    rtc_copy_date(s);
-}
-
 /* PC cmos mappings */
 #define REG_IBM_CENTURY_BYTE        0x32
 #define REG_IBM_PS2_CENTURY_BYTE    0x37
@@ -501,9 +548,15 @@ static void rtc_set_date_from_host(ISADevice *dev)
     struct tm tm;
     int val;
 
-    /* set the CMOS date */
     qemu_get_timedate(&tm, 0);
-    rtc_set_date(dev, &tm);
+
+    s->base_rtc = mktimegm(&tm);
+    s->last_update = qemu_get_clock_ns(rtc_clock);
+    s->offset = 0;
+
+    /* set the CMOS date */
+    s->current_tm = tm;
+    rtc_set_cmos(s);
 
     val = rtc_to_bcd(s, (tm.tm_year / 100) + 19);
     rtc_set_memory(dev, REG_IBM_CENTURY_BYTE, val);
@@ -526,9 +579,9 @@ static int rtc_post_load(void *opaque, int version_id)
 
 static const VMStateDescription vmstate_rtc = {
     .name = "mc146818rtc",
-    .version_id = 2,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
+    .version_id = 3,
+    .minimum_version_id = 3,
+    .minimum_version_id_old = 3,
     .post_load = rtc_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_BUFFER(cmos_data, RTCState),
@@ -542,11 +595,12 @@ static const VMStateDescription vmstate_rtc = {
         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_UINT64_V(base_rtc, RTCState, 3),
+        VMSTATE_UINT64_V(last_update, RTCState, 3),
+        VMSTATE_INT64_V(offset, RTCState, 3),
+        VMSTATE_TIMER_V(update_timer, RTCState, 3),
         VMSTATE_END_OF_LIST()
     }
 };
@@ -557,9 +611,8 @@ static void rtc_notify_clock_reset(Notifier *notifier, void *data)
     int64_t now = *(int64_t *)data;
 
     rtc_set_date_from_host(&s->dev);
-    s->next_second_time = now + (get_ticks_per_sec() * 99) / 100;
-    qemu_mod_timer(s->second_timer2, s->next_second_time);
     periodic_timer_update(s, now);
+    check_update_timer(s);
 #ifdef TARGET_I386
     if (s->lost_tick_policy == LOST_TICK_SLEW) {
         rtc_coalesced_timer_update(s);
@@ -581,6 +634,7 @@ static void rtc_reset(void *opaque)
 
     s->cmos_data[RTC_REG_B] &= ~(REG_B_PIE | REG_B_AIE | REG_B_SQWE);
     s->cmos_data[RTC_REG_C] &= ~(REG_C_UF | REG_C_IRQF | REG_C_PF | REG_C_AF);
+    check_update_timer(s);
 
     qemu_irq_lower(s->irq);
 
@@ -606,6 +660,7 @@ static void rtc_get_date(Object *obj, Visitor *v, void *opaque,
     ISADevice *isa = ISA_DEVICE(obj);
     RTCState *s = DO_UPCAST(RTCState, dev, isa);
 
+    rtc_update_time(s);
     visit_start_struct(v, NULL, "struct tm", name, 0, errp);
     visit_type_int32(v, &s->current_tm.tm_year, "tm_year", errp);
     visit_type_int32(v, &s->current_tm.tm_mon, "tm_mon", errp);
@@ -642,8 +697,8 @@ static int rtc_initfn(ISADevice *dev)
 #endif
 
     s->periodic_timer = qemu_new_timer_ns(rtc_clock, rtc_periodic_timer, s);
-    s->second_timer = qemu_new_timer_ns(rtc_clock, rtc_update_second, s);
-    s->second_timer2 = qemu_new_timer_ns(rtc_clock, rtc_update_second2, s);
+    s->update_timer = qemu_new_timer_ns(rtc_clock, rtc_update_timer, s);
+    check_update_timer(s);
 
     s->clock_reset_notifier.notify = rtc_notify_clock_reset;
     qemu_register_clock_reset_notifier(rtc_clock, &s->clock_reset_notifier);
@@ -651,10 +706,6 @@ static int rtc_initfn(ISADevice *dev)
     s->suspend_notifier.notify = rtc_notify_suspend;
     qemu_register_suspend_notifier(&s->suspend_notifier);
 
-    s->next_second_time =
-        qemu_get_clock_ns(rtc_clock) + (get_ticks_per_sec() * 99) / 100;
-    qemu_mod_timer(s->second_timer2, s->next_second_time);
-
     memory_region_init_io(&s->io, &cmos_ops, s, "rtc", 2);
     isa_register_ioport(dev, &s->io, base);
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 06/10] RTC: Add divider reset support
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
                   ` (4 preceding siblings ...)
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 05/10] RTC: Update the RTC clock only when reading it Paolo Bonzini
@ 2012-08-01 16:41 ` Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 07/10] RTC: Do not fire timer periodically to catch next alarm Paolo Bonzini
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

From: Yang Zhang <yang.z.zhang@intel.com>

The first update cycle begins one-half seconds after divider
reset is removed.  This feature is useful for testing.

Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c |   50 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 2c34a82..1018eb9 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -81,6 +81,12 @@ static void rtc_update_time(RTCState *s);
 static void rtc_set_cmos(RTCState *s);
 static inline int rtc_from_bcd(RTCState *s, int a);
 
+static inline bool rtc_running(RTCState *s)
+{
+    return (!(s->cmos_data[RTC_REG_B] & REG_B_SET) &&
+            (s->cmos_data[RTC_REG_A] & 0x70) <= 0x20);
+}
+
 static uint64_t get_guest_rtc_ns(RTCState *s)
 {
     uint64_t guest_rtc;
@@ -197,11 +203,15 @@ static void check_update_timer(RTCState *s)
     uint64_t next_update_time;
     uint64_t guest_nsec;
 
-    /* From the data sheet: setting the SET bit does not prevent
-     * interrupts from occurring!  However, it will prevent an
-     * alarm interrupt from occurring, because the time of day is
-     * not updated.
+    /* From the data sheet: "Holding the dividers in reset prevents
+     * interrupts from operating, while setting the SET bit allows"
+     * them to occur.  However, it will prevent an alarm interrupt
+     * from occurring, because the time of day is not updated.
      */
+    if ((s->cmos_data[RTC_REG_A] & 0x60) == 0x60) {
+        qemu_del_timer(s->update_timer);
+        return;
+    }
     if ((s->cmos_data[RTC_REG_C] & REG_C_UF) &&
         (s->cmos_data[RTC_REG_B] & REG_B_SET)) {
         qemu_del_timer(s->update_timer);
@@ -266,6 +276,8 @@ static void rtc_update_timer(void *opaque)
     int32_t irqs = REG_C_UF;
     int32_t new_irqs;
 
+    assert((s->cmos_data[RTC_REG_A] & 0x60) != 0x60);
+
     /* UIP might have been latched, update time and clear it.  */
     rtc_update_time(s);
     s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
@@ -310,12 +322,31 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
         case RTC_YEAR:
             s->cmos_data[s->cmos_index] = data;
             /* if in set mode, do not update the time */
-            if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
+            if (rtc_running(s)) {
                 rtc_set_time(s);
                 check_update_timer(s);
             }
             break;
         case RTC_REG_A:
+            if ((data & 0x60) == 0x60) {
+                if (rtc_running(s)) {
+                    rtc_update_time(s);
+                }
+                /* What happens to UIP when divider reset is enabled is
+                 * unclear from the datasheet.  Shouldn't matter much
+                 * though.
+                 */
+                s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
+            } else if (((s->cmos_data[RTC_REG_A] & 0x60) == 0x60) &&
+                    (data & 0x70)  <= 0x20) {
+                /* when the divider reset is removed, the first update cycle
+                 * begins one-half second later*/
+                if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
+                    s->offset = 500000000;
+                    rtc_set_time(s);
+                }
+                s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
+            }
             /* UIP bit is read only */
             s->cmos_data[RTC_REG_A] = (data & ~REG_A_UIP) |
                 (s->cmos_data[RTC_REG_A] & REG_A_UIP);
@@ -325,7 +356,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
         case RTC_REG_B:
             if (data & REG_B_SET) {
                 /* update cmos to when the rtc was stopping */
-                if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
+                if (rtc_running(s)) {
                     rtc_update_time(s);
                 }
                 /* set mode: reset UIP mode */
@@ -333,7 +364,8 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
                 data &= ~REG_B_UIE;
             } else {
                 /* if disabling set mode, update the time */
-                if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
+                if ((s->cmos_data[RTC_REG_B] & REG_B_SET) &&
+                    (s->cmos_data[RTC_REG_A] & 0x70) <= 0x20) {
                     s->offset = get_guest_rtc_ns(s) % NSEC_PER_SEC;
                     rtc_set_time(s);
                 }
@@ -447,7 +479,7 @@ static int update_in_progress(RTCState *s)
 {
     int64_t guest_nsec;
 
-    if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
+    if (!rtc_running(s)) {
         return 0;
     }
     if (qemu_timer_pending(s->update_timer)) {
@@ -484,7 +516,7 @@ static uint32_t cmos_ioport_read(void *opaque, uint32_t addr)
         case RTC_YEAR:
             /* if not in set mode, calibrate cmos before
              * reading*/
-            if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
+            if (rtc_running(s)) {
                 rtc_update_time(s);
             }
             ret = s->cmos_data[s->cmos_index];
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 07/10] RTC: Do not fire timer periodically to catch next alarm
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
                   ` (5 preceding siblings ...)
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 06/10] RTC: Add divider reset support Paolo Bonzini
@ 2012-08-01 16:41 ` Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 08/10] RTC: Get and set time without going through s->current_tm Paolo Bonzini
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

This patch limits further the usage of a periodic timer.  It computes the
time of the next alarm, and uses it to skip all intermediate occurrences
of the timer.

Cc: Yang Zhang <yang.z.zhang@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c |  117 +++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 103 insertions(+), 14 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 1018eb9..345886a 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -46,6 +46,11 @@
 #endif
 
 #define NSEC_PER_SEC    1000000000LL
+#define SEC_PER_MIN     60
+#define MIN_PER_HOUR    60
+#define SEC_PER_HOUR    3600
+#define HOUR_PER_DAY    24
+#define SEC_PER_DAY     86400
 
 #define RTC_REINJECT_ON_ACK_COUNT 20
 
@@ -67,6 +72,7 @@ typedef struct RTCState {
     int64_t next_periodic_time;
     /* update-ended timer */
     QEMUTimer *update_timer;
+    uint64_t next_alarm_time;
     uint16_t irq_reinject_on_ack_count;
     uint32_t irq_coalesced;
     uint32_t period;
@@ -80,6 +86,7 @@ static void rtc_set_time(RTCState *s);
 static void rtc_update_time(RTCState *s);
 static void rtc_set_cmos(RTCState *s);
 static inline int rtc_from_bcd(RTCState *s, int a);
+static uint64_t get_next_alarm(RTCState *s);
 
 static inline bool rtc_running(RTCState *s)
 {
@@ -202,6 +209,7 @@ static void check_update_timer(RTCState *s)
 {
     uint64_t next_update_time;
     uint64_t guest_nsec;
+    int next_alarm_sec;
 
     /* From the data sheet: "Holding the dividers in reset prevents
      * interrupts from operating, while setting the SET bit allows"
@@ -224,9 +232,21 @@ static void check_update_timer(RTCState *s)
     }
 
     guest_nsec = get_guest_rtc_ns(s) % NSEC_PER_SEC;
-    /* reprogram to next second */
+    /* if UF is clear, reprogram to next second */
     next_update_time = qemu_get_clock_ns(rtc_clock)
         + NSEC_PER_SEC - guest_nsec;
+
+    /* Compute time of next alarm.  One second is already accounted
+     * for in next_update_time.
+     */
+    next_alarm_sec = get_next_alarm(s);
+    s->next_alarm_time = next_update_time + (next_alarm_sec - 1) * NSEC_PER_SEC;
+
+    if (s->cmos_data[RTC_REG_C] & REG_C_UF) {
+        /* UF is set, but AF is clear.  Program the timer to target
+         * the alarm time.  */
+        next_update_time = s->next_alarm_time;
+    }
     if (next_update_time != qemu_timer_expire_time_ns(s->update_timer)) {
         qemu_mod_timer(s->update_timer, next_update_time);
     }
@@ -243,31 +263,95 @@ static inline uint8_t convert_hour(RTCState *s, uint8_t hour)
     return hour;
 }
 
-static uint32_t check_alarm(RTCState *s)
+static uint64_t get_next_alarm(RTCState *s)
 {
-    uint8_t alarm_hour, alarm_min, alarm_sec;
-    uint8_t cur_hour, cur_min, cur_sec;
+    int32_t alarm_sec, alarm_min, alarm_hour, cur_hour, cur_min, cur_sec;
+    int32_t hour, min, sec;
+
+    rtc_update_time(s);
 
     alarm_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS_ALARM]);
     alarm_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES_ALARM]);
     alarm_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS_ALARM]);
-    alarm_hour = convert_hour(s, alarm_hour);
+    alarm_hour = alarm_hour == -1 ? -1 : convert_hour(s, alarm_hour);
 
     cur_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS]);
     cur_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES]);
     cur_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS]);
     cur_hour = convert_hour(s, cur_hour);
 
-    if (((s->cmos_data[RTC_SECONDS_ALARM] & 0xc0) == 0xc0
-                || alarm_sec == cur_sec) &&
-            ((s->cmos_data[RTC_MINUTES_ALARM] & 0xc0) == 0xc0
-             || alarm_min == cur_min) &&
-            ((s->cmos_data[RTC_HOURS_ALARM] & 0xc0) == 0xc0
-             || alarm_hour == cur_hour)) {
-        return 1;
+    if (alarm_hour == -1) {
+        alarm_hour = cur_hour;
+        if (alarm_min == -1) {
+            alarm_min = cur_min;
+            if (alarm_sec == -1) {
+                alarm_sec = cur_sec + 1;
+            } else if (cur_sec > alarm_sec) {
+                alarm_min++;
+            }
+        } else if (cur_min == alarm_min) {
+            if (alarm_sec == -1) {
+                alarm_sec = cur_sec + 1;
+            } else {
+                if (cur_sec > alarm_sec) {
+                    alarm_hour++;
+                }
+            }
+            if (alarm_sec == SEC_PER_MIN) {
+                /* wrap to next hour, minutes is not in don't care mode */
+                alarm_sec = 0;
+                alarm_hour++;
+            }
+        } else if (cur_min > alarm_min) {
+            alarm_hour++;
+        }
+    } else if (cur_hour == alarm_hour) {
+        if (alarm_min == -1) {
+            alarm_min = cur_min;
+            if (alarm_sec == -1) {
+                alarm_sec = cur_sec + 1;
+            } else if (cur_sec > alarm_sec) {
+                alarm_min++;
+            }
+
+            if (alarm_sec == SEC_PER_MIN) {
+                alarm_sec = 0;
+                alarm_min++;
+            }
+            /* wrap to next day, hour is not in don't care mode */
+            alarm_min %= MIN_PER_HOUR;
+        } else if (cur_min == alarm_min) {
+            if (alarm_sec == -1) {
+                alarm_sec = cur_sec + 1;
+            }
+            /* wrap to next day, hours+minutes not in don't care mode */
+            alarm_sec %= SEC_PER_MIN;
+        }
     }
-    return 0;
 
+    /* values that are still don't care fire at the next min/sec */
+    if (alarm_min == -1) {
+        alarm_min = 0;
+    }
+    if (alarm_sec == -1) {
+        alarm_sec = 0;
+    }
+
+    /* keep values in range */
+    if (alarm_sec == SEC_PER_MIN) {
+        alarm_sec = 0;
+        alarm_min++;
+    }
+    if (alarm_min == MIN_PER_HOUR) {
+        alarm_min = 0;
+        alarm_hour++;
+    }
+    alarm_hour %= HOUR_PER_DAY;
+
+    hour = alarm_hour - cur_hour;
+    min = hour * MIN_PER_HOUR + alarm_min - cur_min;
+    sec = min * SEC_PER_MIN + alarm_sec - cur_sec;
+    return sec <= 0 ? sec + SEC_PER_DAY : sec;
 }
 
 static void rtc_update_timer(void *opaque)
@@ -282,12 +366,13 @@ static void rtc_update_timer(void *opaque)
     rtc_update_time(s);
     s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
 
-    if (check_alarm(s)) {
+    if (qemu_get_clock_ns(rtc_clock) >= s->next_alarm_time) {
         irqs |= REG_C_AF;
         if (s->cmos_data[RTC_REG_B] & REG_B_AIE) {
             qemu_system_wakeup_request(QEMU_WAKEUP_REASON_RTC);
         }
     }
+
     new_irqs = irqs & ~s->cmos_data[RTC_REG_C];
     s->cmos_data[RTC_REG_C] |= irqs;
     if ((new_irqs & s->cmos_data[RTC_REG_B]) != 0) {
@@ -405,6 +490,9 @@ static inline int rtc_to_bcd(RTCState *s, int a)
 
 static inline int rtc_from_bcd(RTCState *s, int a)
 {
+    if ((a & 0xc0) == 0xc0) {
+        return -1;
+    }
     if (s->cmos_data[RTC_REG_B] & REG_B_DM) {
         return a;
     } else {
@@ -633,6 +721,7 @@ static const VMStateDescription vmstate_rtc = {
         VMSTATE_UINT64_V(last_update, RTCState, 3),
         VMSTATE_INT64_V(offset, RTCState, 3),
         VMSTATE_TIMER_V(update_timer, RTCState, 3),
+        VMSTATE_UINT64_V(next_alarm_time, RTCState, 3),
         VMSTATE_END_OF_LIST()
     }
 };
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 08/10] RTC: Get and set time without going through s->current_tm
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
                   ` (6 preceding siblings ...)
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 07/10] RTC: Do not fire timer periodically to catch next alarm Paolo Bonzini
@ 2012-08-01 16:41 ` Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 09/10] RTC: Remove the current_tm field Paolo Bonzini
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

This patch makes rtc_set_time and rtc_set_cmos work without reading
s->current_tm.  In the case of rtc_set_time I introduce a new
function that retrieves the time and stores into a given struct tm
(not hard-coded to s->current_tm).  In the case of rtc_set_cmos, the
current time is similarly taken from a struct tm rather than
s->current_tm.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c |   38 ++++++++++++++++++++++----------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 345886a..a69ddc3 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -84,7 +84,7 @@ typedef struct RTCState {
 
 static void rtc_set_time(RTCState *s);
 static void rtc_update_time(RTCState *s);
-static void rtc_set_cmos(RTCState *s);
+static void rtc_set_cmos(RTCState *s, const struct tm *tm);
 static inline int rtc_from_bcd(RTCState *s, int a);
 static uint64_t get_next_alarm(RTCState *s);
 
@@ -500,10 +500,8 @@ static inline int rtc_from_bcd(RTCState *s, int a)
     }
 }
 
-static void rtc_set_time(RTCState *s)
+static void rtc_get_time(RTCState *s, struct tm *tm)
 {
-    struct tm *tm = &s->current_tm;
-
     tm->tm_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS]);
     tm->tm_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES]);
     tm->tm_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS] & 0x7f);
@@ -517,16 +515,22 @@ static void rtc_set_time(RTCState *s)
     tm->tm_mday = rtc_from_bcd(s, s->cmos_data[RTC_DAY_OF_MONTH]);
     tm->tm_mon = rtc_from_bcd(s, s->cmos_data[RTC_MONTH]) - 1;
     tm->tm_year = rtc_from_bcd(s, s->cmos_data[RTC_YEAR]) + s->base_year - 1900;
+}
 
-    s->base_rtc = mktimegm(tm);
+static void rtc_set_time(RTCState *s)
+{
+    struct tm tm;
+
+    rtc_get_time(s, &tm);
+    s->current_tm = *tm;
+    s->base_rtc = mktimegm(&tm);
     s->last_update = qemu_get_clock_ns(rtc_clock);
 
-    rtc_change_mon_event(tm);
+    rtc_change_mon_event(&tm);
 }
 
-static void rtc_set_cmos(RTCState *s)
+static void rtc_set_cmos(RTCState *s, const struct tm *tm)
 {
-    const struct tm *tm = &s->current_tm;
     int year;
 
     s->cmos_data[RTC_SECONDS] = rtc_to_bcd(s, tm->tm_sec);
@@ -559,8 +563,8 @@ static void rtc_update_time(RTCState *s)
     guest_nsec = get_guest_rtc_ns(s);
     guest_sec = guest_nsec / NSEC_PER_SEC;
     ret = gmtime(&guest_sec);
+    rtc_set_cmos(s, ret);
     s->current_tm = *ret;
-    rtc_set_cmos(s);
 }
 
 static int update_in_progress(RTCState *s)
@@ -675,8 +679,8 @@ static void rtc_set_date_from_host(ISADevice *dev)
     s->offset = 0;
 
     /* set the CMOS date */
+    rtc_set_cmos(s, &tm);
     s->current_tm = tm;
-    rtc_set_cmos(s);
 
     val = rtc_to_bcd(s, (tm.tm_year / 100) + 19);
     rtc_set_memory(dev, REG_IBM_CENTURY_BYTE, val);
@@ -780,15 +784,17 @@ static void rtc_get_date(Object *obj, Visitor *v, void *opaque,
 {
     ISADevice *isa = ISA_DEVICE(obj);
     RTCState *s = DO_UPCAST(RTCState, dev, isa);
+    struct tm current_tm;
 
     rtc_update_time(s);
+    rtc_get_time(s, &current_tm);
     visit_start_struct(v, NULL, "struct tm", name, 0, errp);
-    visit_type_int32(v, &s->current_tm.tm_year, "tm_year", errp);
-    visit_type_int32(v, &s->current_tm.tm_mon, "tm_mon", errp);
-    visit_type_int32(v, &s->current_tm.tm_mday, "tm_mday", errp);
-    visit_type_int32(v, &s->current_tm.tm_hour, "tm_hour", errp);
-    visit_type_int32(v, &s->current_tm.tm_min, "tm_min", errp);
-    visit_type_int32(v, &s->current_tm.tm_sec, "tm_sec", errp);
+    visit_type_int32(v, &current_tm.tm_year, "tm_year", errp);
+    visit_type_int32(v, &current_tm.tm_mon, "tm_mon", errp);
+    visit_type_int32(v, &current_tm.tm_mday, "tm_mday", errp);
+    visit_type_int32(v, &current_tm.tm_hour, "tm_hour", errp);
+    visit_type_int32(v, &current_tm.tm_min, "tm_min", errp);
+    visit_type_int32(v, &current_tm.tm_sec, "tm_sec", errp);
     visit_end_struct(v, errp);
 }
 
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 09/10] RTC: Remove the current_tm field
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
                   ` (7 preceding siblings ...)
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 08/10] RTC: Get and set time without going through s->current_tm Paolo Bonzini
@ 2012-08-01 16:41 ` Paolo Bonzini
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 10/10] RTC: Allow to migrate from old QEMU Paolo Bonzini
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

This is not used anymore and only written to.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c |   11 -----------
 1 file changed, 11 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index a69ddc3..57a481a 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -59,7 +59,6 @@ typedef struct RTCState {
     MemoryRegion io;
     uint8_t cmos_data[128];
     uint8_t cmos_index;
-    struct tm current_tm;
     int32_t base_year;
     uint64_t base_rtc;
     uint64_t last_update;
@@ -522,7 +521,6 @@ static void rtc_set_time(RTCState *s)
     struct tm tm;
 
     rtc_get_time(s, &tm);
-    s->current_tm = *tm;
     s->base_rtc = mktimegm(&tm);
     s->last_update = qemu_get_clock_ns(rtc_clock);
 
@@ -564,7 +562,6 @@ static void rtc_update_time(RTCState *s)
     guest_sec = guest_nsec / NSEC_PER_SEC;
     ret = gmtime(&guest_sec);
     rtc_set_cmos(s, ret);
-    s->current_tm = *ret;
 }
 
 static int update_in_progress(RTCState *s)
@@ -680,7 +677,6 @@ static void rtc_set_date_from_host(ISADevice *dev)
 
     /* set the CMOS date */
     rtc_set_cmos(s, &tm);
-    s->current_tm = tm;
 
     val = rtc_to_bcd(s, (tm.tm_year / 100) + 19);
     rtc_set_memory(dev, REG_IBM_CENTURY_BYTE, val);
@@ -710,13 +706,6 @@ static const VMStateDescription vmstate_rtc = {
     .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_UINT32_V(irq_coalesced, RTCState, 2),
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Qemu-devel] [PATCH 10/10] RTC: Allow to migrate from old QEMU
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
                   ` (8 preceding siblings ...)
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 09/10] RTC: Remove the current_tm field Paolo Bonzini
@ 2012-08-01 16:41 ` Paolo Bonzini
  2012-08-01 19:51 ` [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Anthony Liguori
  2012-08-02  0:44 ` Zhang, Yang Z
  11 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-01 16:41 UTC (permalink / raw)
  To: qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

From: Yang Zhang <yang.z.zhang@intel.com>

The new logic is compatible with old, and it should not block migration
from old QEMU.  However, the new version cannot migrate to the old one.

Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/mc146818rtc.c |   40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 57a481a..0d7c6f4 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -697,11 +697,47 @@ static int rtc_post_load(void *opaque, int version_id)
     return 0;
 }
 
+static int rtc_load_old(QEMUFile *f, void *opaque, int version_id)
+{
+    RTCState *s = opaque;
+    uint8_t buf[77];
+
+    if (version_id > 2) {
+        return -EINVAL;
+    }
+
+    qemu_get_buffer(f, s->cmos_data, sizeof(s->cmos_data));
+    qemu_get_8s(f, &s->cmos_index);
+
+    /* We used to store a struct tm, but we already have the information
+     * in cmos_data.
+     */
+    qemu_get_buffer(f, buf, 7*4);
+
+    qemu_get_timer(f, s->periodic_timer);
+    s->next_periodic_time = qemu_get_be64(f);
+
+    /* Skip loading of next_second_time, second_timer, second_timer2.  */
+    qemu_get_buffer(f, buf, 3*8);
+
+    rtc_set_time(s);
+    s->offset = 0;
+    check_update_timer(s);
+
+    if (version_id >= 2) {
+        s->irq_coalesced = qemu_get_be32(f);
+        s->period = qemu_get_be32(f);
+    }
+
+    return rtc_post_load(opaque, version_id);
+}
+
 static const VMStateDescription vmstate_rtc = {
     .name = "mc146818rtc",
     .version_id = 3,
     .minimum_version_id = 3,
-    .minimum_version_id_old = 3,
+    .minimum_version_id_old = 1,
+    .load_state_old = rtc_load_old,
     .post_load = rtc_post_load,
     .fields      = (VMStateField []) {
         VMSTATE_BUFFER(cmos_data, RTCState),
@@ -825,7 +861,7 @@ static int rtc_initfn(ISADevice *dev)
     memory_region_init_io(&s->io, &cmos_ops, s, "rtc", 2);
     isa_register_ioport(dev, &s->io, base);
 
-    qdev_set_legacy_instance_id(&dev->qdev, base, 2);
+    qdev_set_legacy_instance_id(&dev->qdev, base, 3);
     qemu_register_reset(rtc_reset, s);
 
     object_property_add(OBJECT(s), "date", "struct tm",
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 05/10] RTC: Update the RTC clock only when reading it
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 05/10] RTC: Update the RTC clock only when reading it Paolo Bonzini
@ 2012-08-01 19:48   ` Anthony Liguori
  2012-08-02  9:09   ` Juan Quintela
  1 sibling, 0 replies; 20+ messages in thread
From: Anthony Liguori @ 2012-08-01 19:48 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

Paolo Bonzini <pbonzini@redhat.com> writes:

> From: Yang Zhang <yang.z.zhang@intel.com>
>
> Calculate guest RTC based on the time of the last update, instead of
> using timers.  The formula is
>
>     (base_rtc + guest_time_now - guest_time_last_update + offset)
>
> Base_rtc is the RTC value when the RTC was last updated.
> Guest_time_now is the guest time when the access happens.
> Guest_time_last_update was the guest time when the RTC was last updated.
> Offset is used when divider reset happens or the set bit is toggled.
>
> The timer is kept in order to signal interrupts, but it only needs to
> run when either UF or AF is cleared.  When the bits are both set, the
> timer does not run.
>
> UIP is now synthesized when reading register A.  If the timer is not set,
> or if there is more than one second before it (as is the case at the
> end of this series), the leading edge of UIP is computed and the rising
> edge occurs 220us later.  If the update timer occurs within one second,
> however, the rising edge of the AF and UF bits should coincide withe
> the falling edge of UIP.  We do not know exactly when this will happen
> because there could be delays in the servicing of the timer.  Hence, in
> this case reading register A only computes for the rising edge of UIP,
> and latches the bit until the timer is fired and clears it.
>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/mc146818rtc.c |  321 +++++++++++++++++++++++++++++++-----------------------
>  1 file changed, 186 insertions(+), 135 deletions(-)
>
> diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
> index 293f174..2c34a82 100644
> --- a/hw/mc146818rtc.c
> +++ b/hw/mc146818rtc.c
> @@ -45,6 +45,8 @@
>  # define DPRINTF_C(format, ...)      do { } while (0)
>  #endif
>  
> +#define NSEC_PER_SEC    1000000000LL
> +
>  #define RTC_REINJECT_ON_ACK_COUNT 20
>  
>  typedef struct RTCState {
> @@ -54,27 +56,40 @@ typedef struct RTCState {
>      uint8_t cmos_index;
>      struct tm current_tm;
>      int32_t base_year;
> +    uint64_t base_rtc;
> +    uint64_t last_update;
> +    int64_t offset;
>      qemu_irq irq;
>      qemu_irq sqw_irq;
>      int it_shift;
>      /* periodic timer */
>      QEMUTimer *periodic_timer;
>      int64_t next_periodic_time;
> -    /* second update */
> -    int64_t next_second_time;
> +    /* update-ended timer */
> +    QEMUTimer *update_timer;
>      uint16_t irq_reinject_on_ack_count;
>      uint32_t irq_coalesced;
>      uint32_t period;
>      QEMUTimer *coalesced_timer;
> -    QEMUTimer *second_timer;
> -    QEMUTimer *second_timer2;
>      Notifier clock_reset_notifier;
>      LostTickPolicy lost_tick_policy;
>      Notifier suspend_notifier;
>  } RTCState;
>  
>  static void rtc_set_time(RTCState *s);
> -static void rtc_copy_date(RTCState *s);
> +static void rtc_update_time(RTCState *s);
> +static void rtc_set_cmos(RTCState *s);
> +static inline int rtc_from_bcd(RTCState *s, int a);
> +
> +static uint64_t get_guest_rtc_ns(RTCState *s)
> +{
> +    uint64_t guest_rtc;
> +    uint64_t guest_clock = qemu_get_clock_ns(rtc_clock);
> +
> +    guest_rtc = s->base_rtc * NSEC_PER_SEC
> +                 + guest_clock - s->last_update + s->offset;
> +    return guest_rtc;
> +}
>  
>  #ifdef TARGET_I386
>  static void rtc_coalesced_timer_update(RTCState *s)
> @@ -110,6 +125,7 @@ static void rtc_coalesced_timer(void *opaque)
>  }
>  #endif
>  
> +/* handle periodic timer */
>  static void periodic_timer_update(RTCState *s, int64_t current_time)
>  {
>      int period_code, period;
> @@ -175,6 +191,100 @@ static void rtc_periodic_timer(void *opaque)
>      }
>  }
>  
> +/* handle update-ended timer */
> +static void check_update_timer(RTCState *s)
> +{
> +    uint64_t next_update_time;
> +    uint64_t guest_nsec;
> +
> +    /* From the data sheet: setting the SET bit does not prevent
> +     * interrupts from occurring!  However, it will prevent an
> +     * alarm interrupt from occurring, because the time of day is
> +     * not updated.
> +     */
> +    if ((s->cmos_data[RTC_REG_C] & REG_C_UF) &&
> +        (s->cmos_data[RTC_REG_B] & REG_B_SET)) {
> +        qemu_del_timer(s->update_timer);
> +        return;
> +    }
> +    if ((s->cmos_data[RTC_REG_C] & REG_C_UF) &&
> +        (s->cmos_data[RTC_REG_C] & REG_C_AF)) {
> +        qemu_del_timer(s->update_timer);
> +        return;
> +    }
> +
> +    guest_nsec = get_guest_rtc_ns(s) % NSEC_PER_SEC;
> +    /* reprogram to next second */
> +    next_update_time = qemu_get_clock_ns(rtc_clock)
> +        + NSEC_PER_SEC - guest_nsec;
> +    if (next_update_time != qemu_timer_expire_time_ns(s->update_timer)) {
> +        qemu_mod_timer(s->update_timer, next_update_time);
> +    }
> +}
> +
> +static inline uint8_t convert_hour(RTCState *s, uint8_t hour)
> +{
> +    if (!(s->cmos_data[RTC_REG_B] & REG_B_24H)) {
> +        hour %= 12;
> +        if (s->cmos_data[RTC_HOURS] & 0x80) {
> +            hour += 12;
> +        }
> +    }
> +    return hour;
> +}
> +
> +static uint32_t check_alarm(RTCState *s)
> +{
> +    uint8_t alarm_hour, alarm_min, alarm_sec;
> +    uint8_t cur_hour, cur_min, cur_sec;
> +
> +    alarm_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS_ALARM]);
> +    alarm_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES_ALARM]);
> +    alarm_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS_ALARM]);
> +    alarm_hour = convert_hour(s, alarm_hour);
> +
> +    cur_sec = rtc_from_bcd(s, s->cmos_data[RTC_SECONDS]);
> +    cur_min = rtc_from_bcd(s, s->cmos_data[RTC_MINUTES]);
> +    cur_hour = rtc_from_bcd(s, s->cmos_data[RTC_HOURS]);
> +    cur_hour = convert_hour(s, cur_hour);
> +
> +    if (((s->cmos_data[RTC_SECONDS_ALARM] & 0xc0) == 0xc0
> +                || alarm_sec == cur_sec) &&
> +            ((s->cmos_data[RTC_MINUTES_ALARM] & 0xc0) == 0xc0
> +             || alarm_min == cur_min) &&
> +            ((s->cmos_data[RTC_HOURS_ALARM] & 0xc0) == 0xc0
> +             || alarm_hour == cur_hour)) {
> +        return 1;
> +    }
> +    return 0;
> +
> +}
> +
> +static void rtc_update_timer(void *opaque)
> +{
> +    RTCState *s = opaque;
> +    int32_t irqs = REG_C_UF;
> +    int32_t new_irqs;
> +
> +    /* UIP might have been latched, update time and clear it.  */
> +    rtc_update_time(s);
> +    s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
> +
> +    if (check_alarm(s)) {
> +        irqs |= REG_C_AF;
> +        if (s->cmos_data[RTC_REG_B] & REG_B_AIE) {
> +            qemu_system_wakeup_request(QEMU_WAKEUP_REASON_RTC);
> +        }
> +    }
> +    new_irqs = irqs & ~s->cmos_data[RTC_REG_C];
> +    s->cmos_data[RTC_REG_C] |= irqs;
> +    if ((new_irqs & s->cmos_data[RTC_REG_B]) != 0) {
> +        s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
> +        qemu_irq_raise(s->irq);
> +    }
> +    check_update_timer(s);
> +}
> +
>  static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>  {
>      RTCState *s = opaque;
> @@ -189,6 +299,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>          case RTC_MINUTES_ALARM:
>          case RTC_HOURS_ALARM:
>              s->cmos_data[s->cmos_index] = data;
> +            check_update_timer(s);
>              break;
>          case RTC_SECONDS:
>          case RTC_MINUTES:
> @@ -201,6 +312,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>              /* if in set mode, do not update the time */
>              if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
>                  rtc_set_time(s);
> +                check_update_timer(s);
>              }
>              break;
>          case RTC_REG_A:
> @@ -208,15 +320,21 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>              s->cmos_data[RTC_REG_A] = (data & ~REG_A_UIP) |
>                  (s->cmos_data[RTC_REG_A] & REG_A_UIP);
>              periodic_timer_update(s, qemu_get_clock_ns(rtc_clock));
> +            check_update_timer(s);
>              break;
>          case RTC_REG_B:
>              if (data & REG_B_SET) {
> +                /* update cmos to when the rtc was stopping */
> +                if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
> +                    rtc_update_time(s);
> +                }
>                  /* set mode: reset UIP mode */
>                  s->cmos_data[RTC_REG_A] &= ~REG_A_UIP;
>                  data &= ~REG_B_UIE;
>              } else {
>                  /* if disabling set mode, update the time */
>                  if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
> +                    s->offset = get_guest_rtc_ns(s) % NSEC_PER_SEC;
>                      rtc_set_time(s);
>                  }
>              }
> @@ -231,6 +349,7 @@ static void cmos_ioport_write(void *opaque, uint32_t addr, uint32_t data)
>              }
>              s->cmos_data[RTC_REG_B] = data;
>              periodic_timer_update(s, qemu_get_clock_ns(rtc_clock));
> +            check_update_timer(s);
>              break;
>          case RTC_REG_C:
>          case RTC_REG_D:
> @@ -279,10 +398,13 @@ static void rtc_set_time(RTCState *s)
>      tm->tm_mon = rtc_from_bcd(s, s->cmos_data[RTC_MONTH]) - 1;
>      tm->tm_year = rtc_from_bcd(s, s->cmos_data[RTC_YEAR]) + s->base_year - 1900;
>  
> +    s->base_rtc = mktimegm(tm);
> +    s->last_update = qemu_get_clock_ns(rtc_clock);
> +
>      rtc_change_mon_event(tm);
>  }
>  
> -static void rtc_copy_date(RTCState *s)
> +static void rtc_set_cmos(RTCState *s)
>  {
>      const struct tm *tm = &s->current_tm;
>      int year;
> @@ -308,122 +430,41 @@ static void rtc_copy_date(RTCState *s)
>      s->cmos_data[RTC_YEAR] = rtc_to_bcd(s, year);
>  }
>  
> -/* month is between 0 and 11. */
> -static int get_days_in_month(int month, int year)
> +static void rtc_update_time(RTCState *s)
>  {
> -    static const int days_tab[12] = {
> -        31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31
> -    };
> -    int d;
> -    if ((unsigned )month >= 12)
> -        return 31;
> -    d = days_tab[month];
> -    if (month == 1) {
> -        if ((year % 4) == 0 && ((year % 100) != 0 || (year % 400) == 0))
> -            d++;
> -    }
> -    return d;
> +    struct tm *ret;
> +    time_t guest_sec;
> +    int64_t guest_nsec;
> +
> +    guest_nsec = get_guest_rtc_ns(s);
> +    guest_sec = guest_nsec / NSEC_PER_SEC;
> +    ret = gmtime(&guest_sec);

gmtime_r() ?

> +    s->current_tm = *ret;
> +    rtc_set_cmos(s);
>  }
>  
> -/* update 'tm' to the next second */
> -static void rtc_next_second(struct tm *tm)
> +static int update_in_progress(RTCState *s)
>  {
> -    int days_in_month;
> -
> -    tm->tm_sec++;
> -    if ((unsigned)tm->tm_sec >= 60) {
> -        tm->tm_sec = 0;
> -        tm->tm_min++;
> -        if ((unsigned)tm->tm_min >= 60) {
> -            tm->tm_min = 0;
> -            tm->tm_hour++;
> -            if ((unsigned)tm->tm_hour >= 24) {
> -                tm->tm_hour = 0;
> -                /* next day */
> -                tm->tm_wday++;
> -                if ((unsigned)tm->tm_wday >= 7)
> -                    tm->tm_wday = 0;
> -                days_in_month = get_days_in_month(tm->tm_mon,
> -                                                  tm->tm_year + 1900);
> -                tm->tm_mday++;
> -                if (tm->tm_mday < 1) {
> -                    tm->tm_mday = 1;
> -                } else if (tm->tm_mday > days_in_month) {
> -                    tm->tm_mday = 1;
> -                    tm->tm_mon++;
> -                    if (tm->tm_mon >= 12) {
> -                        tm->tm_mon = 0;
> -                        tm->tm_year++;
> -                    }
> -                }
> -            }
> -        }
> -    }
> -}
> -
> -
> -static void rtc_update_second(void *opaque)
> -{
> -    RTCState *s = opaque;
> -    int64_t delay;
> -
> -    /* if the oscillator is not in normal operation, we do not update */
> -    if ((s->cmos_data[RTC_REG_A] & 0x70) != 0x20) {
> -        s->next_second_time += get_ticks_per_sec();
> -        qemu_mod_timer(s->second_timer, s->next_second_time);
> -    } else {
> -        rtc_next_second(&s->current_tm);
> -
> -        if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
> -            /* update in progress bit */
> -            s->cmos_data[RTC_REG_A] |= REG_A_UIP;
> -        }
> -        /* should be 244 us = 8 / 32768 seconds, but currently the
> -           timers do not have the necessary resolution. */
> -        delay = (get_ticks_per_sec() * 1) / 100;
> -        if (delay < 1)
> -            delay = 1;
> -        qemu_mod_timer(s->second_timer2,
> -                       s->next_second_time + delay);
> -    }
> -}
> -
> -static void rtc_update_second2(void *opaque)
> -{
> -    RTCState *s = opaque;
> +    int64_t guest_nsec;
>  
> -    if (!(s->cmos_data[RTC_REG_B] & REG_B_SET)) {
> -        rtc_copy_date(s);
> +    if (s->cmos_data[RTC_REG_B] & REG_B_SET) {
> +        return 0;
>      }
> -
> -    /* check alarm */
> -    if (((s->cmos_data[RTC_SECONDS_ALARM] & 0xc0) == 0xc0 ||
> -         rtc_from_bcd(s, s->cmos_data[RTC_SECONDS_ALARM]) == s->current_tm.tm_sec) &&
> -        ((s->cmos_data[RTC_MINUTES_ALARM] & 0xc0) == 0xc0 ||
> -         rtc_from_bcd(s, s->cmos_data[RTC_MINUTES_ALARM]) == s->current_tm.tm_min) &&
> -        ((s->cmos_data[RTC_HOURS_ALARM] & 0xc0) == 0xc0 ||
> -         rtc_from_bcd(s, s->cmos_data[RTC_HOURS_ALARM]) == s->current_tm.tm_hour)) {
> -
> -        s->cmos_data[RTC_REG_C] |= REG_C_AF;
> -        if (s->cmos_data[RTC_REG_B] & REG_B_AIE) {
> -            qemu_system_wakeup_request(QEMU_WAKEUP_REASON_RTC);
> -            qemu_irq_raise(s->irq);
> -            s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
> +    if (qemu_timer_pending(s->update_timer)) {
> +        int64_t next_update_time = qemu_timer_expire_time_ns(s->update_timer);
> +        /* Latch UIP until the timer expires.  */
> +        if (qemu_get_clock_ns(rtc_clock) >= (next_update_time - 244000)) {
> +            s->cmos_data[RTC_REG_A] |= REG_A_UIP;
> +            return 1;
>          }
>      }
>  
> -    /* update ended interrupt */
> -    s->cmos_data[RTC_REG_C] |= REG_C_UF;
> -    if (s->cmos_data[RTC_REG_B] & REG_B_UIE) {
> -        s->cmos_data[RTC_REG_C] |= REG_C_IRQF;
> -        qemu_irq_raise(s->irq);
> +    guest_nsec = get_guest_rtc_ns(s);
> +    /* UIP bit will be set at last 244us of every second. */
> +    if ((guest_nsec % NSEC_PER_SEC) >= (NSEC_PER_SEC - 244000)) {
> +        return 1;
>      }

244us isn't quite right, 8 / 32khz is closer to 245us but it makes more
sense to me to:

/* UIP gets held for 8 cycles */
#define RTC_CLOCK_RATE   (32768)
#define UIP_HOLD_LENGTH  ((8 * NSEC_PER_SEC) / RTC_CLOCK_RATE)

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
                   ` (9 preceding siblings ...)
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 10/10] RTC: Allow to migrate from old QEMU Paolo Bonzini
@ 2012-08-01 19:51 ` Anthony Liguori
  2012-08-02  6:32   ` Paolo Bonzini
  2012-08-02  0:44 ` Zhang, Yang Z
  11 siblings, 1 reply; 20+ messages in thread
From: Anthony Liguori @ 2012-08-01 19:51 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel; +Cc: yang.z.zhang, mdroth, quintela

Paolo Bonzini <pbonzini@redhat.com> writes:

> The current RTC emulation has two timers firing every second, one
> on each edge of the UIP bit.  This will prevent CPUs from staying at
> deep C-states.  Intel's measurements from previous submissions show the
> C6 residency reduced by 6% when running 64 idle guests.
>
> The following patches remove the two timers.  The patches update the RTC
> clock only when the guest tries to read it, and only set timers when
> update or alarm is clear.  Hence, a guest will typically fire the RTC
> timer only twice, respectively one second after it starts and at the
> next midnight.
>
> The patches are mostly the work of Yang Zhang.  My contribution was
> limited to reorganizing them for better bisectability, and cleaning
> up the computation of UIP.
>
> A qtest for this is not as reliable as a test that actually runs code
> in a VM.  A qtest is more deterministic, and the "wiggling" introduced
> by running code in the VM is much more likely to find bugs.  I'll post
> the unit test separately.  Because the patches also improve the quality
> of the emulation, this test fails without the patches.
>
> The first four patches are simple preparatory changes.
>
> The fifth patch removes the timers, and replaces them with a single
> timer that is fired every second until UF and AF.  The update logic is
> moved to the reading of the registers, and so is UIP.
>
> The sixth patch implements support for divider reset, which helps testing
> the RTC because it places it in a known state.  The seventh patch avoids
> firing the timer every second until the next alarm.
>
> The next two patches clean up the state of the RTC to eliminate a useless
> duplication, and the tenth completes migration support.  Still, backwards
> migration is broken because the algorithms in the new device model are
> pretty much completely different.  Downstreams that care should include
> both device models and pick the old one for old machine types.

Other than the few minor comments, the series looks good overall.

Can you talk about the guests that you've tested with this?
Specifically, have you tested win2k8 64-bit?

Regards,

Anthony Liguori

>
> v1->v2: annotate versions correctly in the vmstate, added new
>         patches to remove current_tm
>
> Paolo Bonzini (4):
>   vmstate: add VMSTATE_TIMER_V
>   RTC: Do not fire timer periodically to catch next alarm
>   RTC: Get and set time without going through s->current_tm
>   RTC: Remove the current_tm field
>
> Yang Zhang (6):
>   RTC: Remove the logic to update time format when DM bit changed
>   RTC: Rename rtc_timer_update
>   RTC: Update interrupt state when interrupts are masked/unmasked
>   RTC: Update the RTC clock only when reading it
>   RTC: Add divider reset support
>   RTC: Allow to migrate from old QEMU
>
>  hw/mc146818rtc.c      |  540 ++++++++++++++++++++++++++++++++++---------------
>  hw/mc146818rtc_regs.h |    1 +
>  vmstate.h             |    5 +-
>  3 files changed, 377 insertions(+), 169 deletions(-)
>
> -- 
> 1.7.10.4

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer
  2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
                   ` (10 preceding siblings ...)
  2012-08-01 19:51 ` [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Anthony Liguori
@ 2012-08-02  0:44 ` Zhang, Yang Z
  11 siblings, 0 replies; 20+ messages in thread
From: Zhang, Yang Z @ 2012-08-02  0:44 UTC (permalink / raw)
  To: Paolo Bonzini, qemu-devel@nongnu.org
  Cc: mdroth@linux.vnet.ibm.com, quintela@redhat.com

Paolo Bonzini wrote on 2012-08-02:
> The current RTC emulation has two timers firing every second, one
> on each edge of the UIP bit.  This will prevent CPUs from staying at
> deep C-states.  Intel's measurements from previous submissions show the
> C6 residency reduced by 6% when running 64 idle guests.
> 
> The following patches remove the two timers.  The patches update the RTC
> clock only when the guest tries to read it, and only set timers when
> update or alarm is clear.  Hence, a guest will typically fire the RTC
> timer only twice, respectively one second after it starts and at the
> next midnight.
> 
> The patches are mostly the work of Yang Zhang.  My contribution was
> limited to reorganizing them for better bisectability, and cleaning
> up the computation of UIP.

Thanks for your review and great effort to push those patches. 
 
> A qtest for this is not as reliable as a test that actually runs code
> in a VM.  A qtest is more deterministic, and the "wiggling" introduced
> by running code in the VM is much more likely to find bugs.  I'll post
> the unit test separately.  Because the patches also improve the quality
> of the emulation, this test fails without the patches.
> 
> The first four patches are simple preparatory changes.
> 
> The fifth patch removes the timers, and replaces them with a single
> timer that is fired every second until UF and AF.  The update logic is
> moved to the reading of the registers, and so is UIP.
> 
> The sixth patch implements support for divider reset, which helps testing
> the RTC because it places it in a known state.  The seventh patch avoids
> firing the timer every second until the next alarm.
> 
> The next two patches clean up the state of the RTC to eliminate a useless
> duplication, and the tenth completes migration support.  Still, backwards
> migration is broken because the algorithms in the new device model are
> pretty much completely different.  Downstreams that care should include
> both device models and pick the old one for old machine types.
> 
> v1->v2: annotate versions correctly in the vmstate, added new
>         patches to remove current_tm
> Paolo Bonzini (4):
>   vmstate: add VMSTATE_TIMER_V
>   RTC: Do not fire timer periodically to catch next alarm
>   RTC: Get and set time without going through s->current_tm
>   RTC: Remove the current_tm field
> Yang Zhang (6):
>   RTC: Remove the logic to update time format when DM bit changed
>   RTC: Rename rtc_timer_update
>   RTC: Update interrupt state when interrupts are masked/unmasked
>   RTC: Update the RTC clock only when reading it
>   RTC: Add divider reset support
>   RTC: Allow to migrate from old QEMU
>  hw/mc146818rtc.c      |  540
>  ++++++++++++++++++++++++++++++++++--------------- hw/mc146818rtc_regs.h
>  |    1 + vmstate.h             |    5 +- 3 files changed, 377
>  insertions(+), 169 deletions(-)
> --
> 1.7.10.4

Best regards,
Yang

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer
  2012-08-01 19:51 ` [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Anthony Liguori
@ 2012-08-02  6:32   ` Paolo Bonzini
  0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-02  6:32 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: yang.z.zhang, quintela, qemu-devel, mdroth

Il 01/08/2012 21:51, Anthony Liguori ha scritto:
>> > The next two patches clean up the state of the RTC to eliminate a useless
>> > duplication, and the tenth completes migration support.  Still, backwards
>> > migration is broken because the algorithms in the new device model are
>> > pretty much completely different.  Downstreams that care should include
>> > both device models and pick the old one for old machine types.
> Other than the few minor comments, the series looks good overall.
> 
> Can you talk about the guests that you've tested with this?
> Specifically, have you tested win2k8 64-bit?

I did smoke-test most Windows versions, but I don't expect much coverage
from those tests.  The periodic timer is not affected by the changes,
they only affect the clock.

Paolo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 06/10] vmstate: add VMSTATE_TIMER_V
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 06/10] vmstate: add VMSTATE_TIMER_V Paolo Bonzini
@ 2012-08-02  8:56   ` Juan Quintela
  2012-08-02  9:02     ` Paolo Bonzini
  0 siblings, 1 reply; 20+ messages in thread
From: Juan Quintela @ 2012-08-02  8:56 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: yang.z.zhang, qemu-devel, mdroth

Paolo Bonzini <pbonzini@redhat.com> wrote:
> Also, for consistency with other occurrences, implement VMSTATE_TIMER
> as a special case of VMSTATE_TIMER_V rather than VMSTATE_TIMER_TEST.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  vmstate.h |    5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/vmstate.h b/vmstate.h
> index 5bd2b76..092f21d 100644
> --- a/vmstate.h
> +++ b/vmstate.h
> @@ -503,8 +503,11 @@ extern const VMStateInfo vmstate_info_unused_buffer;
>  #define VMSTATE_TIMER_TEST(_f, _s, _test)                             \
>      VMSTATE_POINTER_TEST(_f, _s, _test, vmstate_info_timer, QEMUTimer *)
>  
> +#define VMSTATE_TIMER_V(_f, _s, _v)                                   \
> +    VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)
> +
>  #define VMSTATE_TIMER(_f, _s)                                         \
> -    VMSTATE_TIMER_TEST(_f, _s, NULL)
> +    VMSTATE_POINTER(_f, _s, 0, vmstate_info_timer, QEMUTimer *)

You didn't did it, it should be something like:

#define VMSTATE_TIMER(_f, _s)                                         \
-    VMSTATE_TIMER_TEST(_f, _s, NULL)
+    VMSTATE_TIMER_V(_f, _s, 0)



>  
>  #define VMSTATE_TIMER_ARRAY(_f, _s, _n)                              \
>      VMSTATE_ARRAY_OF_POINTER(_f, _s, _n, 0, vmstate_info_timer, QEMUTimer *)

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 06/10] vmstate: add VMSTATE_TIMER_V
  2012-08-02  8:56   ` Juan Quintela
@ 2012-08-02  9:02     ` Paolo Bonzini
  0 siblings, 0 replies; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-02  9:02 UTC (permalink / raw)
  To: quintela; +Cc: yang.z.zhang, qemu-devel, mdroth

Il 02/08/2012 10:56, Juan Quintela ha scritto:
>> > @@ -503,8 +503,11 @@ extern const VMStateInfo vmstate_info_unused_buffer;
>> >  #define VMSTATE_TIMER_TEST(_f, _s, _test)                             \
>> >      VMSTATE_POINTER_TEST(_f, _s, _test, vmstate_info_timer, QEMUTimer *)
>> >  
>> > +#define VMSTATE_TIMER_V(_f, _s, _v)                                   \
>> > +    VMSTATE_POINTER(_f, _s, _v, vmstate_info_timer, QEMUTimer *)
>> > +
>> >  #define VMSTATE_TIMER(_f, _s)                                         \
>> > -    VMSTATE_TIMER_TEST(_f, _s, NULL)
>> > +    VMSTATE_POINTER(_f, _s, 0, vmstate_info_timer, QEMUTimer *)
> You didn't did it, it should be something like:
> 
> #define VMSTATE_TIMER(_f, _s)                                         \
> -    VMSTATE_TIMER_TEST(_f, _s, NULL)
> +    VMSTATE_TIMER_V(_f, _s, 0)

I'm following the other examples:

#define VMSTATE_UINT16_EQUAL(_f, _s)                                  \
    VMSTATE_SINGLE(_f, _s, 0, vmstate_info_uint16_equal, uint16_t)

#define VMSTATE_UINT16_EQUAL_V(_f, _s, _v)                            \
    VMSTATE_SINGLE(_f, _s, _v, vmstate_info_uint16_equal, uint16_t)

but I can change it as you indicated.

Paolo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 05/10] RTC: Update the RTC clock only when reading it
  2012-08-01 16:41 ` [Qemu-devel] [PATCH 05/10] RTC: Update the RTC clock only when reading it Paolo Bonzini
  2012-08-01 19:48   ` Anthony Liguori
@ 2012-08-02  9:09   ` Juan Quintela
  2012-08-02  9:14     ` Paolo Bonzini
  1 sibling, 1 reply; 20+ messages in thread
From: Juan Quintela @ 2012-08-02  9:09 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: yang.z.zhang, qemu-devel, mdroth

Paolo Bonzini <pbonzini@redhat.com> wrote:
> From: Yang Zhang <yang.z.zhang@intel.com>
>
> Calculate guest RTC based on the time of the last update, instead of
> using timers.  The formula is
>
>     (base_rtc + guest_time_now - guest_time_last_update + offset)
>
> Base_rtc is the RTC value when the RTC was last updated.
> Guest_time_now is the guest time when the access happens.
> Guest_time_last_update was the guest time when the RTC was last updated.
> Offset is used when divider reset happens or the set bit is toggled.
>
> The timer is kept in order to signal interrupts, but it only needs to
> run when either UF or AF is cleared.  When the bits are both set, the
> timer does not run.
>
> UIP is now synthesized when reading register A.  If the timer is not set,
> or if there is more than one second before it (as is the case at the
> end of this series), the leading edge of UIP is computed and the rising
> edge occurs 220us later.  If the update timer occurs within one second,
> however, the rising edge of the AF and UF bits should coincide withe
> the falling edge of UIP.  We do not know exactly when this will happen
> because there could be delays in the servicing of the timer.  Hence, in
> this case reading register A only computes for the rising edge of UIP,
> and latches the bit until the timer is fired and clears it.
>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

>  
>  static const VMStateDescription vmstate_rtc = {
>      .name = "mc146818rtc",
> -    .version_id = 2,
> -    .minimum_version_id = 1,
> -    .minimum_version_id_old = 1,
> +    .version_id = 3,
> +    .minimum_version_id = 3,
> +    .minimum_version_id_old = 3,
>      .post_load = rtc_post_load,
>      .fields      = (VMStateField []) {
>          VMSTATE_BUFFER(cmos_data, RTCState),
> @@ -542,11 +595,12 @@ static const VMStateDescription vmstate_rtc = {
>          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_UINT64_V(base_rtc, RTCState, 3),
> +        VMSTATE_UINT64_V(last_update, RTCState, 3),
> +        VMSTATE_INT64_V(offset, RTCState, 3),
> +        VMSTATE_TIMER_V(update_timer, RTCState, 3),
>          VMSTATE_END_OF_LIST()
>      }
>  };

Why did you remove all the migration from previous versions?
You can't migrate now from version{1,2}, and we used to be able to do
it?

Why did you remove it?

Later, Juan.

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 05/10] RTC: Update the RTC clock only when reading it
  2012-08-02  9:09   ` Juan Quintela
@ 2012-08-02  9:14     ` Paolo Bonzini
  2012-08-02  9:58       ` Juan Quintela
  0 siblings, 1 reply; 20+ messages in thread
From: Paolo Bonzini @ 2012-08-02  9:14 UTC (permalink / raw)
  To: quintela; +Cc: yang.z.zhang, qemu-devel, mdroth

Il 02/08/2012 11:09, Juan Quintela ha scritto:
> Why did you remove all the migration from previous versions?
> You can't migrate now from version{1,2}, and we used to be able to do
> it?
> 
> Why did you remove it?

Because it won't work; we removed three fields.  You need to add
rtc_load_old which is done later in the series.  But I guess I can use
VMSTATE_UNUSED instead.

Paolo

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Qemu-devel] [PATCH 05/10] RTC: Update the RTC clock only when reading it
  2012-08-02  9:14     ` Paolo Bonzini
@ 2012-08-02  9:58       ` Juan Quintela
  0 siblings, 0 replies; 20+ messages in thread
From: Juan Quintela @ 2012-08-02  9:58 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: yang.z.zhang, qemu-devel, mdroth

Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 02/08/2012 11:09, Juan Quintela ha scritto:
>> Why did you remove all the migration from previous versions?
>> You can't migrate now from version{1,2}, and we used to be able to do
>> it?
>> 
>> Why did you remove it?
>
> Because it won't work; we removed three fields.  You need to add
> rtc_load_old which is done later in the series.  But I guess I can use
> VMSTATE_UNUSED instead.

Something like (completely untested):

static bool version_less_3(void *opaque, int version_id)
{
    return version_id < 3;
}

static const VMStateDescription vmstate_rtc = {
    .name = "mc146818rtc",
    .version_id = 3,
    .minimum_version_id = 1,
    .minimum_version_id_old = 1,
    .post_load = rtc_post_load,
    .fields      = (VMStateField []) {
         VMSTATE_BUFFER(cmos_data, RTCState),
 @@ -542,11 +595,12 @@ static const VMStateDescription vmstate_rtc = {
         VMSTATE_INT32(current_tm.tm_year, RTCState),
         VMSTATE_TIMER(periodic_timer, RTCState),
         VMSTATE_INT64(next_periodic_time, RTCState),
         VMSTATE_UNUSED_TEST(8*3, v_less_3); //* whatever space */
-        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_UINT64_V(base_rtc, RTCState, 3),
+        VMSTATE_UINT64_V(last_update, RTCState, 3),
+        VMSTATE_INT64_V(offset, RTCState, 3),
+        VMSTATE_TIMER_V(update_timer, RTCState, 3),
         VMSTATE_END_OF_LIST()
     }
 };


This will make "migration protocol" work,  I have zero clue if
"obviating" the value of next_second_time, and the two second_timers*
can work, that depends on how rtc works.  Perhaps some extra magic on
post_load() is needed, though.

Could you tell me if you need anything else?

Later, Juan.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2012-08-02  9:59 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-01 16:41 [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Paolo Bonzini
2012-08-01 16:41 ` [Qemu-devel] [PATCH 01/10] RTC: Remove the logic to update time format when DM bit changed Paolo Bonzini
2012-08-01 16:41 ` [Qemu-devel] [PATCH 02/10] RTC: Rename rtc_timer_update Paolo Bonzini
2012-08-01 16:41 ` [Qemu-devel] [PATCH 03/10] RTC: Update interrupt state when interrupts are masked/unmasked Paolo Bonzini
2012-08-01 16:41 ` [Qemu-devel] [PATCH 06/10] vmstate: add VMSTATE_TIMER_V Paolo Bonzini
2012-08-02  8:56   ` Juan Quintela
2012-08-02  9:02     ` Paolo Bonzini
2012-08-01 16:41 ` [Qemu-devel] [PATCH 05/10] RTC: Update the RTC clock only when reading it Paolo Bonzini
2012-08-01 19:48   ` Anthony Liguori
2012-08-02  9:09   ` Juan Quintela
2012-08-02  9:14     ` Paolo Bonzini
2012-08-02  9:58       ` Juan Quintela
2012-08-01 16:41 ` [Qemu-devel] [PATCH 06/10] RTC: Add divider reset support Paolo Bonzini
2012-08-01 16:41 ` [Qemu-devel] [PATCH 07/10] RTC: Do not fire timer periodically to catch next alarm Paolo Bonzini
2012-08-01 16:41 ` [Qemu-devel] [PATCH 08/10] RTC: Get and set time without going through s->current_tm Paolo Bonzini
2012-08-01 16:41 ` [Qemu-devel] [PATCH 09/10] RTC: Remove the current_tm field Paolo Bonzini
2012-08-01 16:41 ` [Qemu-devel] [PATCH 10/10] RTC: Allow to migrate from old QEMU Paolo Bonzini
2012-08-01 19:51 ` [Qemu-devel] [PATCH 0/10] Remove periodic wakeup from RTC timer Anthony Liguori
2012-08-02  6:32   ` Paolo Bonzini
2012-08-02  0:44 ` Zhang, Yang Z

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).