[Qemu-devel] [RFC][PATCH 0/3] Let RTC follow backward jumps of host clock immediately

[Qemu-devel] [RFC][PATCH 0/3] Let RTC follow backward jumps of host clock immediately

[Jan Kiszka]

By default, we base the mc146818 RTC on the host clock (CLOCK_REALTIME).
This works fine if only the frequency of the host clock is tuned (e.g.
by NTP) or if it is set to a future time. However, if the host is tuned
backward, e.g. because NTP obtained the correct time after the guest was
already started or the admin decided to tune the local time, we see an
unpleasant effect in the guest: The RTC will stall for the period the
host clock is set back.

This series tries to address the issue more gracefully. By detecting
those warps and providing a callback mechanism to device models, the
RTC is enabled to update its timers and register content immediately.
Tested successfully with a hwclock readout loop in a Linux guest while
fiddling with the host time.

Note that if this kind of RTC adjustment is not wanted, the user is
still free to decouple the RTC from the host clock and base it on the
VM clock - just like before.

Jan Kiszka (3):
  qemu-timer: Consolidate qemu_get_clock and qemu_get_clock_ns
  qemu-timer: Introduce warp callback
  mc146818rtc: Handle host clock warps

 hw/mc146818rtc.c |   17 ++++++++++++
 qemu-timer.c     |   77 ++++++++++++++++++++++++++++++++++++++++++++----------
 qemu-timer.h     |    5 +++
 3 files changed, 85 insertions(+), 14 deletions(-)

[Qemu-devel] [RFC][PATCH 1/3] qemu-timer: Consolidate qemu_get_clock and qemu_get_clock_ns

[Jan Kiszka]

Both functions have a lot in common, push those bits into a shared
helper.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 qemu-timer.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 95814af..abad93b 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -506,11 +506,9 @@ static void qemu_run_timers(QEMUClock *clock)
     }
 }
 
-int64_t qemu_get_clock(QEMUClock *clock)
+static int64_t get_clock_common(QEMUClock *clock)
 {
     switch(clock->type) {
-    case QEMU_CLOCK_REALTIME:
-        return get_clock() / 1000000;
     default:
     case QEMU_CLOCK_VIRTUAL:
         if (use_icount) {
@@ -523,20 +521,21 @@ int64_t qemu_get_clock(QEMUClock *clock)
     }
 }
 
+int64_t qemu_get_clock(QEMUClock *clock)
+{
+    if (clock->type == QEMU_CLOCK_REALTIME) {
+        return get_clock() / 1000000;
+    } else {
+        return get_clock_common(clock);
+    }
+}
+
 int64_t qemu_get_clock_ns(QEMUClock *clock)
 {
-    switch(clock->type) {
-    case QEMU_CLOCK_REALTIME:
+    if (clock->type == QEMU_CLOCK_REALTIME) {
         return get_clock();
-    default:
-    case QEMU_CLOCK_VIRTUAL:
-        if (use_icount) {
-            return cpu_get_icount();
-        } else {
-            return cpu_get_clock();
-        }
-    case QEMU_CLOCK_HOST:
-        return get_clock_realtime();
+    } else {
+        return get_clock_common(clock);
     }
 }
 
-- 
1.7.1

[Qemu-devel] [RFC][PATCH 2/3] qemu-timer: Introduce warp callback

[Jan Kiszka]

QEMU_CLOCK_HOST is based on the system time which may jump backward in
case the admin or NTP adjusts it. RTC emulations and other device models
can suffer in this case as timers will stall for the period the clock
was tuned back.

This adds a detection mechanism that checks on every host clock readout
if the new time is before the last result. In that case callbacks are
fired that any interested device model can register with the clock.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 qemu-timer.c |   52 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 qemu-timer.h |    5 +++++
 2 files changed, 56 insertions(+), 1 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index abad93b..828e4ad 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -154,9 +154,17 @@ void cpu_disable_ticks(void)
 #define QEMU_CLOCK_VIRTUAL  1
 #define QEMU_CLOCK_HOST     2
 
+struct QEMUClockWarpListener {
+    QEMUClockWarpCB cb;
+    void *opaque;
+    QTAILQ_ENTRY(QEMUClockWarpListener) entry;
+};
+
 struct QEMUClock {
     int type;
     int enabled;
+    QTAILQ_HEAD(warp_listeners, QEMUClockWarpListener) warp_listeners;
+    int64_t last;
     /* XXX: add frequency */
 };
 
@@ -384,9 +392,15 @@ static QEMUTimer *active_timers[QEMU_NUM_CLOCKS];
 static QEMUClock *qemu_new_clock(int type)
 {
     QEMUClock *clock;
+
     clock = qemu_mallocz(sizeof(QEMUClock));
     clock->type = type;
     clock->enabled = 1;
+    QTAILQ_INIT(&clock->warp_listeners);
+    /* required to detect & report backward jumps */
+    if (type == QEMU_CLOCK_HOST) {
+        clock->last = get_clock_realtime();
+    }
     return clock;
 }
 
@@ -508,6 +522,9 @@ static void qemu_run_timers(QEMUClock *clock)
 
 static int64_t get_clock_common(QEMUClock *clock)
 {
+    struct QEMUClockWarpListener *listener;
+    int64_t now, last;
+
     switch(clock->type) {
     default:
     case QEMU_CLOCK_VIRTUAL:
@@ -517,7 +534,15 @@ static int64_t get_clock_common(QEMUClock *clock)
             return cpu_get_clock();
         }
     case QEMU_CLOCK_HOST:
-        return get_clock_realtime();
+        now = get_clock_realtime();
+        last = clock->last;
+        clock->last = now;
+        if (now < last) {
+            QTAILQ_FOREACH(listener, &clock->warp_listeners, entry) {
+                listener->cb(clock, now, listener->opaque);
+            }
+        }
+        return now;
     }
 }
 
@@ -539,6 +564,31 @@ int64_t qemu_get_clock_ns(QEMUClock *clock)
     }
 }
 
+void qemu_register_clock_warp(QEMUClock *clock, QEMUClockWarpCB cb,
+                              void *opaque)
+{
+    struct QEMUClockWarpListener *listener =
+        qemu_malloc(sizeof(struct QEMUClockWarpListener));
+
+    listener->cb = cb;
+    listener->opaque = opaque;
+    QTAILQ_INSERT_TAIL(&clock->warp_listeners, listener, entry);
+}
+
+void qemu_unregister_clock_warp(QEMUClock *clock, QEMUClockWarpCB cb,
+                                void *opaque)
+{
+    struct QEMUClockWarpListener *listener;
+
+    QTAILQ_FOREACH(listener, &clock->warp_listeners, entry) {
+        if (listener->cb == cb && listener->opaque == opaque) {
+            QTAILQ_REMOVE(&clock->warp_listeners, listener, entry);
+            qemu_free(listener);
+            break;
+        }
+    }
+}
+
 void init_clocks(void)
 {
     rt_clock = qemu_new_clock(QEMU_CLOCK_REALTIME);
diff --git a/qemu-timer.h b/qemu-timer.h
index 8cd8f83..a7b37da 100644
--- a/qemu-timer.h
+++ b/qemu-timer.h
@@ -13,6 +13,7 @@
 /* timers */
 
 typedef struct QEMUClock QEMUClock;
+typedef void (*QEMUClockWarpCB)(QEMUClock *clock, int64_t now, void *opaque);
 typedef void QEMUTimerCB(void *opaque);
 
 /* The real time clock should be used only for stuff which does not
@@ -36,6 +37,10 @@ extern QEMUClock *host_clock;
 int64_t qemu_get_clock(QEMUClock *clock);
 int64_t qemu_get_clock_ns(QEMUClock *clock);
 void qemu_clock_enable(QEMUClock *clock, int enabled);
+void qemu_register_clock_warp(QEMUClock *clock, QEMUClockWarpCB cb,
+                              void *opaque);
+void qemu_unregister_clock_warp(QEMUClock *clock, QEMUClockWarpCB cb,
+                                void *opaque);
 
 QEMUTimer *qemu_new_timer(QEMUClock *clock, QEMUTimerCB *cb, void *opaque);
 void qemu_free_timer(QEMUTimer *ts);
-- 
1.7.1

[Qemu-devel] [RFC][PATCH 3/3] mc146818rtc: Handle host clock warps

[Jan Kiszka]

Make use of the new warp notifier to update the RTC whenever rtc_clock
is the host clock and that happens to jump backward. This avoids that
the RTC stalls for the period the host clock was set back.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/mc146818rtc.c |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/hw/mc146818rtc.c b/hw/mc146818rtc.c
index 6466aff..f65ea5a 100644
--- a/hw/mc146818rtc.c
+++ b/hw/mc146818rtc.c
@@ -563,6 +563,21 @@ static const VMStateDescription vmstate_rtc = {
     }
 };
 
+static void rtc_clock_warp(QEMUClock *clock, int64_t now, void *opaque)
+{
+    RTCState *s = opaque;
+
+    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);
+#ifdef TARGET_I386
+    if (rtc_td_hack) {
+        rtc_coalesced_timer_update(s);
+    }
+#endif
+}
+
 static void rtc_reset(void *opaque)
 {
     RTCState *s = opaque;
@@ -599,6 +614,8 @@ static int rtc_initfn(ISADevice *dev)
     s->second_timer = qemu_new_timer(rtc_clock, rtc_update_second, s);
     s->second_timer2 = qemu_new_timer(rtc_clock, rtc_update_second2, s);
 
+    qemu_register_clock_warp(rtc_clock, rtc_clock_warp, s);
+
     s->next_second_time =
         qemu_get_clock(rtc_clock) + (get_ticks_per_sec() * 99) / 100;
     qemu_mod_timer(s->second_timer2, s->next_second_time);
-- 
1.7.1

Re: [Qemu-devel] [RFC][PATCH 0/3] Let RTC follow backward jumps of host clock immediately

[Zachary Amsden]

On 12/17/2010 04:58 AM, Jan Kiszka wrote:
> By default, we base the mc146818 RTC on the host clock (CLOCK_REALTIME).
> This works fine if only the frequency of the host clock is tuned (e.g.
> by NTP) or if it is set to a future time. However, if the host is tuned
> backward, e.g. because NTP obtained the correct time after the guest was
> already started or the admin decided to tune the local time, we see an
> unpleasant effect in the guest: The RTC will stall for the period the
> host clock is set back.
>
> This series tries to address the issue more gracefully. By detecting
> those warps and providing a callback mechanism to device models, the
> RTC is enabled to update its timers and register content immediately.
> Tested successfully with a hwclock readout loop in a Linux guest while
> fiddling with the host time.
>
> Note that if this kind of RTC adjustment is not wanted, the user is
> still free to decouple the RTC from the host clock and base it on the
> VM clock - just like before.
>    

Did you test this with a Windows guest?  They rely heavily on RTC, this 
is probably a better behavior for that case.  I'd be curious if Windows 
accepts the RTC register changing underneath it, but based on earlier 
versions of Windows Time Service, would be surprised if it did not.

Zach

Re: [Qemu-devel] [RFC][PATCH 0/3] Let RTC follow backward jumps of host clock immediately

[Jan Kiszka]

Am 23.12.2010 21:45, Zachary Amsden wrote:
> On 12/17/2010 04:58 AM, Jan Kiszka wrote:
>> By default, we base the mc146818 RTC on the host clock (CLOCK_REALTIME).
>> This works fine if only the frequency of the host clock is tuned (e.g.
>> by NTP) or if it is set to a future time. However, if the host is tuned
>> backward, e.g. because NTP obtained the correct time after the guest was
>> already started or the admin decided to tune the local time, we see an
>> unpleasant effect in the guest: The RTC will stall for the period the
>> host clock is set back.
>>
>> This series tries to address the issue more gracefully. By detecting
>> those warps and providing a callback mechanism to device models, the
>> RTC is enabled to update its timers and register content immediately.
>> Tested successfully with a hwclock readout loop in a Linux guest while
>> fiddling with the host time.
>>
>> Note that if this kind of RTC adjustment is not wanted, the user is
>> still free to decouple the RTC from the host clock and base it on the
>> VM clock - just like before.
>>    
> 
> Did you test this with a Windows guest?  They rely heavily on RTC, this 
> is probably a better behavior for that case.  I'd be curious if Windows 
> accepts the RTC register changing underneath it, but based on earlier 
> versions of Windows Time Service, would be surprised if it did not.

I haven't tried with Windows yet. When does it read the RTC and how can
I check the outcome?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/3] Let RTC follow backward jumps of host clock immediately

[Gleb Natapov]

On Wed, Jan 12, 2011 at 12:26:25PM +0100, Jan Kiszka wrote:
> Am 23.12.2010 21:45, Zachary Amsden wrote:
> > On 12/17/2010 04:58 AM, Jan Kiszka wrote:
> >> By default, we base the mc146818 RTC on the host clock (CLOCK_REALTIME).
> >> This works fine if only the frequency of the host clock is tuned (e.g.
> >> by NTP) or if it is set to a future time. However, if the host is tuned
> >> backward, e.g. because NTP obtained the correct time after the guest was
> >> already started or the admin decided to tune the local time, we see an
> >> unpleasant effect in the guest: The RTC will stall for the period the
> >> host clock is set back.
> >>
> >> This series tries to address the issue more gracefully. By detecting
> >> those warps and providing a callback mechanism to device models, the
> >> RTC is enabled to update its timers and register content immediately.
> >> Tested successfully with a hwclock readout loop in a Linux guest while
> >> fiddling with the host time.
> >>
> >> Note that if this kind of RTC adjustment is not wanted, the user is
> >> still free to decouple the RTC from the host clock and base it on the
> >> VM clock - just like before.
> >>    
> > 
> > Did you test this with a Windows guest?  They rely heavily on RTC, this 
> > is probably a better behavior for that case.  I'd be curious if Windows 
> > accepts the RTC register changing underneath it, but based on earlier 
> > versions of Windows Time Service, would be surprised if it did not.
> 
> I haven't tried with Windows yet. When does it read the RTC and how can
> I check the outcome?
> 
Windows relies on timely delivery of RTC interrupts to calculate wall
clock. If, dues to the stall described above, interrupts will not be
delivered for some period of time Windows guest may experience time
drift.

--
			Gleb.

Re: [Qemu-devel] [RFC][PATCH 0/3] Let RTC follow backward jumps of host clock immediately

[Jan Kiszka]

Am 12.01.2011 13:27, Gleb Natapov wrote:
> On Wed, Jan 12, 2011 at 12:26:25PM +0100, Jan Kiszka wrote:
>> Am 23.12.2010 21:45, Zachary Amsden wrote:
>>> On 12/17/2010 04:58 AM, Jan Kiszka wrote:
>>>> By default, we base the mc146818 RTC on the host clock (CLOCK_REALTIME).
>>>> This works fine if only the frequency of the host clock is tuned (e.g.
>>>> by NTP) or if it is set to a future time. However, if the host is tuned
>>>> backward, e.g. because NTP obtained the correct time after the guest was
>>>> already started or the admin decided to tune the local time, we see an
>>>> unpleasant effect in the guest: The RTC will stall for the period the
>>>> host clock is set back.
>>>>
>>>> This series tries to address the issue more gracefully. By detecting
>>>> those warps and providing a callback mechanism to device models, the
>>>> RTC is enabled to update its timers and register content immediately.
>>>> Tested successfully with a hwclock readout loop in a Linux guest while
>>>> fiddling with the host time.
>>>>
>>>> Note that if this kind of RTC adjustment is not wanted, the user is
>>>> still free to decouple the RTC from the host clock and base it on the
>>>> VM clock - just like before.
>>>>    
>>>
>>> Did you test this with a Windows guest?  They rely heavily on RTC, this 
>>> is probably a better behavior for that case.  I'd be curious if Windows 
>>> accepts the RTC register changing underneath it, but based on earlier 
>>> versions of Windows Time Service, would be surprised if it did not.
>>
>> I haven't tried with Windows yet. When does it read the RTC and how can
>> I check the outcome?
>>
> Windows relies on timely delivery of RTC interrupts to calculate wall
> clock. If, dues to the stall described above, interrupts will not be
> delivered for some period of time Windows guest may experience time
> drift.

Ah, now I remember again. Will check the behavior before/after the patch.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [Qemu-devel] [RFC][PATCH 0/3] Let RTC follow backward jumps of host clock immediately

[Jan Kiszka]

Am 12.01.2011 14:05, Jan Kiszka wrote:
> Am 12.01.2011 13:27, Gleb Natapov wrote:
>> On Wed, Jan 12, 2011 at 12:26:25PM +0100, Jan Kiszka wrote:
>>> Am 23.12.2010 21:45, Zachary Amsden wrote:
>>>> On 12/17/2010 04:58 AM, Jan Kiszka wrote:
>>>>> By default, we base the mc146818 RTC on the host clock (CLOCK_REALTIME).
>>>>> This works fine if only the frequency of the host clock is tuned (e.g.
>>>>> by NTP) or if it is set to a future time. However, if the host is tuned
>>>>> backward, e.g. because NTP obtained the correct time after the guest was
>>>>> already started or the admin decided to tune the local time, we see an
>>>>> unpleasant effect in the guest: The RTC will stall for the period the
>>>>> host clock is set back.
>>>>>
>>>>> This series tries to address the issue more gracefully. By detecting
>>>>> those warps and providing a callback mechanism to device models, the
>>>>> RTC is enabled to update its timers and register content immediately.
>>>>> Tested successfully with a hwclock readout loop in a Linux guest while
>>>>> fiddling with the host time.
>>>>>
>>>>> Note that if this kind of RTC adjustment is not wanted, the user is
>>>>> still free to decouple the RTC from the host clock and base it on the
>>>>> VM clock - just like before.
>>>>>    
>>>>
>>>> Did you test this with a Windows guest?  They rely heavily on RTC, this 
>>>> is probably a better behavior for that case.  I'd be curious if Windows 
>>>> accepts the RTC register changing underneath it, but based on earlier 
>>>> versions of Windows Time Service, would be surprised if it did not.
>>>
>>> I haven't tried with Windows yet. When does it read the RTC and how can
>>> I check the outcome?
>>>
>> Windows relies on timely delivery of RTC interrupts to calculate wall
>> clock. If, dues to the stall described above, interrupts will not be
>> delivered for some period of time Windows guest may experience time
>> drift.
> 
> Ah, now I remember again. Will check the behavior before/after the patch.

Looks good: Without my patches, Windows' clock stops to tick once I
reverse the host time by a significant period. With the patches, the
guest clock proceeds apparently unaffectedly. So this series should be
want-to-have for Windows virtualization as well.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[Qemu-devel] Re: [RFC][PATCH 0/3] Let RTC follow backward jumps of host clock immediately

[Jan Kiszka]

On 2010-12-17 15:58, Jan Kiszka wrote:
> By default, we base the mc146818 RTC on the host clock (CLOCK_REALTIME).
> This works fine if only the frequency of the host clock is tuned (e.g.
> by NTP) or if it is set to a future time. However, if the host is tuned
> backward, e.g. because NTP obtained the correct time after the guest was
> already started or the admin decided to tune the local time, we see an
> unpleasant effect in the guest: The RTC will stall for the period the
> host clock is set back.
> 
> This series tries to address the issue more gracefully. By detecting
> those warps and providing a callback mechanism to device models, the
> RTC is enabled to update its timers and register content immediately.
> Tested successfully with a hwclock readout loop in a Linux guest while
> fiddling with the host time.
> 
> Note that if this kind of RTC adjustment is not wanted, the user is
> still free to decouple the RTC from the host clock and base it on the
> VM clock - just like before.
> 
> Jan Kiszka (3):
>   qemu-timer: Consolidate qemu_get_clock and qemu_get_clock_ns
>   qemu-timer: Introduce warp callback
>   mc146818rtc: Handle host clock warps
> 
>  hw/mc146818rtc.c |   17 ++++++++++++
>  qemu-timer.c     |   77 ++++++++++++++++++++++++++++++++++++++++++++----------
>  qemu-timer.h     |    5 +++
>  3 files changed, 85 insertions(+), 14 deletions(-)
> 

Ping?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux