[Qemu-devel] [for 4.2 PATCH v2 0/4] Remove time reset notifications

[Qemu-devel] [for 4.2 PATCH v2 0/4] Remove time reset notifications

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Timers have a mechanism for detecting host clock jumps; this relied
on noticing if the time had gone backwards or if it had gone forward
more than 60s since we last read it.  This had assumed that we regularly
read the time, which isn't true any more - we might not read the host
timer until the guest explicitly reads the guest RTC (e.g. hwclock).
This falsely triggers the reset mechanism.
 
The reset mechanism was only used by the mc146818 (i.e. PC) RTC
anyway; so lets remove it.

v2
  Remove the host_clock_last field in replay as per Pavel's review

Dr. David Alan Gilbert (4):
  mc146818rtc: Remove reset notifiers
  timer: Remove reset notifiers
  replay: Remove host_clock_last
  timer: last, remove last bits of last

 hw/timer/mc146818rtc.c   | 19 -------------------
 include/qemu/timer.h     | 35 ----------------------------------
 replay/replay-snapshot.c |  7 ++-----
 util/qemu-timer.c        | 41 +---------------------------------------
 4 files changed, 3 insertions(+), 99 deletions(-)

-- 
2.21.0

[Qemu-devel] [for 4.2 PATCH v2 1/4] mc146818rtc: Remove reset notifiers

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The reset notifiers are unreliable and recalculating the offsets
after boot causes problems with migration in cases where explicit
base times are set on the destination.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 hw/timer/mc146818rtc.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/hw/timer/mc146818rtc.c b/hw/timer/mc146818rtc.c
index ce4550b6f2..352ff9d395 100644
--- a/hw/timer/mc146818rtc.c
+++ b/hw/timer/mc146818rtc.c
@@ -92,7 +92,6 @@ typedef struct RTCState {
     uint32_t irq_coalesced;
     uint32_t period;
     QEMUTimer *coalesced_timer;
-    Notifier clock_reset_notifier;
     LostTickPolicy lost_tick_policy;
     Notifier suspend_notifier;
     QLIST_ENTRY(RTCState) link;
@@ -885,20 +884,6 @@ static const VMStateDescription vmstate_rtc = {
     }
 };
 
-static void rtc_notify_clock_reset(Notifier *notifier, void *data)
-{
-    RTCState *s = container_of(notifier, RTCState, clock_reset_notifier);
-    int64_t now = *(int64_t *)data;
-
-    rtc_set_date_from_host(ISA_DEVICE(s));
-    periodic_timer_update(s, now, 0);
-    check_update_timer(s);
-
-    if (s->lost_tick_policy == LOST_TICK_POLICY_SLEW) {
-        rtc_coalesced_timer_update(s);
-    }
-}
-
 /* set CMOS shutdown status register (index 0xF) as S3_resume(0xFE)
    BIOS will read it and start S3 resume at POST Entry */
 static void rtc_notify_suspend(Notifier *notifier, void *data)
@@ -984,10 +969,6 @@ static void rtc_realizefn(DeviceState *dev, Error **errp)
     s->update_timer = timer_new_ns(rtc_clock, rtc_update_timer, s);
     check_update_timer(s);
 
-    s->clock_reset_notifier.notify = rtc_notify_clock_reset;
-    qemu_clock_register_reset_notifier(rtc_clock,
-                                       &s->clock_reset_notifier);
-
     s->suspend_notifier.notify = rtc_notify_suspend;
     qemu_register_suspend_notifier(&s->suspend_notifier);
 
-- 
2.21.0

[Qemu-devel] [for 4.2 PATCH v2 2/4] timer: Remove reset notifiers

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Remove the reset notifer from the core qemu-timer code.
The only user was mc146818 and we've just remove it's use.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/qemu/timer.h | 22 ----------------------
 util/qemu-timer.c    | 21 +--------------------
 2 files changed, 1 insertion(+), 42 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 5d978e1634..6817c78ef4 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -227,28 +227,6 @@ void qemu_clock_enable(QEMUClockType type, bool enabled);
  */
 void qemu_start_warp_timer(void);
 
-/**
- * qemu_clock_register_reset_notifier:
- * @type: the clock type
- * @notifier: the notifier function
- *
- * Register a notifier function to call when the clock
- * concerned is reset.
- */
-void qemu_clock_register_reset_notifier(QEMUClockType type,
-                                        Notifier *notifier);
-
-/**
- * qemu_clock_unregister_reset_notifier:
- * @type: the clock type
- * @notifier: the notifier function
- *
- * Unregister a notifier function to call when the clock
- * concerned is reset.
- */
-void qemu_clock_unregister_reset_notifier(QEMUClockType type,
-                                          Notifier *notifier);
-
 /**
  * qemu_clock_run_timers:
  * @type: clock on which to operate
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 1cc1b2f2c3..2faaaf9926 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -48,7 +48,6 @@ typedef struct QEMUClock {
     /* We rely on BQL to protect the timerlists */
     QLIST_HEAD(, QEMUTimerList) timerlists;
 
-    NotifierList reset_notifiers;
     int64_t last;
 
     QEMUClockType type;
@@ -133,7 +132,6 @@ static void qemu_clock_init(QEMUClockType type, QEMUTimerListNotifyCB *notify_cb
     clock->enabled = (type == QEMU_CLOCK_VIRTUAL ? false : true);
     clock->last = INT64_MIN;
     QLIST_INIT(&clock->timerlists);
-    notifier_list_init(&clock->reset_notifiers);
     main_loop_tlg.tl[type] = timerlist_new(type, notify_cb, NULL);
 }
 
@@ -630,7 +628,7 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg)
 
 int64_t qemu_clock_get_ns(QEMUClockType type)
 {
-    int64_t now, last;
+    int64_t now;
     QEMUClock *clock = qemu_clock_ptr(type);
 
     switch (type) {
@@ -645,11 +643,7 @@ int64_t qemu_clock_get_ns(QEMUClockType type)
         }
     case QEMU_CLOCK_HOST:
         now = REPLAY_CLOCK(REPLAY_CLOCK_HOST, get_clock_realtime());
-        last = clock->last;
         clock->last = now;
-        if (now < last || now > (last + get_max_clock_jump())) {
-            notifier_list_notify(&clock->reset_notifiers, &now);
-        }
         return now;
     case QEMU_CLOCK_VIRTUAL_RT:
         return REPLAY_CLOCK(REPLAY_CLOCK_VIRTUAL_RT, cpu_get_clock());
@@ -668,19 +662,6 @@ void qemu_clock_set_last(QEMUClockType type, uint64_t last)
     clock->last = last;
 }
 
-void qemu_clock_register_reset_notifier(QEMUClockType type,
-                                        Notifier *notifier)
-{
-    QEMUClock *clock = qemu_clock_ptr(type);
-    notifier_list_add(&clock->reset_notifiers, notifier);
-}
-
-void qemu_clock_unregister_reset_notifier(QEMUClockType type,
-                                          Notifier *notifier)
-{
-    notifier_remove(notifier);
-}
-
 void init_clocks(QEMUTimerListNotifyCB *notify_cb)
 {
     QEMUClockType type;
-- 
2.21.0

[Qemu-devel] [for 4.2 PATCH v2 3/4] replay: Remove host_clock_last

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Now we're not using the 'last' field in the timer, remove it from
replay.

Bump the version number of the replay structure since we've
removed the field.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 replay/replay-snapshot.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
index 756f48bc02..3a58734b9a 100644
--- a/replay/replay-snapshot.c
+++ b/replay/replay-snapshot.c
@@ -24,7 +24,6 @@ static int replay_pre_save(void *opaque)
 {
     ReplayState *state = opaque;
     state->file_offset = ftell(replay_file);
-    state->host_clock_last = qemu_clock_get_last(QEMU_CLOCK_HOST);
 
     return 0;
 }
@@ -34,7 +33,6 @@ static int replay_post_load(void *opaque, int version_id)
     ReplayState *state = opaque;
     if (replay_mode == REPLAY_MODE_PLAY) {
         fseek(replay_file, state->file_offset, SEEK_SET);
-        qemu_clock_set_last(QEMU_CLOCK_HOST, state->host_clock_last);
         /* If this was a vmstate, saved in recording mode,
            we need to initialize replay data fields. */
         replay_fetch_data_kind();
@@ -50,8 +48,8 @@ static int replay_post_load(void *opaque, int version_id)
 
 static const VMStateDescription vmstate_replay = {
     .name = "replay",
-    .version_id = 1,
-    .minimum_version_id = 1,
+    .version_id = 2,
+    .minimum_version_id = 2,
     .pre_save = replay_pre_save,
     .post_load = replay_post_load,
     .fields = (VMStateField[]) {
@@ -62,7 +60,6 @@ static const VMStateDescription vmstate_replay = {
         VMSTATE_UINT32(has_unread_data, ReplayState),
         VMSTATE_UINT64(file_offset, ReplayState),
         VMSTATE_UINT64(block_request_id, ReplayState),
-        VMSTATE_UINT64(host_clock_last, ReplayState),
         VMSTATE_INT32(read_event_kind, ReplayState),
         VMSTATE_UINT64(read_event_id, ReplayState),
         VMSTATE_INT32(read_event_checkpoint, ReplayState),
-- 
2.21.0

[Qemu-devel] [for 4.2 PATCH v2 4/4] timer: last, remove last bits of last

[Dr. David Alan Gilbert (git)]

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

The reset notifiers kept a 'last' counter to notice jumps;
now that we've remove the notifier we don't need to keep 'last'.

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 include/qemu/timer.h | 13 -------------
 util/qemu-timer.c    | 22 +---------------------
 2 files changed, 1 insertion(+), 34 deletions(-)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 6817c78ef4..5bcab935f6 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -248,19 +248,6 @@ bool qemu_clock_run_timers(QEMUClockType type);
  */
 bool qemu_clock_run_all_timers(void);
 
-/**
- * qemu_clock_get_last:
- *
- * Returns last clock query time.
- */
-uint64_t qemu_clock_get_last(QEMUClockType type);
-/**
- * qemu_clock_set_last:
- *
- * Sets last clock query time.
- */
-void qemu_clock_set_last(QEMUClockType type, uint64_t last);
-
 
 /*
  * QEMUTimerList
diff --git a/util/qemu-timer.c b/util/qemu-timer.c
index 2faaaf9926..b73041df4e 100644
--- a/util/qemu-timer.c
+++ b/util/qemu-timer.c
@@ -48,8 +48,6 @@ typedef struct QEMUClock {
     /* We rely on BQL to protect the timerlists */
     QLIST_HEAD(, QEMUTimerList) timerlists;
 
-    int64_t last;
-
     QEMUClockType type;
     bool enabled;
 } QEMUClock;
@@ -130,7 +128,6 @@ static void qemu_clock_init(QEMUClockType type, QEMUTimerListNotifyCB *notify_cb
 
     clock->type = type;
     clock->enabled = (type == QEMU_CLOCK_VIRTUAL ? false : true);
-    clock->last = INT64_MIN;
     QLIST_INIT(&clock->timerlists);
     main_loop_tlg.tl[type] = timerlist_new(type, notify_cb, NULL);
 }
@@ -628,9 +625,6 @@ int64_t timerlistgroup_deadline_ns(QEMUTimerListGroup *tlg)
 
 int64_t qemu_clock_get_ns(QEMUClockType type)
 {
-    int64_t now;
-    QEMUClock *clock = qemu_clock_ptr(type);
-
     switch (type) {
     case QEMU_CLOCK_REALTIME:
         return get_clock();
@@ -642,26 +636,12 @@ int64_t qemu_clock_get_ns(QEMUClockType type)
             return cpu_get_clock();
         }
     case QEMU_CLOCK_HOST:
-        now = REPLAY_CLOCK(REPLAY_CLOCK_HOST, get_clock_realtime());
-        clock->last = now;
-        return now;
+        return REPLAY_CLOCK(REPLAY_CLOCK_HOST, get_clock_realtime());
     case QEMU_CLOCK_VIRTUAL_RT:
         return REPLAY_CLOCK(REPLAY_CLOCK_VIRTUAL_RT, cpu_get_clock());
     }
 }
 
-uint64_t qemu_clock_get_last(QEMUClockType type)
-{
-    QEMUClock *clock = qemu_clock_ptr(type);
-    return clock->last;
-}
-
-void qemu_clock_set_last(QEMUClockType type, uint64_t last)
-{
-    QEMUClock *clock = qemu_clock_ptr(type);
-    clock->last = last;
-}
-
 void init_clocks(QEMUTimerListNotifyCB *notify_cb)
 {
     QEMUClockType type;
-- 
2.21.0

Re: [Qemu-devel] [for 4.2 PATCH v2 3/4] replay: Remove host_clock_last

[Pavel Dovgalyuk]

Reviewed-by: Pavel Dovgalyuk <Pavel.Dovgaluk@ispras.ru>


Pavel Dovgalyuk

> -----Original Message-----
> From: Dr. David Alan Gilbert (git) [mailto:dgilbert@redhat.com]
> Sent: Wednesday, July 24, 2019 2:58 PM
> To: qemu-devel@nongnu.org; pbonzini@redhat.com; pavel.dovgaluk@ispras.ru
> Subject: [for 4.2 PATCH v2 3/4] replay: Remove host_clock_last
> 
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> Now we're not using the 'last' field in the timer, remove it from
> replay.
> 
> Bump the version number of the replay structure since we've
> removed the field.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  replay/replay-snapshot.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/replay/replay-snapshot.c b/replay/replay-snapshot.c
> index 756f48bc02..3a58734b9a 100644
> --- a/replay/replay-snapshot.c
> +++ b/replay/replay-snapshot.c
> @@ -24,7 +24,6 @@ static int replay_pre_save(void *opaque)
>  {
>      ReplayState *state = opaque;
>      state->file_offset = ftell(replay_file);
> -    state->host_clock_last = qemu_clock_get_last(QEMU_CLOCK_HOST);
> 
>      return 0;
>  }
> @@ -34,7 +33,6 @@ static int replay_post_load(void *opaque, int version_id)
>      ReplayState *state = opaque;
>      if (replay_mode == REPLAY_MODE_PLAY) {
>          fseek(replay_file, state->file_offset, SEEK_SET);
> -        qemu_clock_set_last(QEMU_CLOCK_HOST, state->host_clock_last);
>          /* If this was a vmstate, saved in recording mode,
>             we need to initialize replay data fields. */
>          replay_fetch_data_kind();
> @@ -50,8 +48,8 @@ static int replay_post_load(void *opaque, int version_id)
> 
>  static const VMStateDescription vmstate_replay = {
>      .name = "replay",
> -    .version_id = 1,
> -    .minimum_version_id = 1,
> +    .version_id = 2,
> +    .minimum_version_id = 2,
>      .pre_save = replay_pre_save,
>      .post_load = replay_post_load,
>      .fields = (VMStateField[]) {
> @@ -62,7 +60,6 @@ static const VMStateDescription vmstate_replay = {
>          VMSTATE_UINT32(has_unread_data, ReplayState),
>          VMSTATE_UINT64(file_offset, ReplayState),
>          VMSTATE_UINT64(block_request_id, ReplayState),
> -        VMSTATE_UINT64(host_clock_last, ReplayState),
>          VMSTATE_INT32(read_event_kind, ReplayState),
>          VMSTATE_UINT64(read_event_id, ReplayState),
>          VMSTATE_INT32(read_event_checkpoint, ReplayState),
> --
> 2.21.0