qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o
@ 2009-12-21  8:09 Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 01/19] centralize handling of -icount Paolo Bonzini
                   ` (19 more replies)
  0 siblings, 20 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

This series makes a few cleanup in the timer handling code and
splits out ~1500 lines out of the huge vl.o file.  So far I've tested
it by booting a live CD both under Linux and by cross-compiling to
Windows.  If the series is considered helpful, I can test further
including actually running the Windows version (Wine doesn't work).

Paolo Bonzini (19):
  centralize handling of -icount
  add qemu_icount_round
  avoid dubiously clever code in win32_start_timer
  fix error in win32_rearm_timer
  only one flag is needed for alarm_timer
  more alarm timer cleanup
  add qemu_get_clock_ns
  move kbd/mouse events to event.c
  remove qemu_rearm_alarm_timer from main loop
  add qemu_bh_scheduled
  use a bottom half to run timers
  new function qemu_icount_delta
  move tcg_has_work to cpu-exec.c and rename it
  disentangle tcg and deadline calculation
  do not provide qemu_event_increment if iothread not used
  tweak qemu_notify_event
  move vmstate registration of vmstate_timers earlier
  introduce qemu_clock_enable
  split out qemu-timer.c

 Makefile        |    2 +-
 Makefile.target |    1 +
 async.c         |    5 +
 cpu-all.h       |    4 +-
 cpu-exec.c      |   16 +-
 event.c         |  238 +++++++++
 hw/xenfb.c      |    6 +-
 qemu-common.h   |    2 +
 qemu-timer.c    | 1218 ++++++++++++++++++++++++++++++++++++++++++++
 qemu-timer.h    |   12 +
 sysemu.h        |    2 +-
 vl.c            | 1529 ++++---------------------------------------------------
 12 files changed, 1592 insertions(+), 1443 deletions(-)
 create mode 100644 event.c
 create mode 100644 qemu-timer.c

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

* [Qemu-devel] [PATCH 01/19] centralize handling of -icount
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 02/19] add qemu_icount_round Paolo Bonzini
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

A simple patch to place together all handling of -icount.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |   33 +++++++++++++++++++--------------
 1 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/vl.c b/vl.c
index c0d98f5..e38cea7 100644
--- a/vl.c
+++ b/vl.c
@@ -890,8 +890,23 @@ static void icount_adjust_vm(void * opaque)
     icount_adjust();
 }
 
-static void init_icount_adjust(void)
+static void configure_icount(const char *option)
 {
+    if (!option)
+        return;
+
+    if (strcmp(option, "auto") != 0) {
+        icount_time_shift = strtol(option, NULL, 0);
+        use_icount = 1;
+        return;
+    }
+
+    use_icount = 2;
+
+    /* 125MIPS seems a reasonable initial guess at the guest speed.
+       It will be corrected fairly quickly anyway.  */
+    icount_time_shift = 3;
+
     /* Have both realtime and virtual time triggers for speed adjustment.
        The realtime trigger catches emulated time passing too slowly,
        the virtual time trigger catches emulated time passing too fast.
@@ -4858,6 +4873,7 @@ int main(int argc, char **argv, char **envp)
     uint32_t boot_devices_bitmap = 0;
     int i;
     int snapshot, linux_boot, net_boot;
+    const char *icount_option = NULL;
     const char *initrd_filename;
     const char *kernel_filename, *kernel_cmdline;
     char boot_devices[33] = "cad"; /* default to HD->floppy->CD-ROM */
@@ -5578,12 +5594,7 @@ int main(int argc, char **argv, char **envp)
                     tb_size = 0;
                 break;
             case QEMU_OPTION_icount:
-                use_icount = 1;
-                if (strcmp(optarg, "auto") == 0) {
-                    icount_time_shift = -1;
-                } else {
-                    icount_time_shift = strtol(optarg, NULL, 0);
-                }
+                icount_option = optarg;
                 break;
             case QEMU_OPTION_incoming:
                 incoming = optarg;
@@ -5811,13 +5822,7 @@ int main(int argc, char **argv, char **envp)
         fprintf(stderr, "could not initialize alarm timer\n");
         exit(1);
     }
-    if (use_icount && icount_time_shift < 0) {
-        use_icount = 2;
-        /* 125MIPS seems a reasonable initial guess at the guest speed.
-           It will be corrected fairly quickly anyway.  */
-        icount_time_shift = 3;
-        init_icount_adjust();
-    }
+    configure_icount(icount_option);
 
 #ifdef _WIN32
     socket_init();
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 02/19] add qemu_icount_round
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 01/19] centralize handling of -icount Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 03/19] avoid dubiously clever code in win32_start_timer Paolo Bonzini
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/vl.c b/vl.c
index e38cea7..97410ad 100644
--- a/vl.c
+++ b/vl.c
@@ -920,6 +920,11 @@ static void configure_icount(const char *option)
                    qemu_get_clock(vm_clock) + get_ticks_per_sec() / 10);
 }
 
+static int64_t qemu_icount_round(int64_t count)
+{
+    return (count + (1 << icount_time_shift) - 1) >> icount_time_shift;
+}
+
 static struct qemu_alarm_timer alarm_timers[] = {
 #ifndef _WIN32
 #ifdef __linux__
@@ -4031,9 +4036,7 @@ static int qemu_cpu_exec(CPUState *env)
         qemu_icount -= (env->icount_decr.u16.low + env->icount_extra);
         env->icount_decr.u16.low = 0;
         env->icount_extra = 0;
-        count = qemu_next_deadline();
-        count = (count + (1 << icount_time_shift) - 1)
-                >> icount_time_shift;
+        count = qemu_icount_round (qemu_next_deadline());
         qemu_icount += count;
         decr = (count > 0xffff) ? 0xffff : count;
         count -= decr;
@@ -4141,9 +4144,7 @@ static int qemu_calculate_timeout(void)
             if (add > 10000000)
                 add = 10000000;
             delta += add;
-            add = (add + (1 << icount_time_shift) - 1)
-                  >> icount_time_shift;
-            qemu_icount += add;
+            qemu_icount += qemu_icount_round (add);
             timeout = delta / 1000000;
             if (timeout < 0)
                 timeout = 0;
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 03/19] avoid dubiously clever code in win32_start_timer
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 01/19] centralize handling of -icount Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 02/19] add qemu_icount_round Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2010-01-04 19:34   ` Anthony Liguori
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 04/19] fix error in win32_rearm_timer Paolo Bonzini
                   ` (16 subsequent siblings)
  19 siblings, 1 reply; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

The code is initializing an unsigned int to UINT_MAX using "-1", so that
the following always-true comparison seems to be always-false at a
first look.  Just remove the if.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/vl.c b/vl.c
index 97410ad..22ec53d 100644
--- a/vl.c
+++ b/vl.c
@@ -813,7 +813,7 @@ static struct qemu_alarm_timer *alarm_timer;
 struct qemu_alarm_win32 {
     MMRESULT timerId;
     unsigned int period;
-} alarm_win32_data = {0, -1};
+} alarm_win32_data = {0, 0};
 
 static int win32_start_timer(struct qemu_alarm_timer *t);
 static void win32_stop_timer(struct qemu_alarm_timer *t);
@@ -1550,9 +1550,7 @@ static int win32_start_timer(struct qemu_alarm_timer *t)
     memset(&tc, 0, sizeof(tc));
     timeGetDevCaps(&tc, sizeof(tc));
 
-    if (data->period < tc.wPeriodMin)
-        data->period = tc.wPeriodMin;
-
+    data->period = tc.wPeriodMin;
     timeBeginPeriod(data->period);
 
     flags = TIME_CALLBACK_FUNCTION;
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 04/19] fix error in win32_rearm_timer
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (2 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 03/19] avoid dubiously clever code in win32_start_timer Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 05/19] only one flag is needed for alarm_timer Paolo Bonzini
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

The TIME_ONESHOT and TIME_PERIODIC flags are mutually exclusive.
The code after the patch matches the flags used in win32_start_timer.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index 22ec53d..58e57c1 100644
--- a/vl.c
+++ b/vl.c
@@ -1598,7 +1598,7 @@ static void win32_rearm_timer(struct qemu_alarm_timer *t)
                         data->period,
                         host_alarm_handler,
                         (DWORD)t,
-                        TIME_ONESHOT | TIME_PERIODIC);
+                        TIME_ONESHOT | TIME_CALLBACK_FUNCTION);
 
     if (!data->timerId) {
         fprintf(stderr, "Failed to re-arm win32 alarm timer %ld\n",
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 05/19] only one flag is needed for alarm_timer
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (3 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 04/19] fix error in win32_rearm_timer Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 06/19] more alarm timer cleanup Paolo Bonzini
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

The ALARM_FLAG_DYNTICKS can be testing simply by checking if there is
a rearm function.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |   31 +++++++++++++++----------------
 1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/vl.c b/vl.c
index 58e57c1..efffaf1 100644
--- a/vl.c
+++ b/vl.c
@@ -779,20 +779,17 @@ struct QEMUTimer {
 
 struct qemu_alarm_timer {
     char const *name;
-    unsigned int flags;
-
     int (*start)(struct qemu_alarm_timer *t);
     void (*stop)(struct qemu_alarm_timer *t);
     void (*rearm)(struct qemu_alarm_timer *t);
     void *priv;
-};
 
-#define ALARM_FLAG_DYNTICKS  0x1
-#define ALARM_FLAG_EXPIRED   0x2
+    unsigned int expired;
+};
 
 static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
 {
-    return t && (t->flags & ALARM_FLAG_DYNTICKS);
+    return t && t->rearm;
 }
 
 static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
@@ -928,18 +925,18 @@ static int64_t qemu_icount_round(int64_t count)
 static struct qemu_alarm_timer alarm_timers[] = {
 #ifndef _WIN32
 #ifdef __linux__
-    {"dynticks", ALARM_FLAG_DYNTICKS, dynticks_start_timer,
+    {"dynticks", dynticks_start_timer,
      dynticks_stop_timer, dynticks_rearm_timer, NULL},
     /* HPET - if available - is preferred */
-    {"hpet", 0, hpet_start_timer, hpet_stop_timer, NULL, NULL},
+    {"hpet", hpet_start_timer, hpet_stop_timer, NULL, NULL},
     /* ...otherwise try RTC */
-    {"rtc", 0, rtc_start_timer, rtc_stop_timer, NULL, NULL},
+    {"rtc", rtc_start_timer, rtc_stop_timer, NULL, NULL},
 #endif
-    {"unix", 0, unix_start_timer, unix_stop_timer, NULL, NULL},
+    {"unix", unix_start_timer, unix_stop_timer, NULL, NULL},
 #else
-    {"dynticks", ALARM_FLAG_DYNTICKS, win32_start_timer,
+    {"dynticks", win32_start_timer,
      win32_stop_timer, win32_rearm_timer, &alarm_win32_data},
-    {"win32", 0, win32_start_timer,
+    {"win32", win32_start_timer,
      win32_stop_timer, NULL, &alarm_win32_data},
 #endif
     {NULL, }
@@ -1087,7 +1084,7 @@ void qemu_mod_timer(QEMUTimer *ts, int64_t expire_time)
 
     /* Rearm if necessary  */
     if (pt == &active_timers[ts->clock->type]) {
-        if ((alarm_timer->flags & ALARM_FLAG_EXPIRED) == 0) {
+        if (!alarm_timer->expired) {
             qemu_rearm_alarm_timer(alarm_timer);
         }
         /* Interrupt execution to force deadline recalculation.  */
@@ -1243,7 +1240,7 @@ static void host_alarm_handler(int host_signum)
         qemu_timer_expired(active_timers[QEMU_CLOCK_HOST],
                            qemu_get_clock(host_clock))) {
         qemu_event_increment();
-        if (alarm_timer) alarm_timer->flags |= ALARM_FLAG_EXPIRED;
+        if (alarm_timer) alarm_timer->expired = 1;
 
 #ifndef CONFIG_IOTHREAD
         if (next_cpu) {
@@ -1472,6 +1469,7 @@ static void dynticks_rearm_timer(struct qemu_alarm_timer *t)
     int64_t nearest_delta_us = INT64_MAX;
     int64_t current_us;
 
+    assert(alarm_has_dynticks(t));
     if (!active_timers[QEMU_CLOCK_REALTIME] &&
         !active_timers[QEMU_CLOCK_VIRTUAL] &&
         !active_timers[QEMU_CLOCK_HOST])
@@ -1587,6 +1585,7 @@ static void win32_rearm_timer(struct qemu_alarm_timer *t)
 {
     struct qemu_alarm_win32 *data = t->priv;
 
+    assert(alarm_has_dynticks(t));
     if (!active_timers[QEMU_CLOCK_REALTIME] &&
         !active_timers[QEMU_CLOCK_VIRTUAL] &&
         !active_timers[QEMU_CLOCK_HOST])
@@ -3993,8 +3992,8 @@ void main_loop_wait(int timeout)
     slirp_select_poll(&rfds, &wfds, &xfds, (ret < 0));
 
     /* rearm timer, if not periodic */
-    if (alarm_timer->flags & ALARM_FLAG_EXPIRED) {
-        alarm_timer->flags &= ~ALARM_FLAG_EXPIRED;
+    if (alarm_timer->expired) {
+        alarm_timer->expired = 0;
         qemu_rearm_alarm_timer(alarm_timer);
     }
 
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 06/19] more alarm timer cleanup
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (4 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 05/19] only one flag is needed for alarm_timer Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 07/19] add qemu_get_clock_ns Paolo Bonzini
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

The timer_alarm_pending variable is related to the alarm timer but not
placed in the struct.  Also, in qemu_mod_timer the wrong flag was being
tested: the timer is rearmed in the alarm timer "bottom half", so the
right flag to test there is the "pending" flag.

Finally, I hoisted the NULL checks from alarm_has_dynticks to
host_alarm_handler.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |   29 ++++++++++++++++++-----------
 1 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/vl.c b/vl.c
index efffaf1..1af2f81 100644
--- a/vl.c
+++ b/vl.c
@@ -253,7 +253,6 @@ uint64_t node_cpumask[MAX_NODES];
 
 static CPUState *cur_cpu;
 static CPUState *next_cpu;
-static int timer_alarm_pending = 1;
 /* Conversion factor from emulated instructions to virtual clock ticks.  */
 static int icount_time_shift;
 /* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
@@ -784,12 +783,13 @@ struct qemu_alarm_timer {
     void (*rearm)(struct qemu_alarm_timer *t);
     void *priv;
 
-    unsigned int expired;
+    char expired;
+    char pending;
 };
 
 static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
 {
-    return t && t->rearm;
+    return !!t->rearm;
 }
 
 static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
@@ -1084,7 +1084,7 @@ void qemu_mod_timer(QEMUTimer *ts, int64_t expire_time)
 
     /* Rearm if necessary  */
     if (pt == &active_timers[ts->clock->type]) {
-        if (!alarm_timer->expired) {
+        if (!alarm_timer->pending) {
             qemu_rearm_alarm_timer(alarm_timer);
         }
         /* Interrupt execution to force deadline recalculation.  */
@@ -1202,6 +1202,10 @@ static void CALLBACK host_alarm_handler(UINT uTimerID, UINT uMsg,
 static void host_alarm_handler(int host_signum)
 #endif
 {
+    struct qemu_alarm_timer *t = alarm_timer;
+    if (!t)
+	return;
+
 #if 0
 #define DISP_FREQ 1000
     {
@@ -1231,7 +1235,7 @@ static void host_alarm_handler(int host_signum)
         last_clock = ti;
     }
 #endif
-    if (alarm_has_dynticks(alarm_timer) ||
+    if (alarm_has_dynticks(t) ||
         (!use_icount &&
             qemu_timer_expired(active_timers[QEMU_CLOCK_VIRTUAL],
                                qemu_get_clock(vm_clock))) ||
@@ -1240,7 +1244,7 @@ static void host_alarm_handler(int host_signum)
         qemu_timer_expired(active_timers[QEMU_CLOCK_HOST],
                            qemu_get_clock(host_clock))) {
         qemu_event_increment();
-        if (alarm_timer) alarm_timer->expired = 1;
+        t->expired = alarm_has_dynticks(t);
 
 #ifndef CONFIG_IOTHREAD
         if (next_cpu) {
@@ -1248,7 +1252,7 @@ static void host_alarm_handler(int host_signum)
             cpu_exit(next_cpu);
         }
 #endif
-        timer_alarm_pending = 1;
+        t->pending = 1;
         qemu_notify_event();
     }
 }
@@ -1628,6 +1632,8 @@ static int init_timer_alarm(void)
         goto fail;
     }
 
+    /* first event is at time 0 */
+    t->pending = 1;
     alarm_timer = t;
 
     return 0;
@@ -1638,8 +1644,9 @@ fail:
 
 static void quit_timers(void)
 {
-    alarm_timer->stop(alarm_timer);
+    struct qemu_alarm_timer *t = alarm_timer;
     alarm_timer = NULL;
+    t->stop(t);
 }
 
 /***********************************************************/
@@ -3997,6 +4004,8 @@ void main_loop_wait(int timeout)
         qemu_rearm_alarm_timer(alarm_timer);
     }
 
+    alarm_timer->pending = 0;
+
     /* vm time timers */
     if (vm_running) {
         if (!cur_cpu || likely(!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER)))
@@ -4066,10 +4075,8 @@ static void tcg_cpu_exec(void)
 
         if (!vm_running)
             break;
-        if (timer_alarm_pending) {
-            timer_alarm_pending = 0;
+        if (alarm_timer->pending)
             break;
-        }
         if (cpu_can_run(env))
             ret = qemu_cpu_exec(env);
         if (ret == EXCP_DEBUG) {
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 07/19] add qemu_get_clock_ns
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (5 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 06/19] more alarm timer cleanup Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 08/19] move kbd/mouse events to event.c Paolo Bonzini
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

Some places use get_clock directly because they want to access the
rt_clock with nanosecond precision.  Add a function to do exactly that
instead of using internal interfaces.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-timer.h |    1 +
 vl.c         |   21 +++++++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/qemu-timer.h b/qemu-timer.h
index e7eaa04..c17b4e6 100644
--- a/qemu-timer.h
+++ b/qemu-timer.h
@@ -25,6 +25,7 @@ extern QEMUClock *vm_clock;
 extern QEMUClock *host_clock;
 
 int64_t qemu_get_clock(QEMUClock *clock);
+int64_t qemu_get_clock_ns(QEMUClock *clock);
 
 QEMUTimer *qemu_new_timer(QEMUClock *clock, QEMUTimerCB *cb, void *opaque);
 void qemu_free_timer(QEMUTimer *ts);
diff --git a/vl.c b/vl.c
index 1af2f81..797c56f 100644
--- a/vl.c
+++ b/vl.c
@@ -1144,6 +1144,23 @@ int64_t qemu_get_clock(QEMUClock *clock)
     }
 }
 
+int64_t qemu_get_clock_ns(QEMUClock *clock)
+{
+    switch(clock->type) {
+    case 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();
+    }
+}
+
 static void init_clocks(void)
 {
     init_get_clock();
@@ -3094,7 +3111,7 @@ static int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
     }
 
     bytes_transferred_last = bytes_transferred;
-    bwidth = get_clock();
+    bwidth = qemu_get_clock_ns(rt_clock);
 
     while (!qemu_file_rate_limit(f)) {
         int ret;
@@ -3105,7 +3122,7 @@ static int ram_save_live(Monitor *mon, QEMUFile *f, int stage, void *opaque)
             break;
     }
 
-    bwidth = get_clock() - bwidth;
+    bwidth = qemu_get_clock_ns(rt_clock) - bwidth;
     bwidth = (bytes_transferred - bytes_transferred_last) / bwidth;
 
     /* if we haven't transferred anything this round, force expected_time to a
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 08/19] move kbd/mouse events to event.c
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (6 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 07/19] add qemu_get_clock_ns Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2010-01-04 20:19   ` Anthony Liguori
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 09/19] remove qemu_rearm_alarm_timer from main loop Paolo Bonzini
                   ` (11 subsequent siblings)
  19 siblings, 1 reply; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

As a side exercise, move 200 lines out of vl.c already into common
code that only needs to be compiled once.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile |    2 +-
 event.c  |  238 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 vl.c     |  214 +-------------------------------------------------------
 3 files changed, 241 insertions(+), 213 deletions(-)
 create mode 100644 event.c

diff --git a/Makefile b/Makefile
index 918b7f5..373a861 100644
--- a/Makefile
+++ b/Makefile
@@ -130,7 +130,7 @@ net-obj-y += $(addprefix net/, $(net-nested-y))
 obj-y = $(block-obj-y)
 obj-y += $(net-obj-y)
 obj-y += $(qobject-obj-y)
-obj-y += readline.o console.o
+obj-y += readline.o console.o event.o
 
 obj-y += tcg-runtime.o host-utils.o
 obj-y += irq.o ioport.o
diff --git a/event.c b/event.c
new file mode 100644
index 0000000..955b9ab
--- /dev/null
+++ b/event.c
@@ -0,0 +1,238 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "sysemu.h"
+#include "net.h"
+#include "monitor.h"
+#include "console.h"
+#include "qjson.h"
+
+
+static QEMUPutKBDEvent *qemu_put_kbd_event;
+static void *qemu_put_kbd_event_opaque;
+static QEMUPutMouseEntry *qemu_put_mouse_event_head;
+static QEMUPutMouseEntry *qemu_put_mouse_event_current;
+
+void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
+{
+    qemu_put_kbd_event_opaque = opaque;
+    qemu_put_kbd_event = func;
+}
+
+QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
+                                                void *opaque, int absolute,
+                                                const char *name)
+{
+    QEMUPutMouseEntry *s, *cursor;
+
+    s = qemu_mallocz(sizeof(QEMUPutMouseEntry));
+
+    s->qemu_put_mouse_event = func;
+    s->qemu_put_mouse_event_opaque = opaque;
+    s->qemu_put_mouse_event_absolute = absolute;
+    s->qemu_put_mouse_event_name = qemu_strdup(name);
+    s->next = NULL;
+
+    if (!qemu_put_mouse_event_head) {
+        qemu_put_mouse_event_head = qemu_put_mouse_event_current = s;
+        return s;
+    }
+
+    cursor = qemu_put_mouse_event_head;
+    while (cursor->next != NULL)
+        cursor = cursor->next;
+
+    cursor->next = s;
+    qemu_put_mouse_event_current = s;
+
+    return s;
+}
+
+void qemu_remove_mouse_event_handler(QEMUPutMouseEntry *entry)
+{
+    QEMUPutMouseEntry *prev = NULL, *cursor;
+
+    if (!qemu_put_mouse_event_head || entry == NULL)
+        return;
+
+    cursor = qemu_put_mouse_event_head;
+    while (cursor != NULL && cursor != entry) {
+        prev = cursor;
+        cursor = cursor->next;
+    }
+
+    if (cursor == NULL) // does not exist or list empty
+        return;
+    else if (prev == NULL) { // entry is head
+        qemu_put_mouse_event_head = cursor->next;
+        if (qemu_put_mouse_event_current == entry)
+            qemu_put_mouse_event_current = cursor->next;
+        qemu_free(entry->qemu_put_mouse_event_name);
+        qemu_free(entry);
+        return;
+    }
+
+    prev->next = entry->next;
+
+    if (qemu_put_mouse_event_current == entry)
+        qemu_put_mouse_event_current = prev;
+
+    qemu_free(entry->qemu_put_mouse_event_name);
+    qemu_free(entry);
+}
+
+void kbd_put_keycode(int keycode)
+{
+    if (qemu_put_kbd_event) {
+        qemu_put_kbd_event(qemu_put_kbd_event_opaque, keycode);
+    }
+}
+
+void kbd_mouse_event(int dx, int dy, int dz, int buttons_state)
+{
+    QEMUPutMouseEvent *mouse_event;
+    void *mouse_event_opaque;
+    int width;
+
+    if (!qemu_put_mouse_event_current) {
+        return;
+    }
+
+    mouse_event =
+        qemu_put_mouse_event_current->qemu_put_mouse_event;
+    mouse_event_opaque =
+        qemu_put_mouse_event_current->qemu_put_mouse_event_opaque;
+
+    if (mouse_event) {
+        if (graphic_rotate) {
+            if (qemu_put_mouse_event_current->qemu_put_mouse_event_absolute)
+                width = 0x7fff;
+            else
+                width = graphic_width - 1;
+            mouse_event(mouse_event_opaque,
+                                 width - dy, dx, dz, buttons_state);
+        } else
+            mouse_event(mouse_event_opaque,
+                                 dx, dy, dz, buttons_state);
+    }
+}
+
+int kbd_mouse_is_absolute(void)
+{
+    if (!qemu_put_mouse_event_current)
+        return 0;
+
+    return qemu_put_mouse_event_current->qemu_put_mouse_event_absolute;
+}
+
+static void info_mice_iter(QObject *data, void *opaque)
+{
+    QDict *mouse;
+    Monitor *mon = opaque;
+
+    mouse = qobject_to_qdict(data);
+    monitor_printf(mon, "%c Mouse #%" PRId64 ": %s\n",
+                  (qdict_get_bool(mouse, "current") ? '*' : ' '),
+                  qdict_get_int(mouse, "index"), qdict_get_str(mouse, "name"));
+}
+
+void do_info_mice_print(Monitor *mon, const QObject *data)
+{
+    QList *mice_list;
+
+    mice_list = qobject_to_qlist(data);
+    if (qlist_empty(mice_list)) {
+        monitor_printf(mon, "No mouse devices connected\n");
+        return;
+    }
+
+    qlist_iter(mice_list, info_mice_iter, mon);
+}
+
+/**
+ * do_info_mice(): Show VM mice information
+ *
+ * Each mouse is represented by a QDict, the returned QObject is a QList of
+ * all mice.
+ *
+ * The mouse QDict contains the following:
+ *
+ * - "name": mouse's name
+ * - "index": mouse's index
+ * - "current": true if this mouse is receiving events, false otherwise
+ *
+ * Example:
+ *
+ * [ { "name": "QEMU Microsoft Mouse", "index": 0, "current": false },
+ *   { "name": "QEMU PS/2 Mouse", "index": 1, "current": true } ]
+ */
+void do_info_mice(Monitor *mon, QObject **ret_data)
+{
+    QEMUPutMouseEntry *cursor;
+    QList *mice_list;
+    int index = 0;
+
+    mice_list = qlist_new();
+
+    if (!qemu_put_mouse_event_head) {
+        goto out;
+    }
+
+    cursor = qemu_put_mouse_event_head;
+    while (cursor != NULL) {
+        QObject *obj;
+        obj = qobject_from_jsonf("{ 'name': %s, 'index': %d, 'current': %i }",
+                                 cursor->qemu_put_mouse_event_name,
+                                 index, cursor == qemu_put_mouse_event_current);
+        qlist_append_obj(mice_list, obj);
+        index++;
+        cursor = cursor->next;
+    }
+
+out:
+    *ret_data = QOBJECT(mice_list);
+}
+
+void do_mouse_set(Monitor *mon, const QDict *qdict)
+{
+    QEMUPutMouseEntry *cursor;
+    int i = 0;
+    int index = qdict_get_int(qdict, "index");
+
+    if (!qemu_put_mouse_event_head) {
+        monitor_printf(mon, "No mouse devices connected\n");
+        return;
+    }
+
+    cursor = qemu_put_mouse_event_head;
+    while (cursor != NULL && index != i) {
+        i++;
+        cursor = cursor->next;
+    }
+
+    if (cursor != NULL)
+        qemu_put_mouse_event_current = cursor;
+    else
+        monitor_printf(mon, "Mouse at given index not found\n");
+}
diff --git a/vl.c b/vl.c
index 797c56f..c32dc5d 100644
--- a/vl.c
+++ b/vl.c
@@ -370,216 +370,9 @@ ram_addr_t qemu_balloon_status(void)
     return 0;
 }
 
-/***********************************************************/
-/* keyboard/mouse */
-
-static QEMUPutKBDEvent *qemu_put_kbd_event;
-static void *qemu_put_kbd_event_opaque;
-static QEMUPutMouseEntry *qemu_put_mouse_event_head;
-static QEMUPutMouseEntry *qemu_put_mouse_event_current;
-
-void qemu_add_kbd_event_handler(QEMUPutKBDEvent *func, void *opaque)
-{
-    qemu_put_kbd_event_opaque = opaque;
-    qemu_put_kbd_event = func;
-}
-
-QEMUPutMouseEntry *qemu_add_mouse_event_handler(QEMUPutMouseEvent *func,
-                                                void *opaque, int absolute,
-                                                const char *name)
-{
-    QEMUPutMouseEntry *s, *cursor;
-
-    s = qemu_mallocz(sizeof(QEMUPutMouseEntry));
-
-    s->qemu_put_mouse_event = func;
-    s->qemu_put_mouse_event_opaque = opaque;
-    s->qemu_put_mouse_event_absolute = absolute;
-    s->qemu_put_mouse_event_name = qemu_strdup(name);
-    s->next = NULL;
-
-    if (!qemu_put_mouse_event_head) {
-        qemu_put_mouse_event_head = qemu_put_mouse_event_current = s;
-        return s;
-    }
-
-    cursor = qemu_put_mouse_event_head;
-    while (cursor->next != NULL)
-        cursor = cursor->next;
-
-    cursor->next = s;
-    qemu_put_mouse_event_current = s;
-
-    return s;
-}
-
-void qemu_remove_mouse_event_handler(QEMUPutMouseEntry *entry)
-{
-    QEMUPutMouseEntry *prev = NULL, *cursor;
-
-    if (!qemu_put_mouse_event_head || entry == NULL)
-        return;
-
-    cursor = qemu_put_mouse_event_head;
-    while (cursor != NULL && cursor != entry) {
-        prev = cursor;
-        cursor = cursor->next;
-    }
-
-    if (cursor == NULL) // does not exist or list empty
-        return;
-    else if (prev == NULL) { // entry is head
-        qemu_put_mouse_event_head = cursor->next;
-        if (qemu_put_mouse_event_current == entry)
-            qemu_put_mouse_event_current = cursor->next;
-        qemu_free(entry->qemu_put_mouse_event_name);
-        qemu_free(entry);
-        return;
-    }
-
-    prev->next = entry->next;
-
-    if (qemu_put_mouse_event_current == entry)
-        qemu_put_mouse_event_current = prev;
-
-    qemu_free(entry->qemu_put_mouse_event_name);
-    qemu_free(entry);
-}
-
-void kbd_put_keycode(int keycode)
-{
-    if (qemu_put_kbd_event) {
-        qemu_put_kbd_event(qemu_put_kbd_event_opaque, keycode);
-    }
-}
-
-void kbd_mouse_event(int dx, int dy, int dz, int buttons_state)
-{
-    QEMUPutMouseEvent *mouse_event;
-    void *mouse_event_opaque;
-    int width;
-
-    if (!qemu_put_mouse_event_current) {
-        return;
-    }
-
-    mouse_event =
-        qemu_put_mouse_event_current->qemu_put_mouse_event;
-    mouse_event_opaque =
-        qemu_put_mouse_event_current->qemu_put_mouse_event_opaque;
-
-    if (mouse_event) {
-        if (graphic_rotate) {
-            if (qemu_put_mouse_event_current->qemu_put_mouse_event_absolute)
-                width = 0x7fff;
-            else
-                width = graphic_width - 1;
-            mouse_event(mouse_event_opaque,
-                                 width - dy, dx, dz, buttons_state);
-        } else
-            mouse_event(mouse_event_opaque,
-                                 dx, dy, dz, buttons_state);
-    }
-}
-
-int kbd_mouse_is_absolute(void)
-{
-    if (!qemu_put_mouse_event_current)
-        return 0;
-
-    return qemu_put_mouse_event_current->qemu_put_mouse_event_absolute;
-}
-
-static void info_mice_iter(QObject *data, void *opaque)
-{
-    QDict *mouse;
-    Monitor *mon = opaque;
-
-    mouse = qobject_to_qdict(data);
-    monitor_printf(mon, "%c Mouse #%" PRId64 ": %s\n",
-                  (qdict_get_bool(mouse, "current") ? '*' : ' '),
-                  qdict_get_int(mouse, "index"), qdict_get_str(mouse, "name"));
-}
-
-void do_info_mice_print(Monitor *mon, const QObject *data)
-{
-    QList *mice_list;
-
-    mice_list = qobject_to_qlist(data);
-    if (qlist_empty(mice_list)) {
-        monitor_printf(mon, "No mouse devices connected\n");
-        return;
-    }
-
-    qlist_iter(mice_list, info_mice_iter, mon);
-}
-
-/**
- * do_info_mice(): Show VM mice information
- *
- * Each mouse is represented by a QDict, the returned QObject is a QList of
- * all mice.
- *
- * The mouse QDict contains the following:
- *
- * - "name": mouse's name
- * - "index": mouse's index
- * - "current": true if this mouse is receiving events, false otherwise
- *
- * Example:
- *
- * [ { "name": "QEMU Microsoft Mouse", "index": 0, "current": false },
- *   { "name": "QEMU PS/2 Mouse", "index": 1, "current": true } ]
- */
-void do_info_mice(Monitor *mon, QObject **ret_data)
-{
-    QEMUPutMouseEntry *cursor;
-    QList *mice_list;
-    int index = 0;
-
-    mice_list = qlist_new();
-
-    if (!qemu_put_mouse_event_head) {
-        goto out;
-    }
-
-    cursor = qemu_put_mouse_event_head;
-    while (cursor != NULL) {
-        QObject *obj;
-        obj = qobject_from_jsonf("{ 'name': %s, 'index': %d, 'current': %i }",
-                                 cursor->qemu_put_mouse_event_name,
-                                 index, cursor == qemu_put_mouse_event_current);
-        qlist_append_obj(mice_list, obj);
-        index++;
-        cursor = cursor->next;
-    }
-
-out:
-    *ret_data = QOBJECT(mice_list);
-}
-
-void do_mouse_set(Monitor *mon, const QDict *qdict)
-{
-    QEMUPutMouseEntry *cursor;
-    int i = 0;
-    int index = qdict_get_int(qdict, "index");
-
-    if (!qemu_put_mouse_event_head) {
-        monitor_printf(mon, "No mouse devices connected\n");
-        return;
-    }
-
-    cursor = qemu_put_mouse_event_head;
-    while (cursor != NULL && index != i) {
-        i++;
-        cursor = cursor->next;
-    }
 
-    if (cursor != NULL)
-        qemu_put_mouse_event_current = cursor;
-    else
-        monitor_printf(mon, "Mouse at given index not found\n");
-}
+/***********************************************************/
+/* real time host monotonic timer */
 
 /* compute with 96 bit intermediate result: (a*b)/c */
 uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
@@ -605,9 +398,6 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
     return res.ll;
 }
 
-/***********************************************************/
-/* real time host monotonic timer */
-
 static int64_t get_clock_realtime(void)
 {
     struct timeval tv;
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 09/19] remove qemu_rearm_alarm_timer from main loop
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (7 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 08/19] move kbd/mouse events to event.c Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 10/19] add qemu_bh_scheduled Paolo Bonzini
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

Make the timer subsystem register its own callback instead.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/vl.c b/vl.c
index c32dc5d..78807f5 100644
--- a/vl.c
+++ b/vl.c
@@ -1421,6 +1421,12 @@ static void win32_rearm_timer(struct qemu_alarm_timer *t)
 
 #endif /* _WIN32 */
 
+static void alarm_timer_on_change_state_rearm(void *opaque, int running, int reason)
+{
+    if (running)
+        qemu_rearm_alarm_timer((struct qemu_alarm_timer *) opaque);
+}
+
 static int init_timer_alarm(void)
 {
     struct qemu_alarm_timer *t = NULL;
@@ -1442,6 +1448,7 @@ static int init_timer_alarm(void)
     /* first event is at time 0 */
     t->pending = 1;
     alarm_timer = t;
+    qemu_add_vm_change_state_handler(alarm_timer_on_change_state_rearm, t);
 
     return 0;
 
@@ -3094,7 +3101,6 @@ void vm_start(void)
         cpu_enable_ticks();
         vm_running = 1;
         vm_state_notify(1, 0);
-        qemu_rearm_alarm_timer(alarm_timer);
         resume_all_vcpus();
     }
 }
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 10/19] add qemu_bh_scheduled
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (8 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 09/19] remove qemu_rearm_alarm_timer from main loop Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 11/19] use a bottom half to run timers Paolo Bonzini
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 async.c       |    5 +++++
 qemu-common.h |    2 ++
 2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/async.c b/async.c
index 57ac3a8..d58a1d5 100644
--- a/async.c
+++ b/async.c
@@ -188,6 +188,11 @@ void qemu_bh_cancel(QEMUBH *bh)
     bh->scheduled = 0;
 }
 
+bool qemu_bh_scheduled(QEMUBH *bh)
+{
+    return bh->scheduled && !bh->deleted;
+}
+
 void qemu_bh_delete(QEMUBH *bh)
 {
     bh->scheduled = 0;
diff --git a/qemu-common.h b/qemu-common.h
index 8630f8c..fac6a18 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -16,6 +16,7 @@
 
 /* we put basic includes here to avoid repeating them in device drivers */
 #include <stdlib.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdarg.h>
 #include <string.h>
@@ -106,6 +107,7 @@ void qemu_bh_schedule(QEMUBH *bh);
  * iteration.
  */
 void qemu_bh_schedule_idle(QEMUBH *bh);
+bool qemu_bh_scheduled(QEMUBH *bh);
 void qemu_bh_cancel(QEMUBH *bh);
 void qemu_bh_delete(QEMUBH *bh);
 int qemu_bh_poll(void);
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 11/19] use a bottom half to run timers
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (9 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 10/19] add qemu_bh_scheduled Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2010-01-04 20:24   ` Anthony Liguori
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 12/19] new function qemu_icount_delta Paolo Bonzini
                   ` (8 subsequent siblings)
  19 siblings, 1 reply; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

Make the timer subsystem register its own bottom half instead of
placing the bottom half code in the heart of the main loop.  To
test if an alarm timer is pending, just check if the bottom half is
scheduled.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |   68 ++++++++++++++++++++++++++++++++++++-----------------------------
 1 files changed, 38 insertions(+), 30 deletions(-)

diff --git a/vl.c b/vl.c
index 78807f5..289aadc 100644
--- a/vl.c
+++ b/vl.c
@@ -573,10 +573,17 @@ struct qemu_alarm_timer {
     void (*rearm)(struct qemu_alarm_timer *t);
     void *priv;
 
+    QEMUBH *bh;
     char expired;
-    char pending;
 };
 
+static struct qemu_alarm_timer *alarm_timer;
+
+static inline int qemu_alarm_pending(void)
+{
+    return qemu_bh_scheduled(alarm_timer->bh);
+}
+
 static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
 {
     return !!t->rearm;
@@ -593,8 +600,6 @@ static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
 /* TODO: MIN_TIMER_REARM_US should be optimized */
 #define MIN_TIMER_REARM_US 250
 
-static struct qemu_alarm_timer *alarm_timer;
-
 #ifdef _WIN32
 
 struct qemu_alarm_win32 {
@@ -874,7 +879,7 @@ void qemu_mod_timer(QEMUTimer *ts, int64_t expire_time)
 
     /* Rearm if necessary  */
     if (pt == &active_timers[ts->clock->type]) {
-        if (!alarm_timer->pending) {
+        if (!qemu_alarm_pending()) {
             qemu_rearm_alarm_timer(alarm_timer);
         }
         /* Interrupt execution to force deadline recalculation.  */
@@ -1001,6 +1006,31 @@ static const VMStateDescription vmstate_timers = {
 
 static void qemu_event_increment(void);
 
+static void qemu_timer_bh(void *opaque)
+{
+    struct qemu_alarm_timer *t = opaque;
+
+    /* rearm timer, if not periodic */
+    if (t->expired) {
+        t->expired = 0;
+        qemu_rearm_alarm_timer(t);
+    }
+
+    /* vm time timers */
+    if (vm_running) {
+        if (!cur_cpu || likely(!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER)))
+            qemu_run_timers(&active_timers[QEMU_CLOCK_VIRTUAL],
+                            qemu_get_clock(vm_clock));
+    }
+
+    /* real time timers */
+    qemu_run_timers(&active_timers[QEMU_CLOCK_REALTIME],
+                    qemu_get_clock(rt_clock));
+
+    qemu_run_timers(&active_timers[QEMU_CLOCK_HOST],
+                    qemu_get_clock(host_clock));
+}
+
 #ifdef _WIN32
 static void CALLBACK host_alarm_handler(UINT uTimerID, UINT uMsg,
                                         DWORD_PTR dwUser, DWORD_PTR dw1,
@@ -1059,8 +1089,7 @@ static void host_alarm_handler(int host_signum)
             cpu_exit(next_cpu);
         }
 #endif
-        t->pending = 1;
-        qemu_notify_event();
+        qemu_bh_schedule(t->bh);
     }
 }
 
@@ -1446,7 +1475,8 @@ static int init_timer_alarm(void)
     }
 
     /* first event is at time 0 */
-    t->pending = 1;
+    t->bh = qemu_bh_new(qemu_timer_bh, t);
+    qemu_bh_schedule(t->bh);
     alarm_timer = t;
     qemu_add_vm_change_state_handler(alarm_timer_on_change_state_rearm, t);
 
@@ -3811,28 +3841,6 @@ void main_loop_wait(int timeout)
 
     slirp_select_poll(&rfds, &wfds, &xfds, (ret < 0));
 
-    /* rearm timer, if not periodic */
-    if (alarm_timer->expired) {
-        alarm_timer->expired = 0;
-        qemu_rearm_alarm_timer(alarm_timer);
-    }
-
-    alarm_timer->pending = 0;
-
-    /* vm time timers */
-    if (vm_running) {
-        if (!cur_cpu || likely(!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER)))
-            qemu_run_timers(&active_timers[QEMU_CLOCK_VIRTUAL],
-                            qemu_get_clock(vm_clock));
-    }
-
-    /* real time timers */
-    qemu_run_timers(&active_timers[QEMU_CLOCK_REALTIME],
-                    qemu_get_clock(rt_clock));
-
-    qemu_run_timers(&active_timers[QEMU_CLOCK_HOST],
-                    qemu_get_clock(host_clock));
-
     /* Check bottom-halves last in case any of the earlier events triggered
        them.  */
     qemu_bh_poll();
@@ -3888,7 +3896,7 @@ static void tcg_cpu_exec(void)
 
         if (!vm_running)
             break;
-        if (alarm_timer->pending)
+        if (qemu_alarm_pending())
             break;
         if (cpu_can_run(env))
             ret = qemu_cpu_exec(env);
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 12/19] new function qemu_icount_delta
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (10 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 11/19] use a bottom half to run timers Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 13/19] move tcg_has_work to cpu-exec.c and rename it Paolo Bonzini
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

Tweaking the rounding in qemu_next_deadline ensures that there's
no change whatsoever.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/vl.c b/vl.c
index 289aadc..9f363c8 100644
--- a/vl.c
+++ b/vl.c
@@ -525,6 +525,20 @@ static int64_t cpu_get_clock(void)
     }
 }
 
+static int64_t qemu_icount_delta(void)
+{
+    if (!use_icount) {
+        return 5000 * (int64_t) 1000000;
+    } else if (use_icount == 1) {
+        /* When not using an adaptive execution frequency
+           we tend to get badly out of sync with real time,
+           so just delay for a reasonable amount of time.  */
+        return 0;
+    } else {
+        return cpu_get_icount() - cpu_get_clock();
+    }
+}
+
 /* enable cpu_get_ticks() */
 void cpu_enable_ticks(void)
 {
@@ -3940,25 +3954,16 @@ static int qemu_calculate_timeout(void)
         timeout = 5000;
     else if (tcg_has_work())
         timeout = 0;
-    else if (!use_icount)
-        timeout = 5000;
     else {
      /* XXX: use timeout computed from timers */
         int64_t add;
         int64_t delta;
         /* Advance virtual time to the next event.  */
-        if (use_icount == 1) {
-            /* When not using an adaptive execution frequency
-               we tend to get badly out of sync with real time,
-               so just delay for a reasonable amount of time.  */
-            delta = 0;
-        } else {
-            delta = cpu_get_icount() - cpu_get_clock();
-        }
+	delta = qemu_icount_delta();
         if (delta > 0) {
             /* If virtual time is ahead of real time then just
                wait for IO.  */
-            timeout = (delta / 1000000) + 1;
+            timeout = (delta + 999999) / 1000000;
         } else {
             /* Wait for either IO to occur or the next
                timer event.  */
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 13/19] move tcg_has_work to cpu-exec.c and rename it
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (11 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 12/19] new function qemu_icount_delta Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 14/19] disentangle tcg and deadline calculation Paolo Bonzini
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

tcg_has_work is the only user of cpu-exec.c's qemu_cpu_has_work export.
Just move everything into cpu-exec.c.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 cpu-all.h  |    2 +-
 cpu-exec.c |   16 ++++++++++++++--
 vl.c       |   28 ++--------------------------
 3 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/cpu-all.h b/cpu-all.h
index e214374..aef594d 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -781,7 +781,7 @@ void cpu_reset_interrupt(CPUState *env, int mask);
 
 void cpu_exit(CPUState *s);
 
-int qemu_cpu_has_work(CPUState *env);
+int qemu_cpus_have_work(void);
 
 /* Breakpoint/watchpoint flags */
 #define BP_MEM_READ           0x01
diff --git a/cpu-exec.c b/cpu-exec.c
index af4595b..b458484 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -49,9 +49,21 @@ int tb_invalidated_flag;
 //#define CONFIG_DEBUG_EXEC
 //#define DEBUG_SIGNAL
 
-int qemu_cpu_has_work(CPUState *env)
+int qemu_cpus_have_work(void)
 {
-    return cpu_has_work(env);
+    CPUState *env;
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        if (env->stop)
+            return 1;
+        if (env->stopped)
+            return 0;
+        if (!env->halted)
+            return 1;
+        if (cpu_has_work(env))
+            return 1;
+    }
+    return 0;
 }
 
 void cpu_loop_exit(void)
diff --git a/vl.c b/vl.c
index 9f363c8..9bf9806 100644
--- a/vl.c
+++ b/vl.c
@@ -3435,7 +3435,6 @@ static QemuCond qemu_pause_cond;
 
 static void block_io_signals(void);
 static void unblock_io_signals(void);
-static int tcg_has_work(void);
 
 static int qemu_init_main_loop(void)
 {
@@ -3458,7 +3457,7 @@ static int qemu_init_main_loop(void)
 
 static void qemu_wait_io_event(CPUState *env)
 {
-    while (!tcg_has_work())
+    while (!qemu_cpus_have_work())
         qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
 
     qemu_mutex_unlock(&qemu_global_mutex);
@@ -3922,29 +3921,6 @@ static void tcg_cpu_exec(void)
     }
 }
 
-static int cpu_has_work(CPUState *env)
-{
-    if (env->stop)
-        return 1;
-    if (env->stopped)
-        return 0;
-    if (!env->halted)
-        return 1;
-    if (qemu_cpu_has_work(env))
-        return 1;
-    return 0;
-}
-
-static int tcg_has_work(void)
-{
-    CPUState *env;
-
-    for (env = first_cpu; env != NULL; env = env->next_cpu)
-        if (cpu_has_work(env))
-            return 1;
-    return 0;
-}
-
 static int qemu_calculate_timeout(void)
 {
 #ifndef CONFIG_IOTHREAD
@@ -3952,7 +3928,7 @@ static int qemu_calculate_timeout(void)
 
     if (!vm_running)
         timeout = 5000;
-    else if (tcg_has_work())
+    else if (qemu_cpus_have_work())
         timeout = 0;
     else {
      /* XXX: use timeout computed from timers */
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 14/19] disentangle tcg and deadline calculation
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (12 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 13/19] move tcg_has_work to cpu-exec.c and rename it Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 15/19] do not provide qemu_event_increment if iothread not used Paolo Bonzini
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

Just tell main_loop_wait whether to be blocking or nonblocking, so that
there is no need to call qemu_cpus_have_work from the timer subsystem.
Instead, tcg_cpu_exec can say "we want the main loop not to block because
we have stuff to do".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/xenfb.c |    6 ++++--
 sysemu.h   |    2 +-
 vl.c       |   25 ++++++++++++++++---------
 3 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/hw/xenfb.c b/hw/xenfb.c
index 795a326..422cd53 100644
--- a/hw/xenfb.c
+++ b/hw/xenfb.c
@@ -983,12 +983,14 @@ void xen_init_display(int domid)
 
 wait_more:
     i++;
-    main_loop_wait(10); /* miliseconds */
+    main_loop_wait(true);
     xfb = xen_be_find_xendev("vfb", domid, 0);
     xin = xen_be_find_xendev("vkbd", domid, 0);
     if (!xfb || !xin) {
-        if (i < 256)
+        if (i < 256) {
+            usleep(10000);
             goto wait_more;
+        }
         xen_be_printf(NULL, 1, "displaystate setup failed\n");
         return;
     }
diff --git a/sysemu.h b/sysemu.h
index 9d80bb2..1384ef9 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -60,7 +60,7 @@ void do_info_snapshots(Monitor *mon);
 
 void qemu_announce_self(void);
 
-void main_loop_wait(int timeout);
+void main_loop_wait(bool nonblocking);
 
 int qemu_savevm_state_begin(Monitor *mon, QEMUFile *f, int blk_enable,
                             int shared);
diff --git a/vl.c b/vl.c
index 9bf9806..f7472db 100644
--- a/vl.c
+++ b/vl.c
@@ -592,6 +592,7 @@ struct qemu_alarm_timer {
 };
 
 static struct qemu_alarm_timer *alarm_timer;
+static int qemu_calculate_timeout(void);
 
 static inline int qemu_alarm_pending(void)
 {
@@ -3507,7 +3508,7 @@ static void *kvm_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void tcg_cpu_exec(void);
+static bool tcg_cpu_exec(void);
 
 static void *tcg_cpu_thread_fn(void *arg)
 {
@@ -3786,14 +3787,20 @@ static void host_main_loop_wait(int *timeout)
 }
 #endif
 
-void main_loop_wait(int timeout)
+void main_loop_wait(bool nonblocking)
 {
     IOHandlerRecord *ioh;
     fd_set rfds, wfds, xfds;
     int ret, nfds;
     struct timeval tv;
+    int timeout;
 
-    qemu_bh_update_timeout(&timeout);
+    if (nonblocking)
+        timeout = 0;
+    else {
+        timeout = qemu_calculate_timeout();
+        qemu_bh_update_timeout(&timeout);
+    }
 
     host_main_loop_wait(&timeout);
 
@@ -3898,7 +3905,7 @@ static int qemu_cpu_exec(CPUState *env)
     return ret;
 }
 
-static void tcg_cpu_exec(void)
+static bool tcg_cpu_exec(void)
 {
     int ret = 0;
 
@@ -3908,7 +3915,7 @@ static void tcg_cpu_exec(void)
         CPUState *env = cur_cpu = next_cpu;
 
         if (!vm_running)
-            break;
+            return false;
         if (qemu_alarm_pending())
             break;
         if (cpu_can_run(env))
@@ -3919,6 +3926,7 @@ static void tcg_cpu_exec(void)
             break;
         }
     }
+    return qemu_cpus_have_work();
 }
 
 static int qemu_calculate_timeout(void)
@@ -3928,8 +3936,6 @@ static int qemu_calculate_timeout(void)
 
     if (!vm_running)
         timeout = 5000;
-    else if (qemu_cpus_have_work())
-        timeout = 0;
     else {
      /* XXX: use timeout computed from timers */
         int64_t add;
@@ -3989,16 +3995,17 @@ static void main_loop(void)
 
     for (;;) {
         do {
+            bool nonblocking = false;
 #ifdef CONFIG_PROFILER
             int64_t ti;
 #endif
 #ifndef CONFIG_IOTHREAD
-            tcg_cpu_exec();
+            nonblocking = tcg_cpu_exec();
 #endif
 #ifdef CONFIG_PROFILER
             ti = profile_getclock();
 #endif
-            main_loop_wait(qemu_calculate_timeout());
+            main_loop_wait(nonblocking);
 #ifdef CONFIG_PROFILER
             dev_time += profile_getclock() - ti;
 #endif
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 15/19] do not provide qemu_event_increment if iothread not used
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (13 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 14/19] disentangle tcg and deadline calculation Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 16/19] tweak qemu_notify_event Paolo Bonzini
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

host_alarm_handler is using qemu_event_increment instead of qemu_notify_event.
After fixing this, the whole event business can be omitted if not using the
iothread.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |  130 ++++++++++++++++++++++++++++++++----------------------------------
 1 files changed, 63 insertions(+), 67 deletions(-)

diff --git a/vl.c b/vl.c
index f7472db..11b1b70 100644
--- a/vl.c
+++ b/vl.c
@@ -1019,8 +1019,6 @@ static const VMStateDescription vmstate_timers = {
     }
 };
 
-static void qemu_event_increment(void);
-
 static void qemu_timer_bh(void *opaque)
 {
     struct qemu_alarm_timer *t = opaque;
@@ -1095,7 +1093,7 @@ static void host_alarm_handler(int host_signum)
                            qemu_get_clock(rt_clock)) ||
         qemu_timer_expired(active_timers[QEMU_CLOCK_HOST],
                            qemu_get_clock(host_clock))) {
-        qemu_event_increment();
+        qemu_notify_event();
         t->expired = alarm_has_dynticks(t);
 
 #ifndef CONFIG_IOTHREAD
@@ -3265,13 +3263,21 @@ void qemu_system_powerdown_request(void)
     qemu_notify_event();
 }
 
+static int cpu_can_run(CPUState *env)
+{
+    if (env->stop)
+        return 0;
+    if (env->stopped)
+        return 0;
+    return 1;
+}
+
 #ifdef CONFIG_IOTHREAD
 static void qemu_system_vmstop_request(int reason)
 {
     vmstop_requested = reason;
     qemu_notify_event();
 }
-#endif
 
 #ifndef _WIN32
 static int io_thread_fd = -1;
@@ -3354,69 +3360,6 @@ static void qemu_event_increment(void)
 }
 #endif
 
-static int cpu_can_run(CPUState *env)
-{
-    if (env->stop)
-        return 0;
-    if (env->stopped)
-        return 0;
-    return 1;
-}
-
-#ifndef CONFIG_IOTHREAD
-static int qemu_init_main_loop(void)
-{
-    return qemu_event_init();
-}
-
-void qemu_init_vcpu(void *_env)
-{
-    CPUState *env = _env;
-
-    if (kvm_enabled())
-        kvm_init_vcpu(env);
-    env->nr_cores = smp_cores;
-    env->nr_threads = smp_threads;
-    return;
-}
-
-int qemu_cpu_self(void *env)
-{
-    return 1;
-}
-
-static void resume_all_vcpus(void)
-{
-}
-
-static void pause_all_vcpus(void)
-{
-}
-
-void qemu_cpu_kick(void *env)
-{
-    return;
-}
-
-void qemu_notify_event(void)
-{
-    CPUState *env = cpu_single_env;
-
-    if (env) {
-        cpu_exit(env);
-    }
-}
-
-void qemu_mutex_lock_iothread(void) {}
-void qemu_mutex_unlock_iothread(void) {}
-
-void vm_stop(int reason)
-{
-    do_vm_stop(reason);
-}
-
-#else /* CONFIG_IOTHREAD */
-
 #include "qemu-thread.h"
 
 QemuMutex qemu_global_mutex;
@@ -3734,6 +3677,59 @@ void vm_stop(int reason)
     do_vm_stop(reason);
 }
 
+#else /* CONFIG_IOTHREAD */
+
+static int qemu_init_main_loop(void)
+{
+    return 0;
+}
+
+void qemu_init_vcpu(void *_env)
+{
+    CPUState *env = _env;
+
+    if (kvm_enabled())
+        kvm_init_vcpu(env);
+    env->nr_cores = smp_cores;
+    env->nr_threads = smp_threads;
+    return;
+}
+
+int qemu_cpu_self(void *env)
+{
+    return 1;
+}
+
+static void resume_all_vcpus(void)
+{
+}
+
+static void pause_all_vcpus(void)
+{
+}
+
+void qemu_cpu_kick(void *env)
+{
+    return;
+}
+
+void qemu_notify_event(void)
+{
+    CPUState *env = cpu_single_env;
+
+    if (env) {
+        cpu_exit(env);
+    }
+}
+
+void qemu_mutex_lock_iothread(void) {}
+void qemu_mutex_unlock_iothread(void) {}
+
+void vm_stop(int reason)
+{
+    do_vm_stop(reason);
+}
+
 #endif
 
 
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 16/19] tweak qemu_notify_event
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (14 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 15/19] do not provide qemu_event_increment if iothread not used Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 17/19] move vmstate registration of vmstate_timers earlier Paolo Bonzini
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

Instead of testing specially next_cpu in host_alarm_handler, just do
that in qemu_notify_event.  The idea is, if we are not running (or
not yet running) target CPU code, prepare things so that the execution
loop is exited asap; just make that clear.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/vl.c b/vl.c
index 11b1b70..23ba687 100644
--- a/vl.c
+++ b/vl.c
@@ -1095,13 +1095,6 @@ static void host_alarm_handler(int host_signum)
                            qemu_get_clock(host_clock))) {
         qemu_notify_event();
         t->expired = alarm_has_dynticks(t);
-
-#ifndef CONFIG_IOTHREAD
-        if (next_cpu) {
-            /* stop the currently executing cpu because a timer occured */
-            cpu_exit(next_cpu);
-        }
-#endif
         qemu_bh_schedule(t->bh);
     }
 }
@@ -3719,6 +3712,9 @@ void qemu_notify_event(void)
 
     if (env) {
         cpu_exit(env);
+    } else if (next_cpu) {
+        /* stop the currently executing cpu because a timer occured */
+        cpu_exit(next_cpu);
     }
 }
 
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 17/19] move vmstate registration of vmstate_timers earlier
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (15 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 16/19] tweak qemu_notify_event Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 18/19] introduce qemu_clock_enable Paolo Bonzini
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |   62 +++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 31 insertions(+), 31 deletions(-)

diff --git a/vl.c b/vl.c
index 23ba687..204b6a0 100644
--- a/vl.c
+++ b/vl.c
@@ -697,36 +697,6 @@ static void icount_adjust_vm(void * opaque)
     icount_adjust();
 }
 
-static void configure_icount(const char *option)
-{
-    if (!option)
-        return;
-
-    if (strcmp(option, "auto") != 0) {
-        icount_time_shift = strtol(option, NULL, 0);
-        use_icount = 1;
-        return;
-    }
-
-    use_icount = 2;
-
-    /* 125MIPS seems a reasonable initial guess at the guest speed.
-       It will be corrected fairly quickly anyway.  */
-    icount_time_shift = 3;
-
-    /* Have both realtime and virtual time triggers for speed adjustment.
-       The realtime trigger catches emulated time passing too slowly,
-       the virtual time trigger catches emulated time passing too fast.
-       Realtime triggers occur even when idle, so use them less frequently
-       than VM triggers.  */
-    icount_rt_timer = qemu_new_timer(rt_clock, icount_adjust_rt, NULL);
-    qemu_mod_timer(icount_rt_timer,
-                   qemu_get_clock(rt_clock) + 1000);
-    icount_vm_timer = qemu_new_timer(vm_clock, icount_adjust_vm, NULL);
-    qemu_mod_timer(icount_vm_timer,
-                   qemu_get_clock(vm_clock) + get_ticks_per_sec() / 10);
-}
-
 static int64_t qemu_icount_round(int64_t count)
 {
     return (count + (1 << icount_time_shift) - 1) >> icount_time_shift;
@@ -1019,6 +989,37 @@ static const VMStateDescription vmstate_timers = {
     }
 };
 
+static void configure_icount(const char *option)
+{
+    vmstate_register(0, &vmstate_timers, &timers_state);
+    if (!option)
+        return;
+
+    if (strcmp(option, "auto") != 0) {
+        icount_time_shift = strtol(option, NULL, 0);
+        use_icount = 1;
+        return;
+    }
+
+    use_icount = 2;
+
+    /* 125MIPS seems a reasonable initial guess at the guest speed.
+       It will be corrected fairly quickly anyway.  */
+    icount_time_shift = 3;
+
+    /* Have both realtime and virtual time triggers for speed adjustment.
+       The realtime trigger catches emulated time passing too slowly,
+       the virtual time trigger catches emulated time passing too fast.
+       Realtime triggers occur even when idle, so use them less frequently
+       than VM triggers.  */
+    icount_rt_timer = qemu_new_timer(rt_clock, icount_adjust_rt, NULL);
+    qemu_mod_timer(icount_rt_timer,
+                   qemu_get_clock(rt_clock) + 1000);
+    icount_vm_timer = qemu_new_timer(vm_clock, icount_adjust_vm, NULL);
+    qemu_mod_timer(icount_vm_timer,
+                   qemu_get_clock(vm_clock) + get_ticks_per_sec() / 10);
+}
+
 static void qemu_timer_bh(void *opaque)
 {
     struct qemu_alarm_timer *t = opaque;
@@ -5673,7 +5674,6 @@ int main(int argc, char **argv, char **envp)
     if (qemu_opts_foreach(&qemu_drive_opts, drive_init_func, machine, 1) != 0)
         exit(1);
 
-    vmstate_register(0, &vmstate_timers ,&timers_state);
     register_savevm_live("ram", 0, 3, NULL, ram_save_live, NULL, 
                          ram_load, NULL);
 
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 18/19] introduce qemu_clock_enable
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (16 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 17/19] move vmstate registration of vmstate_timers earlier Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 19/19] split out qemu-timer.c Paolo Bonzini
  2010-01-04 19:26 ` [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Anthony Liguori
  19 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

By adding the possibility to turn on/off a clock, yet another
incestuous relationship between timers and CPUs can be disentangled.

This also needs qemu_run_timers to accept a QEMUClock, and nicely
simplifies host_alarm_handler.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c |   32 +++++++++++++++++++++-----------
 1 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/vl.c b/vl.c
index 204b6a0..17ad374 100644
--- a/vl.c
+++ b/vl.c
@@ -569,6 +569,7 @@ void cpu_disable_ticks(void)
 
 struct QEMUClock {
     int type;
+    int enabled;
     /* XXX: add frequency */
 };
 
@@ -799,9 +800,15 @@ static QEMUClock *qemu_new_clock(int type)
     QEMUClock *clock;
     clock = qemu_mallocz(sizeof(QEMUClock));
     clock->type = type;
+    clock->enabled = 1;
     return clock;
 }
 
+static void qemu_clock_enable(QEMUClock *clock, int enabled)
+{
+    clock->enabled = enabled;
+}
+
 QEMUTimer *qemu_new_timer(QEMUClock *clock, QEMUTimerCB *cb, void *opaque)
 {
     QEMUTimer *ts;
@@ -890,10 +897,16 @@ int qemu_timer_expired(QEMUTimer *timer_head, int64_t current_time)
     return (timer_head->expire_time <= current_time);
 }
 
-static void qemu_run_timers(QEMUTimer **ptimer_head, int64_t current_time)
+static void qemu_run_timers(QEMUClock *clock)
 {
-    QEMUTimer *ts;
+    QEMUTimer **ptimer_head, *ts;
+    int64_t current_time;
+   
+    if (!clock->enabled)
+        return;
 
+    current_time = qemu_get_clock (clock);
+    ptimer_head = &active_timers[clock->type];
     for(;;) {
         ts = *ptimer_head;
         if (!ts || ts->expire_time > current_time)
@@ -1032,17 +1045,11 @@ static void qemu_timer_bh(void *opaque)
 
     /* vm time timers */
     if (vm_running) {
-        if (!cur_cpu || likely(!(cur_cpu->singlestep_enabled & SSTEP_NOTIMER)))
-            qemu_run_timers(&active_timers[QEMU_CLOCK_VIRTUAL],
-                            qemu_get_clock(vm_clock));
+        qemu_run_timers(vm_clock);
     }
 
-    /* real time timers */
-    qemu_run_timers(&active_timers[QEMU_CLOCK_REALTIME],
-                    qemu_get_clock(rt_clock));
-
-    qemu_run_timers(&active_timers[QEMU_CLOCK_HOST],
-                    qemu_get_clock(host_clock));
+    qemu_run_timers(rt_clock);
+    qemu_run_timers(host_clock);
 }
 
 #ifdef _WIN32
@@ -3907,6 +3914,9 @@ static bool tcg_cpu_exec(void)
     for (; next_cpu != NULL; next_cpu = next_cpu->next_cpu) {
         CPUState *env = cur_cpu = next_cpu;
 
+        qemu_clock_enable(vm_clock, 
+                          (cur_cpu->singlestep_enabled & SSTEP_NOTIMER) != 0);
+
         if (!vm_running)
             return false;
         if (qemu_alarm_pending())
-- 
1.6.5.2

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

* [Qemu-devel] [PATCH 19/19] split out qemu-timer.c
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (17 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 18/19] introduce qemu_clock_enable Paolo Bonzini
@ 2009-12-21  8:09 ` Paolo Bonzini
  2010-01-04 20:26   ` Anthony Liguori
  2010-01-04 19:26 ` [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Anthony Liguori
  19 siblings, 1 reply; 41+ messages in thread
From: Paolo Bonzini @ 2009-12-21  8:09 UTC (permalink / raw)
  To: qemu-devel

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.target |    1 +
 cpu-all.h       |    2 +
 qemu-timer.c    | 1218 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qemu-timer.h    |   11 +
 vl.c            | 1164 ----------------------------------------------------
 5 files changed, 1232 insertions(+), 1164 deletions(-)
 create mode 100644 qemu-timer.c

diff --git a/Makefile.target b/Makefile.target
index 7c1f30c..0504d3b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -154,6 +154,7 @@ endif #CONFIG_BSD_USER
 ifdef CONFIG_SOFTMMU
 
 obj-y = vl.o async.o monitor.o pci.o pci_host.o pcie_host.o machine.o gdbstub.o
+obj-y += qemu-timer.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
 obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-console.o virtio-pci.o
diff --git a/cpu-all.h b/cpu-all.h
index aef594d..c8d3f43 100644
--- a/cpu-all.h
+++ b/cpu-all.h
@@ -760,6 +760,8 @@ void QEMU_NORETURN cpu_abort(CPUState *env, const char *fmt, ...)
     __attribute__ ((__format__ (__printf__, 2, 3)));
 extern CPUState *first_cpu;
 extern CPUState *cpu_single_env;
+
+int64_t qemu_icount_round(int64_t count);
 extern int64_t qemu_icount;
 extern int use_icount;
 
diff --git a/qemu-timer.c b/qemu-timer.c
new file mode 100644
index 0000000..1553702
--- /dev/null
+++ b/qemu-timer.c
@@ -0,0 +1,1218 @@
+/*
+ * QEMU System Emulator
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "sysemu.h"
+#include "net.h"
+#include "monitor.h"
+#include "console.h"
+
+#include "hw/hw.h"
+
+#include <unistd.h>
+#include <fcntl.h>
+#include <time.h>
+#include <errno.h>
+#include <sys/time.h>
+#include <signal.h>
+
+#ifdef __linux__
+#include <sys/ioctl.h>
+#include <linux/rtc.h>
+/* For the benefit of older linux systems which don't supply it,
+   we use a local copy of hpet.h. */
+/* #include <linux/hpet.h> */
+#include "hpet.h"
+#endif
+
+#ifdef _WIN32
+#include <windows.h>
+#include <mmsystem.h>
+#endif
+
+#include "cpu-defs.h"
+#include "qemu-timer.h"
+#include "exec-all.h"
+
+/* Conversion factor from emulated instructions to virtual clock ticks.  */
+static int icount_time_shift;
+/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
+#define MAX_ICOUNT_SHIFT 10
+/* Compensate for varying guest execution speed.  */
+static int64_t qemu_icount_bias;
+static QEMUTimer *icount_rt_timer;
+static QEMUTimer *icount_vm_timer;
+
+
+/***********************************************************/
+/* real time host monotonic timer */
+
+
+static int64_t get_clock_realtime(void)
+{
+    struct timeval tv;
+
+    gettimeofday(&tv, NULL);
+    return tv.tv_sec * 1000000000LL + (tv.tv_usec * 1000);
+}
+
+#ifdef WIN32
+
+static int64_t clock_freq;
+
+static void init_get_clock(void)
+{
+    LARGE_INTEGER freq;
+    int ret;
+    ret = QueryPerformanceFrequency(&freq);
+    if (ret == 0) {
+        fprintf(stderr, "Could not calibrate ticks\n");
+        exit(1);
+    }
+    clock_freq = freq.QuadPart;
+}
+
+static int64_t get_clock(void)
+{
+    LARGE_INTEGER ti;
+    QueryPerformanceCounter(&ti);
+    return muldiv64(ti.QuadPart, get_ticks_per_sec(), clock_freq);
+}
+
+#else
+
+static int use_rt_clock;
+
+static void init_get_clock(void)
+{
+    use_rt_clock = 0;
+#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
+    || defined(__DragonFly__) || defined(__FreeBSD_kernel__)
+    {
+        struct timespec ts;
+        if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
+            use_rt_clock = 1;
+        }
+    }
+#endif
+}
+
+static int64_t get_clock(void)
+{
+#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
+	|| defined(__DragonFly__) || defined(__FreeBSD_kernel__)
+    if (use_rt_clock) {
+        struct timespec ts;
+        clock_gettime(CLOCK_MONOTONIC, &ts);
+        return ts.tv_sec * 1000000000LL + ts.tv_nsec;
+    } else
+#endif
+    {
+        /* XXX: using gettimeofday leads to problems if the date
+           changes, so it should be avoided. */
+        return get_clock_realtime();
+    }
+}
+#endif
+
+/* Return the virtual CPU time, based on the instruction counter.  */
+static int64_t cpu_get_icount(void)
+{
+    int64_t icount;
+    CPUState *env = cpu_single_env;;
+    icount = qemu_icount;
+    if (env) {
+        if (!can_do_io(env))
+            fprintf(stderr, "Bad clock read\n");
+        icount -= (env->icount_decr.u16.low + env->icount_extra);
+    }
+    return qemu_icount_bias + (icount << icount_time_shift);
+}
+
+/***********************************************************/
+/* guest cycle counter */
+
+typedef struct TimersState {
+    int64_t cpu_ticks_prev;
+    int64_t cpu_ticks_offset;
+    int64_t cpu_clock_offset;
+    int32_t cpu_ticks_enabled;
+    int64_t dummy;
+} TimersState;
+
+TimersState timers_state;
+
+/* return the host CPU cycle counter and handle stop/restart */
+int64_t cpu_get_ticks(void)
+{
+    if (use_icount) {
+        return cpu_get_icount();
+    }
+    if (!timers_state.cpu_ticks_enabled) {
+        return timers_state.cpu_ticks_offset;
+    } else {
+        int64_t ticks;
+        ticks = cpu_get_real_ticks();
+        if (timers_state.cpu_ticks_prev > ticks) {
+            /* Note: non increasing ticks may happen if the host uses
+               software suspend */
+            timers_state.cpu_ticks_offset += timers_state.cpu_ticks_prev - ticks;
+        }
+        timers_state.cpu_ticks_prev = ticks;
+        return ticks + timers_state.cpu_ticks_offset;
+    }
+}
+
+/* return the host CPU monotonic timer and handle stop/restart */
+static int64_t cpu_get_clock(void)
+{
+    int64_t ti;
+    if (!timers_state.cpu_ticks_enabled) {
+        return timers_state.cpu_clock_offset;
+    } else {
+        ti = get_clock();
+        return ti + timers_state.cpu_clock_offset;
+    }
+}
+
+static int64_t qemu_icount_delta(void)
+{
+    if (!use_icount) {
+        return 5000 * (int64_t) 1000000;
+    } else if (use_icount == 1) {
+        /* When not using an adaptive execution frequency
+           we tend to get badly out of sync with real time,
+           so just delay for a reasonable amount of time.  */
+        return 0;
+    } else {
+        return cpu_get_icount() - cpu_get_clock();
+    }
+}
+
+/* enable cpu_get_ticks() */
+void cpu_enable_ticks(void)
+{
+    if (!timers_state.cpu_ticks_enabled) {
+        timers_state.cpu_ticks_offset -= cpu_get_real_ticks();
+        timers_state.cpu_clock_offset -= get_clock();
+        timers_state.cpu_ticks_enabled = 1;
+    }
+}
+
+/* disable cpu_get_ticks() : the clock is stopped. You must not call
+   cpu_get_ticks() after that.  */
+void cpu_disable_ticks(void)
+{
+    if (timers_state.cpu_ticks_enabled) {
+        timers_state.cpu_ticks_offset = cpu_get_ticks();
+        timers_state.cpu_clock_offset = cpu_get_clock();
+        timers_state.cpu_ticks_enabled = 0;
+    }
+}
+
+/***********************************************************/
+/* timers */
+
+#define QEMU_CLOCK_REALTIME 0
+#define QEMU_CLOCK_VIRTUAL  1
+#define QEMU_CLOCK_HOST     2
+
+struct QEMUClock {
+    int type;
+    int enabled;
+    /* XXX: add frequency */
+};
+
+struct QEMUTimer {
+    QEMUClock *clock;
+    int64_t expire_time;
+    QEMUTimerCB *cb;
+    void *opaque;
+    struct QEMUTimer *next;
+};
+
+struct qemu_alarm_timer {
+    char const *name;
+    int (*start)(struct qemu_alarm_timer *t);
+    void (*stop)(struct qemu_alarm_timer *t);
+    void (*rearm)(struct qemu_alarm_timer *t);
+    void *priv;
+
+    QEMUBH *bh;
+    char expired;
+};
+
+static struct qemu_alarm_timer *alarm_timer;
+
+int qemu_alarm_pending(void)
+{
+    return qemu_bh_scheduled(alarm_timer->bh);
+}
+
+static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
+{
+    return !!t->rearm;
+}
+
+static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
+{
+    if (!alarm_has_dynticks(t))
+        return;
+
+    t->rearm(t);
+}
+
+/* TODO: MIN_TIMER_REARM_US should be optimized */
+#define MIN_TIMER_REARM_US 250
+
+#ifdef _WIN32
+
+struct qemu_alarm_win32 {
+    MMRESULT timerId;
+    unsigned int period;
+} alarm_win32_data = {0, 0};
+
+static int win32_start_timer(struct qemu_alarm_timer *t);
+static void win32_stop_timer(struct qemu_alarm_timer *t);
+static void win32_rearm_timer(struct qemu_alarm_timer *t);
+
+#else
+
+static int unix_start_timer(struct qemu_alarm_timer *t);
+static void unix_stop_timer(struct qemu_alarm_timer *t);
+
+#ifdef __linux__
+
+static int dynticks_start_timer(struct qemu_alarm_timer *t);
+static void dynticks_stop_timer(struct qemu_alarm_timer *t);
+static void dynticks_rearm_timer(struct qemu_alarm_timer *t);
+
+static int hpet_start_timer(struct qemu_alarm_timer *t);
+static void hpet_stop_timer(struct qemu_alarm_timer *t);
+
+static int rtc_start_timer(struct qemu_alarm_timer *t);
+static void rtc_stop_timer(struct qemu_alarm_timer *t);
+
+#endif /* __linux__ */
+
+#endif /* _WIN32 */
+
+/* Correlation between real and virtual time is always going to be
+   fairly approximate, so ignore small variation.
+   When the guest is idle real and virtual time will be aligned in
+   the IO wait loop.  */
+#define ICOUNT_WOBBLE (get_ticks_per_sec() / 10)
+
+static void icount_adjust(void)
+{
+    int64_t cur_time;
+    int64_t cur_icount;
+    int64_t delta;
+    static int64_t last_delta;
+    /* If the VM is not running, then do nothing.  */
+    if (!vm_running)
+        return;
+
+    cur_time = cpu_get_clock();
+    cur_icount = qemu_get_clock(vm_clock);
+    delta = cur_icount - cur_time;
+    /* FIXME: This is a very crude algorithm, somewhat prone to oscillation.  */
+    if (delta > 0
+        && last_delta + ICOUNT_WOBBLE < delta * 2
+        && icount_time_shift > 0) {
+        /* The guest is getting too far ahead.  Slow time down.  */
+        icount_time_shift--;
+    }
+    if (delta < 0
+        && last_delta - ICOUNT_WOBBLE > delta * 2
+        && icount_time_shift < MAX_ICOUNT_SHIFT) {
+        /* The guest is getting too far behind.  Speed time up.  */
+        icount_time_shift++;
+    }
+    last_delta = delta;
+    qemu_icount_bias = cur_icount - (qemu_icount << icount_time_shift);
+}
+
+static void icount_adjust_rt(void * opaque)
+{
+    qemu_mod_timer(icount_rt_timer,
+                   qemu_get_clock(rt_clock) + 1000);
+    icount_adjust();
+}
+
+static void icount_adjust_vm(void * opaque)
+{
+    qemu_mod_timer(icount_vm_timer,
+                   qemu_get_clock(vm_clock) + get_ticks_per_sec() / 10);
+    icount_adjust();
+}
+
+int64_t qemu_icount_round(int64_t count)
+{
+    return (count + (1 << icount_time_shift) - 1) >> icount_time_shift;
+}
+
+static struct qemu_alarm_timer alarm_timers[] = {
+#ifndef _WIN32
+#ifdef __linux__
+    {"dynticks", dynticks_start_timer,
+     dynticks_stop_timer, dynticks_rearm_timer, NULL},
+    /* HPET - if available - is preferred */
+    {"hpet", hpet_start_timer, hpet_stop_timer, NULL, NULL},
+    /* ...otherwise try RTC */
+    {"rtc", rtc_start_timer, rtc_stop_timer, NULL, NULL},
+#endif
+    {"unix", unix_start_timer, unix_stop_timer, NULL, NULL},
+#else
+    {"dynticks", win32_start_timer,
+     win32_stop_timer, win32_rearm_timer, &alarm_win32_data},
+    {"win32", win32_start_timer,
+     win32_stop_timer, NULL, &alarm_win32_data},
+#endif
+    {NULL, }
+};
+
+static void show_available_alarms(void)
+{
+    int i;
+
+    printf("Available alarm timers, in order of precedence:\n");
+    for (i = 0; alarm_timers[i].name; i++)
+        printf("%s\n", alarm_timers[i].name);
+}
+
+void configure_alarms(char const *opt)
+{
+    int i;
+    int cur = 0;
+    int count = ARRAY_SIZE(alarm_timers) - 1;
+    char *arg;
+    char *name;
+    struct qemu_alarm_timer tmp;
+
+    if (!strcmp(opt, "?")) {
+        show_available_alarms();
+        exit(0);
+    }
+
+    arg = qemu_strdup(opt);
+
+    /* Reorder the array */
+    name = strtok(arg, ",");
+    while (name) {
+        for (i = 0; i < count && alarm_timers[i].name; i++) {
+            if (!strcmp(alarm_timers[i].name, name))
+                break;
+        }
+
+        if (i == count) {
+            fprintf(stderr, "Unknown clock %s\n", name);
+            goto next;
+        }
+
+        if (i < cur)
+            /* Ignore */
+            goto next;
+
+	/* Swap */
+        tmp = alarm_timers[i];
+        alarm_timers[i] = alarm_timers[cur];
+        alarm_timers[cur] = tmp;
+
+        cur++;
+next:
+        name = strtok(NULL, ",");
+    }
+
+    qemu_free(arg);
+
+    if (cur) {
+        /* Disable remaining timers */
+        for (i = cur; i < count; i++)
+            alarm_timers[i].name = NULL;
+    } else {
+        show_available_alarms();
+        exit(1);
+    }
+}
+
+#define QEMU_NUM_CLOCKS 3
+
+QEMUClock *rt_clock;
+QEMUClock *vm_clock;
+QEMUClock *host_clock;
+
+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;
+    return clock;
+}
+
+void qemu_clock_enable(QEMUClock *clock, int enabled)
+{
+    clock->enabled = enabled;
+}
+
+QEMUTimer *qemu_new_timer(QEMUClock *clock, QEMUTimerCB *cb, void *opaque)
+{
+    QEMUTimer *ts;
+
+    ts = qemu_mallocz(sizeof(QEMUTimer));
+    ts->clock = clock;
+    ts->cb = cb;
+    ts->opaque = opaque;
+    return ts;
+}
+
+void qemu_free_timer(QEMUTimer *ts)
+{
+    qemu_free(ts);
+}
+
+/* stop a timer, but do not dealloc it */
+void qemu_del_timer(QEMUTimer *ts)
+{
+    QEMUTimer **pt, *t;
+
+    /* NOTE: this code must be signal safe because
+       qemu_timer_expired() can be called from a signal. */
+    pt = &active_timers[ts->clock->type];
+    for(;;) {
+        t = *pt;
+        if (!t)
+            break;
+        if (t == ts) {
+            *pt = t->next;
+            break;
+        }
+        pt = &t->next;
+    }
+}
+
+/* modify the current timer so that it will be fired when current_time
+   >= expire_time. The corresponding callback will be called. */
+void qemu_mod_timer(QEMUTimer *ts, int64_t expire_time)
+{
+    QEMUTimer **pt, *t;
+
+    qemu_del_timer(ts);
+
+    /* add the timer in the sorted list */
+    /* NOTE: this code must be signal safe because
+       qemu_timer_expired() can be called from a signal. */
+    pt = &active_timers[ts->clock->type];
+    for(;;) {
+        t = *pt;
+        if (!t)
+            break;
+        if (t->expire_time > expire_time)
+            break;
+        pt = &t->next;
+    }
+    ts->expire_time = expire_time;
+    ts->next = *pt;
+    *pt = ts;
+
+    /* Rearm if necessary  */
+    if (pt == &active_timers[ts->clock->type]) {
+        if (!qemu_alarm_pending()) {
+            qemu_rearm_alarm_timer(alarm_timer);
+        }
+        /* Interrupt execution to force deadline recalculation.  */
+        if (use_icount)
+            qemu_notify_event();
+    }
+}
+
+int qemu_timer_pending(QEMUTimer *ts)
+{
+    QEMUTimer *t;
+    for(t = active_timers[ts->clock->type]; t != NULL; t = t->next) {
+        if (t == ts)
+            return 1;
+    }
+    return 0;
+}
+
+int qemu_timer_expired(QEMUTimer *timer_head, int64_t current_time)
+{
+    if (!timer_head)
+        return 0;
+    return (timer_head->expire_time <= current_time);
+}
+
+static void qemu_run_timers(QEMUClock *clock)
+{
+    QEMUTimer **ptimer_head, *ts;
+    int64_t current_time;
+   
+    if (!clock->enabled)
+        return;
+
+    current_time = qemu_get_clock (clock);
+    ptimer_head = &active_timers[clock->type];
+    for(;;) {
+        ts = *ptimer_head;
+        if (!ts || ts->expire_time > current_time)
+            break;
+        /* remove timer from the list before calling the callback */
+        *ptimer_head = ts->next;
+        ts->next = NULL;
+
+        /* run the callback (the timer list can be modified) */
+        ts->cb(ts->opaque);
+    }
+}
+
+int64_t qemu_get_clock(QEMUClock *clock)
+{
+    switch(clock->type) {
+    case QEMU_CLOCK_REALTIME:
+        return get_clock() / 1000000;
+    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();
+    }
+}
+
+int64_t qemu_get_clock_ns(QEMUClock *clock)
+{
+    switch(clock->type) {
+    case 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();
+    }
+}
+
+void init_clocks(void)
+{
+    init_get_clock();
+    rt_clock = qemu_new_clock(QEMU_CLOCK_REALTIME);
+    vm_clock = qemu_new_clock(QEMU_CLOCK_VIRTUAL);
+    host_clock = qemu_new_clock(QEMU_CLOCK_HOST);
+
+    rtc_clock = host_clock;
+}
+
+/* save a timer */
+void qemu_put_timer(QEMUFile *f, QEMUTimer *ts)
+{
+    uint64_t expire_time;
+
+    if (qemu_timer_pending(ts)) {
+        expire_time = ts->expire_time;
+    } else {
+        expire_time = -1;
+    }
+    qemu_put_be64(f, expire_time);
+}
+
+void qemu_get_timer(QEMUFile *f, QEMUTimer *ts)
+{
+    uint64_t expire_time;
+
+    expire_time = qemu_get_be64(f);
+    if (expire_time != -1) {
+        qemu_mod_timer(ts, expire_time);
+    } else {
+        qemu_del_timer(ts);
+    }
+}
+
+static const VMStateDescription vmstate_timers = {
+    .name = "timer",
+    .version_id = 2,
+    .minimum_version_id = 1,
+    .minimum_version_id_old = 1,
+    .fields      = (VMStateField []) {
+        VMSTATE_INT64(cpu_ticks_offset, TimersState),
+        VMSTATE_INT64(dummy, TimersState),
+        VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+void configure_icount(const char *option)
+{
+    vmstate_register(0, &vmstate_timers, &timers_state);
+    if (!option)
+        return;
+
+    if (strcmp(option, "auto") != 0) {
+        icount_time_shift = strtol(option, NULL, 0);
+        use_icount = 1;
+        return;
+    }
+
+    use_icount = 2;
+
+    /* 125MIPS seems a reasonable initial guess at the guest speed.
+       It will be corrected fairly quickly anyway.  */
+    icount_time_shift = 3;
+
+    /* Have both realtime and virtual time triggers for speed adjustment.
+       The realtime trigger catches emulated time passing too slowly,
+       the virtual time trigger catches emulated time passing too fast.
+       Realtime triggers occur even when idle, so use them less frequently
+       than VM triggers.  */
+    icount_rt_timer = qemu_new_timer(rt_clock, icount_adjust_rt, NULL);
+    qemu_mod_timer(icount_rt_timer,
+                   qemu_get_clock(rt_clock) + 1000);
+    icount_vm_timer = qemu_new_timer(vm_clock, icount_adjust_vm, NULL);
+    qemu_mod_timer(icount_vm_timer,
+                   qemu_get_clock(vm_clock) + get_ticks_per_sec() / 10);
+}
+
+static void qemu_timer_bh(void *opaque)
+{
+    struct qemu_alarm_timer *t = opaque;
+
+    /* rearm timer, if not periodic */
+    if (t->expired) {
+        t->expired = 0;
+        qemu_rearm_alarm_timer(t);
+    }
+
+    /* vm time timers */
+    if (vm_running) {
+        qemu_run_timers(vm_clock);
+    }
+
+    qemu_run_timers(rt_clock);
+    qemu_run_timers(host_clock);
+}
+
+#ifdef _WIN32
+static void CALLBACK host_alarm_handler(UINT uTimerID, UINT uMsg,
+                                        DWORD_PTR dwUser, DWORD_PTR dw1,
+                                        DWORD_PTR dw2)
+#else
+static void host_alarm_handler(int host_signum)
+#endif
+{
+    struct qemu_alarm_timer *t = alarm_timer;
+    if (!t)
+	return;
+
+#if 0
+#define DISP_FREQ 1000
+    {
+        static int64_t delta_min = INT64_MAX;
+        static int64_t delta_max, delta_cum, last_clock, delta, ti;
+        static int count;
+        ti = qemu_get_clock(vm_clock);
+        if (last_clock != 0) {
+            delta = ti - last_clock;
+            if (delta < delta_min)
+                delta_min = delta;
+            if (delta > delta_max)
+                delta_max = delta;
+            delta_cum += delta;
+            if (++count == DISP_FREQ) {
+                printf("timer: min=%" PRId64 " us max=%" PRId64 " us avg=%" PRId64 " us avg_freq=%0.3f Hz\n",
+                       muldiv64(delta_min, 1000000, get_ticks_per_sec()),
+                       muldiv64(delta_max, 1000000, get_ticks_per_sec()),
+                       muldiv64(delta_cum, 1000000 / DISP_FREQ, get_ticks_per_sec()),
+                       (double)get_ticks_per_sec() / ((double)delta_cum / DISP_FREQ));
+                count = 0;
+                delta_min = INT64_MAX;
+                delta_max = 0;
+                delta_cum = 0;
+            }
+        }
+        last_clock = ti;
+    }
+#endif
+    if (alarm_has_dynticks(t) ||
+        (!use_icount &&
+            qemu_timer_expired(active_timers[QEMU_CLOCK_VIRTUAL],
+                               qemu_get_clock(vm_clock))) ||
+        qemu_timer_expired(active_timers[QEMU_CLOCK_REALTIME],
+                           qemu_get_clock(rt_clock)) ||
+        qemu_timer_expired(active_timers[QEMU_CLOCK_HOST],
+                           qemu_get_clock(host_clock))) {
+        qemu_notify_event();
+        t->expired = alarm_has_dynticks(t);
+        qemu_bh_schedule(t->bh);
+    }
+}
+
+int64_t qemu_next_deadline(void)
+{
+    /* To avoid problems with overflow limit this to 2^32.  */
+    int64_t delta = INT32_MAX;
+
+    if (active_timers[QEMU_CLOCK_VIRTUAL]) {
+        delta = active_timers[QEMU_CLOCK_VIRTUAL]->expire_time -
+                     qemu_get_clock(vm_clock);
+    }
+    if (active_timers[QEMU_CLOCK_HOST]) {
+        int64_t hdelta = active_timers[QEMU_CLOCK_HOST]->expire_time -
+                 qemu_get_clock(host_clock);
+        if (hdelta < delta)
+            delta = hdelta;
+    }
+
+    if (delta < 0)
+        delta = 0;
+
+    return delta;
+}
+
+#if defined(__linux__)
+static uint64_t qemu_next_deadline_dyntick(void)
+{
+    int64_t delta;
+    int64_t rtdelta;
+
+    if (use_icount)
+        delta = INT32_MAX;
+    else
+        delta = (qemu_next_deadline() + 999) / 1000;
+
+    if (active_timers[QEMU_CLOCK_REALTIME]) {
+        rtdelta = (active_timers[QEMU_CLOCK_REALTIME]->expire_time -
+                 qemu_get_clock(rt_clock))*1000;
+        if (rtdelta < delta)
+            delta = rtdelta;
+    }
+
+    if (delta < MIN_TIMER_REARM_US)
+        delta = MIN_TIMER_REARM_US;
+
+    return delta;
+}
+#endif
+
+#ifndef _WIN32
+
+/* Sets a specific flag */
+static int fcntl_setfl(int fd, int flag)
+{
+    int flags;
+
+    flags = fcntl(fd, F_GETFL);
+    if (flags == -1)
+        return -errno;
+
+    if (fcntl(fd, F_SETFL, flags | flag) == -1)
+        return -errno;
+
+    return 0;
+}
+
+#if defined(__linux__)
+
+#define RTC_FREQ 1024
+
+static void enable_sigio_timer(int fd)
+{
+    struct sigaction act;
+
+    /* timer signal */
+    sigfillset(&act.sa_mask);
+    act.sa_flags = 0;
+    act.sa_handler = host_alarm_handler;
+
+    sigaction(SIGIO, &act, NULL);
+    fcntl_setfl(fd, O_ASYNC);
+    fcntl(fd, F_SETOWN, getpid());
+}
+
+static int hpet_start_timer(struct qemu_alarm_timer *t)
+{
+    struct hpet_info info;
+    int r, fd;
+
+    fd = qemu_open("/dev/hpet", O_RDONLY);
+    if (fd < 0)
+        return -1;
+
+    /* Set frequency */
+    r = ioctl(fd, HPET_IRQFREQ, RTC_FREQ);
+    if (r < 0) {
+        fprintf(stderr, "Could not configure '/dev/hpet' to have a 1024Hz timer. This is not a fatal\n"
+                "error, but for better emulation accuracy type:\n"
+                "'echo 1024 > /proc/sys/dev/hpet/max-user-freq' as root.\n");
+        goto fail;
+    }
+
+    /* Check capabilities */
+    r = ioctl(fd, HPET_INFO, &info);
+    if (r < 0)
+        goto fail;
+
+    /* Enable periodic mode */
+    r = ioctl(fd, HPET_EPI, 0);
+    if (info.hi_flags && (r < 0))
+        goto fail;
+
+    /* Enable interrupt */
+    r = ioctl(fd, HPET_IE_ON, 0);
+    if (r < 0)
+        goto fail;
+
+    enable_sigio_timer(fd);
+    t->priv = (void *)(long)fd;
+
+    return 0;
+fail:
+    close(fd);
+    return -1;
+}
+
+static void hpet_stop_timer(struct qemu_alarm_timer *t)
+{
+    int fd = (long)t->priv;
+
+    close(fd);
+}
+
+static int rtc_start_timer(struct qemu_alarm_timer *t)
+{
+    int rtc_fd;
+    unsigned long current_rtc_freq = 0;
+
+    TFR(rtc_fd = qemu_open("/dev/rtc", O_RDONLY));
+    if (rtc_fd < 0)
+        return -1;
+    ioctl(rtc_fd, RTC_IRQP_READ, &current_rtc_freq);
+    if (current_rtc_freq != RTC_FREQ &&
+        ioctl(rtc_fd, RTC_IRQP_SET, RTC_FREQ) < 0) {
+        fprintf(stderr, "Could not configure '/dev/rtc' to have a 1024 Hz timer. This is not a fatal\n"
+                "error, but for better emulation accuracy either use a 2.6 host Linux kernel or\n"
+                "type 'echo 1024 > /proc/sys/dev/rtc/max-user-freq' as root.\n");
+        goto fail;
+    }
+    if (ioctl(rtc_fd, RTC_PIE_ON, 0) < 0) {
+    fail:
+        close(rtc_fd);
+        return -1;
+    }
+
+    enable_sigio_timer(rtc_fd);
+
+    t->priv = (void *)(long)rtc_fd;
+
+    return 0;
+}
+
+static void rtc_stop_timer(struct qemu_alarm_timer *t)
+{
+    int rtc_fd = (long)t->priv;
+
+    close(rtc_fd);
+}
+
+static int dynticks_start_timer(struct qemu_alarm_timer *t)
+{
+    struct sigevent ev;
+    timer_t host_timer;
+    struct sigaction act;
+
+    sigfillset(&act.sa_mask);
+    act.sa_flags = 0;
+    act.sa_handler = host_alarm_handler;
+
+    sigaction(SIGALRM, &act, NULL);
+
+    /* 
+     * Initialize ev struct to 0 to avoid valgrind complaining
+     * about uninitialized data in timer_create call
+     */
+    memset(&ev, 0, sizeof(ev));
+    ev.sigev_value.sival_int = 0;
+    ev.sigev_notify = SIGEV_SIGNAL;
+    ev.sigev_signo = SIGALRM;
+
+    if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) {
+        perror("timer_create");
+
+        /* disable dynticks */
+        fprintf(stderr, "Dynamic Ticks disabled\n");
+
+        return -1;
+    }
+
+    t->priv = (void *)(long)host_timer;
+
+    return 0;
+}
+
+static void dynticks_stop_timer(struct qemu_alarm_timer *t)
+{
+    timer_t host_timer = (timer_t)(long)t->priv;
+
+    timer_delete(host_timer);
+}
+
+static void dynticks_rearm_timer(struct qemu_alarm_timer *t)
+{
+    timer_t host_timer = (timer_t)(long)t->priv;
+    struct itimerspec timeout;
+    int64_t nearest_delta_us = INT64_MAX;
+    int64_t current_us;
+
+    assert(alarm_has_dynticks(t));
+    if (!active_timers[QEMU_CLOCK_REALTIME] &&
+        !active_timers[QEMU_CLOCK_VIRTUAL] &&
+        !active_timers[QEMU_CLOCK_HOST])
+        return;
+
+    nearest_delta_us = qemu_next_deadline_dyntick();
+
+    /* check whether a timer is already running */
+    if (timer_gettime(host_timer, &timeout)) {
+        perror("gettime");
+        fprintf(stderr, "Internal timer error: aborting\n");
+        exit(1);
+    }
+    current_us = timeout.it_value.tv_sec * 1000000 + timeout.it_value.tv_nsec/1000;
+    if (current_us && current_us <= nearest_delta_us)
+        return;
+
+    timeout.it_interval.tv_sec = 0;
+    timeout.it_interval.tv_nsec = 0; /* 0 for one-shot timer */
+    timeout.it_value.tv_sec =  nearest_delta_us / 1000000;
+    timeout.it_value.tv_nsec = (nearest_delta_us % 1000000) * 1000;
+    if (timer_settime(host_timer, 0 /* RELATIVE */, &timeout, NULL)) {
+        perror("settime");
+        fprintf(stderr, "Internal timer error: aborting\n");
+        exit(1);
+    }
+}
+
+#endif /* defined(__linux__) */
+
+static int unix_start_timer(struct qemu_alarm_timer *t)
+{
+    struct sigaction act;
+    struct itimerval itv;
+    int err;
+
+    /* timer signal */
+    sigfillset(&act.sa_mask);
+    act.sa_flags = 0;
+    act.sa_handler = host_alarm_handler;
+
+    sigaction(SIGALRM, &act, NULL);
+
+    itv.it_interval.tv_sec = 0;
+    /* for i386 kernel 2.6 to get 1 ms */
+    itv.it_interval.tv_usec = 999;
+    itv.it_value.tv_sec = 0;
+    itv.it_value.tv_usec = 10 * 1000;
+
+    err = setitimer(ITIMER_REAL, &itv, NULL);
+    if (err)
+        return -1;
+
+    return 0;
+}
+
+static void unix_stop_timer(struct qemu_alarm_timer *t)
+{
+    struct itimerval itv;
+
+    memset(&itv, 0, sizeof(itv));
+    setitimer(ITIMER_REAL, &itv, NULL);
+}
+
+#endif /* !defined(_WIN32) */
+
+
+#ifdef _WIN32
+
+static int win32_start_timer(struct qemu_alarm_timer *t)
+{
+    TIMECAPS tc;
+    struct qemu_alarm_win32 *data = t->priv;
+    UINT flags;
+
+    memset(&tc, 0, sizeof(tc));
+    timeGetDevCaps(&tc, sizeof(tc));
+
+    data->period = tc.wPeriodMin;
+    timeBeginPeriod(data->period);
+
+    flags = TIME_CALLBACK_FUNCTION;
+    if (alarm_has_dynticks(t))
+        flags |= TIME_ONESHOT;
+    else
+        flags |= TIME_PERIODIC;
+
+    data->timerId = timeSetEvent(1,         // interval (ms)
+                        data->period,       // resolution
+                        host_alarm_handler, // function
+                        (DWORD)t,           // parameter
+                        flags);
+
+    if (!data->timerId) {
+        fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n",
+                GetLastError());
+        timeEndPeriod(data->period);
+        return -1;
+    }
+
+    return 0;
+}
+
+static void win32_stop_timer(struct qemu_alarm_timer *t)
+{
+    struct qemu_alarm_win32 *data = t->priv;
+
+    timeKillEvent(data->timerId);
+    timeEndPeriod(data->period);
+}
+
+static void win32_rearm_timer(struct qemu_alarm_timer *t)
+{
+    struct qemu_alarm_win32 *data = t->priv;
+
+    assert(alarm_has_dynticks(t));
+    if (!active_timers[QEMU_CLOCK_REALTIME] &&
+        !active_timers[QEMU_CLOCK_VIRTUAL] &&
+        !active_timers[QEMU_CLOCK_HOST])
+        return;
+
+    timeKillEvent(data->timerId);
+
+    data->timerId = timeSetEvent(1,
+                        data->period,
+                        host_alarm_handler,
+                        (DWORD)t,
+                        TIME_ONESHOT | TIME_CALLBACK_FUNCTION);
+
+    if (!data->timerId) {
+        fprintf(stderr, "Failed to re-arm win32 alarm timer %ld\n",
+                GetLastError());
+
+        timeEndPeriod(data->period);
+        exit(1);
+    }
+}
+
+#endif /* _WIN32 */
+
+static void alarm_timer_on_change_state_rearm(void *opaque, int running, int reason)
+{
+    if (running)
+        qemu_rearm_alarm_timer((struct qemu_alarm_timer *) opaque);
+}
+
+int init_timer_alarm(void)
+{
+    struct qemu_alarm_timer *t = NULL;
+    int i, err = -1;
+
+    for (i = 0; alarm_timers[i].name; i++) {
+        t = &alarm_timers[i];
+
+        err = t->start(t);
+        if (!err)
+            break;
+    }
+
+    if (err) {
+        err = -ENOENT;
+        goto fail;
+    }
+
+    /* first event is at time 0 */
+    t->bh = qemu_bh_new(qemu_timer_bh, t);
+    qemu_bh_schedule(t->bh);
+    alarm_timer = t;
+    qemu_add_vm_change_state_handler(alarm_timer_on_change_state_rearm, t);
+
+    return 0;
+
+fail:
+    return err;
+}
+
+void quit_timers(void)
+{
+    struct qemu_alarm_timer *t = alarm_timer;
+    alarm_timer = NULL;
+    t->stop(t);
+}
+
+int qemu_calculate_timeout(void)
+{
+#ifndef CONFIG_IOTHREAD
+    int timeout;
+
+    if (!vm_running)
+        timeout = 5000;
+    else {
+     /* XXX: use timeout computed from timers */
+        int64_t add;
+        int64_t delta;
+        /* Advance virtual time to the next event.  */
+	delta = qemu_icount_delta();
+        if (delta > 0) {
+            /* If virtual time is ahead of real time then just
+               wait for IO.  */
+            timeout = (delta + 999999) / 1000000;
+        } else {
+            /* Wait for either IO to occur or the next
+               timer event.  */
+            add = qemu_next_deadline();
+            /* We advance the timer before checking for IO.
+               Limit the amount we advance so that early IO
+               activity won't get the guest too far ahead.  */
+            if (add > 10000000)
+                add = 10000000;
+            delta += add;
+            qemu_icount += qemu_icount_round (add);
+            timeout = delta / 1000000;
+            if (timeout < 0)
+                timeout = 0;
+        }
+    }
+
+    return timeout;
+#else /* CONFIG_IOTHREAD */
+    return 1000;
+#endif
+}
+
diff --git a/qemu-timer.h b/qemu-timer.h
index c17b4e6..d114bb8 100644
--- a/qemu-timer.h
+++ b/qemu-timer.h
@@ -26,6 +26,7 @@ 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);
 
 QEMUTimer *qemu_new_timer(QEMUClock *clock, QEMUTimerCB *cb, void *opaque);
 void qemu_free_timer(QEMUTimer *ts);
@@ -34,11 +35,21 @@ void qemu_mod_timer(QEMUTimer *ts, int64_t expire_time);
 int qemu_timer_pending(QEMUTimer *ts);
 int qemu_timer_expired(QEMUTimer *timer_head, int64_t current_time);
 
+int qemu_alarm_pending(void);
+int64_t qemu_next_deadline(void);
+void configure_alarms(char const *opt);
+void configure_icount(const char *option);
+int qemu_calculate_timeout(void);
+void init_clocks(void);
+int init_timer_alarm(void);
+void quit_timers(void);
+
 static inline int64_t get_ticks_per_sec(void)
 {
     return 1000000000LL;
 }
 
+
 void qemu_get_timer(QEMUFile *f, QEMUTimer *ts);
 void qemu_put_timer(QEMUFile *f, QEMUTimer *ts);
 
diff --git a/vl.c b/vl.c
index 17ad374..a7a9193 100644
--- a/vl.c
+++ b/vl.c
@@ -59,14 +59,8 @@
 #ifdef __linux__
 #include <pty.h>
 #include <malloc.h>
-#include <linux/rtc.h>
 #include <sys/prctl.h>
 
-/* For the benefit of older linux systems which don't supply it,
-   we use a local copy of hpet.h. */
-/* #include <linux/hpet.h> */
-#include "hpet.h"
-
 #include <linux/ppdev.h>
 #include <linux/parport.h>
 #endif
@@ -101,7 +95,6 @@ extern int madvise(caddr_t, size_t, int);
 
 #ifdef _WIN32
 #include <windows.h>
-#include <mmsystem.h>
 #endif
 
 #ifdef CONFIG_SDL
@@ -253,14 +246,6 @@ uint64_t node_cpumask[MAX_NODES];
 
 static CPUState *cur_cpu;
 static CPUState *next_cpu;
-/* Conversion factor from emulated instructions to virtual clock ticks.  */
-static int icount_time_shift;
-/* Arbitrarily pick 1MIPS as the minimum allowable speed.  */
-#define MAX_ICOUNT_SHIFT 10
-/* Compensate for varying guest execution speed.  */
-static int64_t qemu_icount_bias;
-static QEMUTimer *icount_rt_timer;
-static QEMUTimer *icount_vm_timer;
 static QEMUTimer *nographic_timer;
 
 uint8_t qemu_uuid[16];
@@ -398,1115 +383,6 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c)
     return res.ll;
 }
 
-static int64_t get_clock_realtime(void)
-{
-    struct timeval tv;
-
-    gettimeofday(&tv, NULL);
-    return tv.tv_sec * 1000000000LL + (tv.tv_usec * 1000);
-}
-
-#ifdef WIN32
-
-static int64_t clock_freq;
-
-static void init_get_clock(void)
-{
-    LARGE_INTEGER freq;
-    int ret;
-    ret = QueryPerformanceFrequency(&freq);
-    if (ret == 0) {
-        fprintf(stderr, "Could not calibrate ticks\n");
-        exit(1);
-    }
-    clock_freq = freq.QuadPart;
-}
-
-static int64_t get_clock(void)
-{
-    LARGE_INTEGER ti;
-    QueryPerformanceCounter(&ti);
-    return muldiv64(ti.QuadPart, get_ticks_per_sec(), clock_freq);
-}
-
-#else
-
-static int use_rt_clock;
-
-static void init_get_clock(void)
-{
-    use_rt_clock = 0;
-#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
-    || defined(__DragonFly__) || defined(__FreeBSD_kernel__)
-    {
-        struct timespec ts;
-        if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) {
-            use_rt_clock = 1;
-        }
-    }
-#endif
-}
-
-static int64_t get_clock(void)
-{
-#if defined(__linux__) || (defined(__FreeBSD__) && __FreeBSD_version >= 500000) \
-	|| defined(__DragonFly__) || defined(__FreeBSD_kernel__)
-    if (use_rt_clock) {
-        struct timespec ts;
-        clock_gettime(CLOCK_MONOTONIC, &ts);
-        return ts.tv_sec * 1000000000LL + ts.tv_nsec;
-    } else
-#endif
-    {
-        /* XXX: using gettimeofday leads to problems if the date
-           changes, so it should be avoided. */
-        return get_clock_realtime();
-    }
-}
-#endif
-
-/* Return the virtual CPU time, based on the instruction counter.  */
-static int64_t cpu_get_icount(void)
-{
-    int64_t icount;
-    CPUState *env = cpu_single_env;;
-    icount = qemu_icount;
-    if (env) {
-        if (!can_do_io(env))
-            fprintf(stderr, "Bad clock read\n");
-        icount -= (env->icount_decr.u16.low + env->icount_extra);
-    }
-    return qemu_icount_bias + (icount << icount_time_shift);
-}
-
-/***********************************************************/
-/* guest cycle counter */
-
-typedef struct TimersState {
-    int64_t cpu_ticks_prev;
-    int64_t cpu_ticks_offset;
-    int64_t cpu_clock_offset;
-    int32_t cpu_ticks_enabled;
-    int64_t dummy;
-} TimersState;
-
-TimersState timers_state;
-
-/* return the host CPU cycle counter and handle stop/restart */
-int64_t cpu_get_ticks(void)
-{
-    if (use_icount) {
-        return cpu_get_icount();
-    }
-    if (!timers_state.cpu_ticks_enabled) {
-        return timers_state.cpu_ticks_offset;
-    } else {
-        int64_t ticks;
-        ticks = cpu_get_real_ticks();
-        if (timers_state.cpu_ticks_prev > ticks) {
-            /* Note: non increasing ticks may happen if the host uses
-               software suspend */
-            timers_state.cpu_ticks_offset += timers_state.cpu_ticks_prev - ticks;
-        }
-        timers_state.cpu_ticks_prev = ticks;
-        return ticks + timers_state.cpu_ticks_offset;
-    }
-}
-
-/* return the host CPU monotonic timer and handle stop/restart */
-static int64_t cpu_get_clock(void)
-{
-    int64_t ti;
-    if (!timers_state.cpu_ticks_enabled) {
-        return timers_state.cpu_clock_offset;
-    } else {
-        ti = get_clock();
-        return ti + timers_state.cpu_clock_offset;
-    }
-}
-
-static int64_t qemu_icount_delta(void)
-{
-    if (!use_icount) {
-        return 5000 * (int64_t) 1000000;
-    } else if (use_icount == 1) {
-        /* When not using an adaptive execution frequency
-           we tend to get badly out of sync with real time,
-           so just delay for a reasonable amount of time.  */
-        return 0;
-    } else {
-        return cpu_get_icount() - cpu_get_clock();
-    }
-}
-
-/* enable cpu_get_ticks() */
-void cpu_enable_ticks(void)
-{
-    if (!timers_state.cpu_ticks_enabled) {
-        timers_state.cpu_ticks_offset -= cpu_get_real_ticks();
-        timers_state.cpu_clock_offset -= get_clock();
-        timers_state.cpu_ticks_enabled = 1;
-    }
-}
-
-/* disable cpu_get_ticks() : the clock is stopped. You must not call
-   cpu_get_ticks() after that.  */
-void cpu_disable_ticks(void)
-{
-    if (timers_state.cpu_ticks_enabled) {
-        timers_state.cpu_ticks_offset = cpu_get_ticks();
-        timers_state.cpu_clock_offset = cpu_get_clock();
-        timers_state.cpu_ticks_enabled = 0;
-    }
-}
-
-/***********************************************************/
-/* timers */
-
-#define QEMU_CLOCK_REALTIME 0
-#define QEMU_CLOCK_VIRTUAL  1
-#define QEMU_CLOCK_HOST     2
-
-struct QEMUClock {
-    int type;
-    int enabled;
-    /* XXX: add frequency */
-};
-
-struct QEMUTimer {
-    QEMUClock *clock;
-    int64_t expire_time;
-    QEMUTimerCB *cb;
-    void *opaque;
-    struct QEMUTimer *next;
-};
-
-struct qemu_alarm_timer {
-    char const *name;
-    int (*start)(struct qemu_alarm_timer *t);
-    void (*stop)(struct qemu_alarm_timer *t);
-    void (*rearm)(struct qemu_alarm_timer *t);
-    void *priv;
-
-    QEMUBH *bh;
-    char expired;
-};
-
-static struct qemu_alarm_timer *alarm_timer;
-static int qemu_calculate_timeout(void);
-
-static inline int qemu_alarm_pending(void)
-{
-    return qemu_bh_scheduled(alarm_timer->bh);
-}
-
-static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
-{
-    return !!t->rearm;
-}
-
-static void qemu_rearm_alarm_timer(struct qemu_alarm_timer *t)
-{
-    if (!alarm_has_dynticks(t))
-        return;
-
-    t->rearm(t);
-}
-
-/* TODO: MIN_TIMER_REARM_US should be optimized */
-#define MIN_TIMER_REARM_US 250
-
-#ifdef _WIN32
-
-struct qemu_alarm_win32 {
-    MMRESULT timerId;
-    unsigned int period;
-} alarm_win32_data = {0, 0};
-
-static int win32_start_timer(struct qemu_alarm_timer *t);
-static void win32_stop_timer(struct qemu_alarm_timer *t);
-static void win32_rearm_timer(struct qemu_alarm_timer *t);
-
-#else
-
-static int unix_start_timer(struct qemu_alarm_timer *t);
-static void unix_stop_timer(struct qemu_alarm_timer *t);
-
-#ifdef __linux__
-
-static int dynticks_start_timer(struct qemu_alarm_timer *t);
-static void dynticks_stop_timer(struct qemu_alarm_timer *t);
-static void dynticks_rearm_timer(struct qemu_alarm_timer *t);
-
-static int hpet_start_timer(struct qemu_alarm_timer *t);
-static void hpet_stop_timer(struct qemu_alarm_timer *t);
-
-static int rtc_start_timer(struct qemu_alarm_timer *t);
-static void rtc_stop_timer(struct qemu_alarm_timer *t);
-
-#endif /* __linux__ */
-
-#endif /* _WIN32 */
-
-/* Correlation between real and virtual time is always going to be
-   fairly approximate, so ignore small variation.
-   When the guest is idle real and virtual time will be aligned in
-   the IO wait loop.  */
-#define ICOUNT_WOBBLE (get_ticks_per_sec() / 10)
-
-static void icount_adjust(void)
-{
-    int64_t cur_time;
-    int64_t cur_icount;
-    int64_t delta;
-    static int64_t last_delta;
-    /* If the VM is not running, then do nothing.  */
-    if (!vm_running)
-        return;
-
-    cur_time = cpu_get_clock();
-    cur_icount = qemu_get_clock(vm_clock);
-    delta = cur_icount - cur_time;
-    /* FIXME: This is a very crude algorithm, somewhat prone to oscillation.  */
-    if (delta > 0
-        && last_delta + ICOUNT_WOBBLE < delta * 2
-        && icount_time_shift > 0) {
-        /* The guest is getting too far ahead.  Slow time down.  */
-        icount_time_shift--;
-    }
-    if (delta < 0
-        && last_delta - ICOUNT_WOBBLE > delta * 2
-        && icount_time_shift < MAX_ICOUNT_SHIFT) {
-        /* The guest is getting too far behind.  Speed time up.  */
-        icount_time_shift++;
-    }
-    last_delta = delta;
-    qemu_icount_bias = cur_icount - (qemu_icount << icount_time_shift);
-}
-
-static void icount_adjust_rt(void * opaque)
-{
-    qemu_mod_timer(icount_rt_timer,
-                   qemu_get_clock(rt_clock) + 1000);
-    icount_adjust();
-}
-
-static void icount_adjust_vm(void * opaque)
-{
-    qemu_mod_timer(icount_vm_timer,
-                   qemu_get_clock(vm_clock) + get_ticks_per_sec() / 10);
-    icount_adjust();
-}
-
-static int64_t qemu_icount_round(int64_t count)
-{
-    return (count + (1 << icount_time_shift) - 1) >> icount_time_shift;
-}
-
-static struct qemu_alarm_timer alarm_timers[] = {
-#ifndef _WIN32
-#ifdef __linux__
-    {"dynticks", dynticks_start_timer,
-     dynticks_stop_timer, dynticks_rearm_timer, NULL},
-    /* HPET - if available - is preferred */
-    {"hpet", hpet_start_timer, hpet_stop_timer, NULL, NULL},
-    /* ...otherwise try RTC */
-    {"rtc", rtc_start_timer, rtc_stop_timer, NULL, NULL},
-#endif
-    {"unix", unix_start_timer, unix_stop_timer, NULL, NULL},
-#else
-    {"dynticks", win32_start_timer,
-     win32_stop_timer, win32_rearm_timer, &alarm_win32_data},
-    {"win32", win32_start_timer,
-     win32_stop_timer, NULL, &alarm_win32_data},
-#endif
-    {NULL, }
-};
-
-static void show_available_alarms(void)
-{
-    int i;
-
-    printf("Available alarm timers, in order of precedence:\n");
-    for (i = 0; alarm_timers[i].name; i++)
-        printf("%s\n", alarm_timers[i].name);
-}
-
-static void configure_alarms(char const *opt)
-{
-    int i;
-    int cur = 0;
-    int count = ARRAY_SIZE(alarm_timers) - 1;
-    char *arg;
-    char *name;
-    struct qemu_alarm_timer tmp;
-
-    if (!strcmp(opt, "?")) {
-        show_available_alarms();
-        exit(0);
-    }
-
-    arg = qemu_strdup(opt);
-
-    /* Reorder the array */
-    name = strtok(arg, ",");
-    while (name) {
-        for (i = 0; i < count && alarm_timers[i].name; i++) {
-            if (!strcmp(alarm_timers[i].name, name))
-                break;
-        }
-
-        if (i == count) {
-            fprintf(stderr, "Unknown clock %s\n", name);
-            goto next;
-        }
-
-        if (i < cur)
-            /* Ignore */
-            goto next;
-
-	/* Swap */
-        tmp = alarm_timers[i];
-        alarm_timers[i] = alarm_timers[cur];
-        alarm_timers[cur] = tmp;
-
-        cur++;
-next:
-        name = strtok(NULL, ",");
-    }
-
-    qemu_free(arg);
-
-    if (cur) {
-        /* Disable remaining timers */
-        for (i = cur; i < count; i++)
-            alarm_timers[i].name = NULL;
-    } else {
-        show_available_alarms();
-        exit(1);
-    }
-}
-
-#define QEMU_NUM_CLOCKS 3
-
-QEMUClock *rt_clock;
-QEMUClock *vm_clock;
-QEMUClock *host_clock;
-
-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;
-    return clock;
-}
-
-static void qemu_clock_enable(QEMUClock *clock, int enabled)
-{
-    clock->enabled = enabled;
-}
-
-QEMUTimer *qemu_new_timer(QEMUClock *clock, QEMUTimerCB *cb, void *opaque)
-{
-    QEMUTimer *ts;
-
-    ts = qemu_mallocz(sizeof(QEMUTimer));
-    ts->clock = clock;
-    ts->cb = cb;
-    ts->opaque = opaque;
-    return ts;
-}
-
-void qemu_free_timer(QEMUTimer *ts)
-{
-    qemu_free(ts);
-}
-
-/* stop a timer, but do not dealloc it */
-void qemu_del_timer(QEMUTimer *ts)
-{
-    QEMUTimer **pt, *t;
-
-    /* NOTE: this code must be signal safe because
-       qemu_timer_expired() can be called from a signal. */
-    pt = &active_timers[ts->clock->type];
-    for(;;) {
-        t = *pt;
-        if (!t)
-            break;
-        if (t == ts) {
-            *pt = t->next;
-            break;
-        }
-        pt = &t->next;
-    }
-}
-
-/* modify the current timer so that it will be fired when current_time
-   >= expire_time. The corresponding callback will be called. */
-void qemu_mod_timer(QEMUTimer *ts, int64_t expire_time)
-{
-    QEMUTimer **pt, *t;
-
-    qemu_del_timer(ts);
-
-    /* add the timer in the sorted list */
-    /* NOTE: this code must be signal safe because
-       qemu_timer_expired() can be called from a signal. */
-    pt = &active_timers[ts->clock->type];
-    for(;;) {
-        t = *pt;
-        if (!t)
-            break;
-        if (t->expire_time > expire_time)
-            break;
-        pt = &t->next;
-    }
-    ts->expire_time = expire_time;
-    ts->next = *pt;
-    *pt = ts;
-
-    /* Rearm if necessary  */
-    if (pt == &active_timers[ts->clock->type]) {
-        if (!qemu_alarm_pending()) {
-            qemu_rearm_alarm_timer(alarm_timer);
-        }
-        /* Interrupt execution to force deadline recalculation.  */
-        if (use_icount)
-            qemu_notify_event();
-    }
-}
-
-int qemu_timer_pending(QEMUTimer *ts)
-{
-    QEMUTimer *t;
-    for(t = active_timers[ts->clock->type]; t != NULL; t = t->next) {
-        if (t == ts)
-            return 1;
-    }
-    return 0;
-}
-
-int qemu_timer_expired(QEMUTimer *timer_head, int64_t current_time)
-{
-    if (!timer_head)
-        return 0;
-    return (timer_head->expire_time <= current_time);
-}
-
-static void qemu_run_timers(QEMUClock *clock)
-{
-    QEMUTimer **ptimer_head, *ts;
-    int64_t current_time;
-   
-    if (!clock->enabled)
-        return;
-
-    current_time = qemu_get_clock (clock);
-    ptimer_head = &active_timers[clock->type];
-    for(;;) {
-        ts = *ptimer_head;
-        if (!ts || ts->expire_time > current_time)
-            break;
-        /* remove timer from the list before calling the callback */
-        *ptimer_head = ts->next;
-        ts->next = NULL;
-
-        /* run the callback (the timer list can be modified) */
-        ts->cb(ts->opaque);
-    }
-}
-
-int64_t qemu_get_clock(QEMUClock *clock)
-{
-    switch(clock->type) {
-    case QEMU_CLOCK_REALTIME:
-        return get_clock() / 1000000;
-    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();
-    }
-}
-
-int64_t qemu_get_clock_ns(QEMUClock *clock)
-{
-    switch(clock->type) {
-    case 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();
-    }
-}
-
-static void init_clocks(void)
-{
-    init_get_clock();
-    rt_clock = qemu_new_clock(QEMU_CLOCK_REALTIME);
-    vm_clock = qemu_new_clock(QEMU_CLOCK_VIRTUAL);
-    host_clock = qemu_new_clock(QEMU_CLOCK_HOST);
-
-    rtc_clock = host_clock;
-}
-
-/* save a timer */
-void qemu_put_timer(QEMUFile *f, QEMUTimer *ts)
-{
-    uint64_t expire_time;
-
-    if (qemu_timer_pending(ts)) {
-        expire_time = ts->expire_time;
-    } else {
-        expire_time = -1;
-    }
-    qemu_put_be64(f, expire_time);
-}
-
-void qemu_get_timer(QEMUFile *f, QEMUTimer *ts)
-{
-    uint64_t expire_time;
-
-    expire_time = qemu_get_be64(f);
-    if (expire_time != -1) {
-        qemu_mod_timer(ts, expire_time);
-    } else {
-        qemu_del_timer(ts);
-    }
-}
-
-static const VMStateDescription vmstate_timers = {
-    .name = "timer",
-    .version_id = 2,
-    .minimum_version_id = 1,
-    .minimum_version_id_old = 1,
-    .fields      = (VMStateField []) {
-        VMSTATE_INT64(cpu_ticks_offset, TimersState),
-        VMSTATE_INT64(dummy, TimersState),
-        VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
-static void configure_icount(const char *option)
-{
-    vmstate_register(0, &vmstate_timers, &timers_state);
-    if (!option)
-        return;
-
-    if (strcmp(option, "auto") != 0) {
-        icount_time_shift = strtol(option, NULL, 0);
-        use_icount = 1;
-        return;
-    }
-
-    use_icount = 2;
-
-    /* 125MIPS seems a reasonable initial guess at the guest speed.
-       It will be corrected fairly quickly anyway.  */
-    icount_time_shift = 3;
-
-    /* Have both realtime and virtual time triggers for speed adjustment.
-       The realtime trigger catches emulated time passing too slowly,
-       the virtual time trigger catches emulated time passing too fast.
-       Realtime triggers occur even when idle, so use them less frequently
-       than VM triggers.  */
-    icount_rt_timer = qemu_new_timer(rt_clock, icount_adjust_rt, NULL);
-    qemu_mod_timer(icount_rt_timer,
-                   qemu_get_clock(rt_clock) + 1000);
-    icount_vm_timer = qemu_new_timer(vm_clock, icount_adjust_vm, NULL);
-    qemu_mod_timer(icount_vm_timer,
-                   qemu_get_clock(vm_clock) + get_ticks_per_sec() / 10);
-}
-
-static void qemu_timer_bh(void *opaque)
-{
-    struct qemu_alarm_timer *t = opaque;
-
-    /* rearm timer, if not periodic */
-    if (t->expired) {
-        t->expired = 0;
-        qemu_rearm_alarm_timer(t);
-    }
-
-    /* vm time timers */
-    if (vm_running) {
-        qemu_run_timers(vm_clock);
-    }
-
-    qemu_run_timers(rt_clock);
-    qemu_run_timers(host_clock);
-}
-
-#ifdef _WIN32
-static void CALLBACK host_alarm_handler(UINT uTimerID, UINT uMsg,
-                                        DWORD_PTR dwUser, DWORD_PTR dw1,
-                                        DWORD_PTR dw2)
-#else
-static void host_alarm_handler(int host_signum)
-#endif
-{
-    struct qemu_alarm_timer *t = alarm_timer;
-    if (!t)
-	return;
-
-#if 0
-#define DISP_FREQ 1000
-    {
-        static int64_t delta_min = INT64_MAX;
-        static int64_t delta_max, delta_cum, last_clock, delta, ti;
-        static int count;
-        ti = qemu_get_clock(vm_clock);
-        if (last_clock != 0) {
-            delta = ti - last_clock;
-            if (delta < delta_min)
-                delta_min = delta;
-            if (delta > delta_max)
-                delta_max = delta;
-            delta_cum += delta;
-            if (++count == DISP_FREQ) {
-                printf("timer: min=%" PRId64 " us max=%" PRId64 " us avg=%" PRId64 " us avg_freq=%0.3f Hz\n",
-                       muldiv64(delta_min, 1000000, get_ticks_per_sec()),
-                       muldiv64(delta_max, 1000000, get_ticks_per_sec()),
-                       muldiv64(delta_cum, 1000000 / DISP_FREQ, get_ticks_per_sec()),
-                       (double)get_ticks_per_sec() / ((double)delta_cum / DISP_FREQ));
-                count = 0;
-                delta_min = INT64_MAX;
-                delta_max = 0;
-                delta_cum = 0;
-            }
-        }
-        last_clock = ti;
-    }
-#endif
-    if (alarm_has_dynticks(t) ||
-        (!use_icount &&
-            qemu_timer_expired(active_timers[QEMU_CLOCK_VIRTUAL],
-                               qemu_get_clock(vm_clock))) ||
-        qemu_timer_expired(active_timers[QEMU_CLOCK_REALTIME],
-                           qemu_get_clock(rt_clock)) ||
-        qemu_timer_expired(active_timers[QEMU_CLOCK_HOST],
-                           qemu_get_clock(host_clock))) {
-        qemu_notify_event();
-        t->expired = alarm_has_dynticks(t);
-        qemu_bh_schedule(t->bh);
-    }
-}
-
-static int64_t qemu_next_deadline(void)
-{
-    /* To avoid problems with overflow limit this to 2^32.  */
-    int64_t delta = INT32_MAX;
-
-    if (active_timers[QEMU_CLOCK_VIRTUAL]) {
-        delta = active_timers[QEMU_CLOCK_VIRTUAL]->expire_time -
-                     qemu_get_clock(vm_clock);
-    }
-    if (active_timers[QEMU_CLOCK_HOST]) {
-        int64_t hdelta = active_timers[QEMU_CLOCK_HOST]->expire_time -
-                 qemu_get_clock(host_clock);
-        if (hdelta < delta)
-            delta = hdelta;
-    }
-
-    if (delta < 0)
-        delta = 0;
-
-    return delta;
-}
-
-#if defined(__linux__)
-static uint64_t qemu_next_deadline_dyntick(void)
-{
-    int64_t delta;
-    int64_t rtdelta;
-
-    if (use_icount)
-        delta = INT32_MAX;
-    else
-        delta = (qemu_next_deadline() + 999) / 1000;
-
-    if (active_timers[QEMU_CLOCK_REALTIME]) {
-        rtdelta = (active_timers[QEMU_CLOCK_REALTIME]->expire_time -
-                 qemu_get_clock(rt_clock))*1000;
-        if (rtdelta < delta)
-            delta = rtdelta;
-    }
-
-    if (delta < MIN_TIMER_REARM_US)
-        delta = MIN_TIMER_REARM_US;
-
-    return delta;
-}
-#endif
-
-#ifndef _WIN32
-
-/* Sets a specific flag */
-static int fcntl_setfl(int fd, int flag)
-{
-    int flags;
-
-    flags = fcntl(fd, F_GETFL);
-    if (flags == -1)
-        return -errno;
-
-    if (fcntl(fd, F_SETFL, flags | flag) == -1)
-        return -errno;
-
-    return 0;
-}
-
-#if defined(__linux__)
-
-#define RTC_FREQ 1024
-
-static void enable_sigio_timer(int fd)
-{
-    struct sigaction act;
-
-    /* timer signal */
-    sigfillset(&act.sa_mask);
-    act.sa_flags = 0;
-    act.sa_handler = host_alarm_handler;
-
-    sigaction(SIGIO, &act, NULL);
-    fcntl_setfl(fd, O_ASYNC);
-    fcntl(fd, F_SETOWN, getpid());
-}
-
-static int hpet_start_timer(struct qemu_alarm_timer *t)
-{
-    struct hpet_info info;
-    int r, fd;
-
-    fd = qemu_open("/dev/hpet", O_RDONLY);
-    if (fd < 0)
-        return -1;
-
-    /* Set frequency */
-    r = ioctl(fd, HPET_IRQFREQ, RTC_FREQ);
-    if (r < 0) {
-        fprintf(stderr, "Could not configure '/dev/hpet' to have a 1024Hz timer. This is not a fatal\n"
-                "error, but for better emulation accuracy type:\n"
-                "'echo 1024 > /proc/sys/dev/hpet/max-user-freq' as root.\n");
-        goto fail;
-    }
-
-    /* Check capabilities */
-    r = ioctl(fd, HPET_INFO, &info);
-    if (r < 0)
-        goto fail;
-
-    /* Enable periodic mode */
-    r = ioctl(fd, HPET_EPI, 0);
-    if (info.hi_flags && (r < 0))
-        goto fail;
-
-    /* Enable interrupt */
-    r = ioctl(fd, HPET_IE_ON, 0);
-    if (r < 0)
-        goto fail;
-
-    enable_sigio_timer(fd);
-    t->priv = (void *)(long)fd;
-
-    return 0;
-fail:
-    close(fd);
-    return -1;
-}
-
-static void hpet_stop_timer(struct qemu_alarm_timer *t)
-{
-    int fd = (long)t->priv;
-
-    close(fd);
-}
-
-static int rtc_start_timer(struct qemu_alarm_timer *t)
-{
-    int rtc_fd;
-    unsigned long current_rtc_freq = 0;
-
-    TFR(rtc_fd = qemu_open("/dev/rtc", O_RDONLY));
-    if (rtc_fd < 0)
-        return -1;
-    ioctl(rtc_fd, RTC_IRQP_READ, &current_rtc_freq);
-    if (current_rtc_freq != RTC_FREQ &&
-        ioctl(rtc_fd, RTC_IRQP_SET, RTC_FREQ) < 0) {
-        fprintf(stderr, "Could not configure '/dev/rtc' to have a 1024 Hz timer. This is not a fatal\n"
-                "error, but for better emulation accuracy either use a 2.6 host Linux kernel or\n"
-                "type 'echo 1024 > /proc/sys/dev/rtc/max-user-freq' as root.\n");
-        goto fail;
-    }
-    if (ioctl(rtc_fd, RTC_PIE_ON, 0) < 0) {
-    fail:
-        close(rtc_fd);
-        return -1;
-    }
-
-    enable_sigio_timer(rtc_fd);
-
-    t->priv = (void *)(long)rtc_fd;
-
-    return 0;
-}
-
-static void rtc_stop_timer(struct qemu_alarm_timer *t)
-{
-    int rtc_fd = (long)t->priv;
-
-    close(rtc_fd);
-}
-
-static int dynticks_start_timer(struct qemu_alarm_timer *t)
-{
-    struct sigevent ev;
-    timer_t host_timer;
-    struct sigaction act;
-
-    sigfillset(&act.sa_mask);
-    act.sa_flags = 0;
-    act.sa_handler = host_alarm_handler;
-
-    sigaction(SIGALRM, &act, NULL);
-
-    /* 
-     * Initialize ev struct to 0 to avoid valgrind complaining
-     * about uninitialized data in timer_create call
-     */
-    memset(&ev, 0, sizeof(ev));
-    ev.sigev_value.sival_int = 0;
-    ev.sigev_notify = SIGEV_SIGNAL;
-    ev.sigev_signo = SIGALRM;
-
-    if (timer_create(CLOCK_REALTIME, &ev, &host_timer)) {
-        perror("timer_create");
-
-        /* disable dynticks */
-        fprintf(stderr, "Dynamic Ticks disabled\n");
-
-        return -1;
-    }
-
-    t->priv = (void *)(long)host_timer;
-
-    return 0;
-}
-
-static void dynticks_stop_timer(struct qemu_alarm_timer *t)
-{
-    timer_t host_timer = (timer_t)(long)t->priv;
-
-    timer_delete(host_timer);
-}
-
-static void dynticks_rearm_timer(struct qemu_alarm_timer *t)
-{
-    timer_t host_timer = (timer_t)(long)t->priv;
-    struct itimerspec timeout;
-    int64_t nearest_delta_us = INT64_MAX;
-    int64_t current_us;
-
-    assert(alarm_has_dynticks(t));
-    if (!active_timers[QEMU_CLOCK_REALTIME] &&
-        !active_timers[QEMU_CLOCK_VIRTUAL] &&
-        !active_timers[QEMU_CLOCK_HOST])
-        return;
-
-    nearest_delta_us = qemu_next_deadline_dyntick();
-
-    /* check whether a timer is already running */
-    if (timer_gettime(host_timer, &timeout)) {
-        perror("gettime");
-        fprintf(stderr, "Internal timer error: aborting\n");
-        exit(1);
-    }
-    current_us = timeout.it_value.tv_sec * 1000000 + timeout.it_value.tv_nsec/1000;
-    if (current_us && current_us <= nearest_delta_us)
-        return;
-
-    timeout.it_interval.tv_sec = 0;
-    timeout.it_interval.tv_nsec = 0; /* 0 for one-shot timer */
-    timeout.it_value.tv_sec =  nearest_delta_us / 1000000;
-    timeout.it_value.tv_nsec = (nearest_delta_us % 1000000) * 1000;
-    if (timer_settime(host_timer, 0 /* RELATIVE */, &timeout, NULL)) {
-        perror("settime");
-        fprintf(stderr, "Internal timer error: aborting\n");
-        exit(1);
-    }
-}
-
-#endif /* defined(__linux__) */
-
-static int unix_start_timer(struct qemu_alarm_timer *t)
-{
-    struct sigaction act;
-    struct itimerval itv;
-    int err;
-
-    /* timer signal */
-    sigfillset(&act.sa_mask);
-    act.sa_flags = 0;
-    act.sa_handler = host_alarm_handler;
-
-    sigaction(SIGALRM, &act, NULL);
-
-    itv.it_interval.tv_sec = 0;
-    /* for i386 kernel 2.6 to get 1 ms */
-    itv.it_interval.tv_usec = 999;
-    itv.it_value.tv_sec = 0;
-    itv.it_value.tv_usec = 10 * 1000;
-
-    err = setitimer(ITIMER_REAL, &itv, NULL);
-    if (err)
-        return -1;
-
-    return 0;
-}
-
-static void unix_stop_timer(struct qemu_alarm_timer *t)
-{
-    struct itimerval itv;
-
-    memset(&itv, 0, sizeof(itv));
-    setitimer(ITIMER_REAL, &itv, NULL);
-}
-
-#endif /* !defined(_WIN32) */
-
-
-#ifdef _WIN32
-
-static int win32_start_timer(struct qemu_alarm_timer *t)
-{
-    TIMECAPS tc;
-    struct qemu_alarm_win32 *data = t->priv;
-    UINT flags;
-
-    memset(&tc, 0, sizeof(tc));
-    timeGetDevCaps(&tc, sizeof(tc));
-
-    data->period = tc.wPeriodMin;
-    timeBeginPeriod(data->period);
-
-    flags = TIME_CALLBACK_FUNCTION;
-    if (alarm_has_dynticks(t))
-        flags |= TIME_ONESHOT;
-    else
-        flags |= TIME_PERIODIC;
-
-    data->timerId = timeSetEvent(1,         // interval (ms)
-                        data->period,       // resolution
-                        host_alarm_handler, // function
-                        (DWORD)t,           // parameter
-                        flags);
-
-    if (!data->timerId) {
-        fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n",
-                GetLastError());
-        timeEndPeriod(data->period);
-        return -1;
-    }
-
-    return 0;
-}
-
-static void win32_stop_timer(struct qemu_alarm_timer *t)
-{
-    struct qemu_alarm_win32 *data = t->priv;
-
-    timeKillEvent(data->timerId);
-    timeEndPeriod(data->period);
-}
-
-static void win32_rearm_timer(struct qemu_alarm_timer *t)
-{
-    struct qemu_alarm_win32 *data = t->priv;
-
-    assert(alarm_has_dynticks(t));
-    if (!active_timers[QEMU_CLOCK_REALTIME] &&
-        !active_timers[QEMU_CLOCK_VIRTUAL] &&
-        !active_timers[QEMU_CLOCK_HOST])
-        return;
-
-    timeKillEvent(data->timerId);
-
-    data->timerId = timeSetEvent(1,
-                        data->period,
-                        host_alarm_handler,
-                        (DWORD)t,
-                        TIME_ONESHOT | TIME_CALLBACK_FUNCTION);
-
-    if (!data->timerId) {
-        fprintf(stderr, "Failed to re-arm win32 alarm timer %ld\n",
-                GetLastError());
-
-        timeEndPeriod(data->period);
-        exit(1);
-    }
-}
-
-#endif /* _WIN32 */
-
-static void alarm_timer_on_change_state_rearm(void *opaque, int running, int reason)
-{
-    if (running)
-        qemu_rearm_alarm_timer((struct qemu_alarm_timer *) opaque);
-}
-
-static int init_timer_alarm(void)
-{
-    struct qemu_alarm_timer *t = NULL;
-    int i, err = -1;
-
-    for (i = 0; alarm_timers[i].name; i++) {
-        t = &alarm_timers[i];
-
-        err = t->start(t);
-        if (!err)
-            break;
-    }
-
-    if (err) {
-        err = -ENOENT;
-        goto fail;
-    }
-
-    /* first event is at time 0 */
-    t->bh = qemu_bh_new(qemu_timer_bh, t);
-    qemu_bh_schedule(t->bh);
-    alarm_timer = t;
-    qemu_add_vm_change_state_handler(alarm_timer_on_change_state_rearm, t);
-
-    return 0;
-
-fail:
-    return err;
-}
-
-static void quit_timers(void)
-{
-    struct qemu_alarm_timer *t = alarm_timer;
-    alarm_timer = NULL;
-    t->stop(t);
-}
-
 /***********************************************************/
 /* host time/date access */
 void qemu_get_timedate(struct tm *tm, int offset)
@@ -3932,46 +2808,6 @@ static bool tcg_cpu_exec(void)
     return qemu_cpus_have_work();
 }
 
-static int qemu_calculate_timeout(void)
-{
-#ifndef CONFIG_IOTHREAD
-    int timeout;
-
-    if (!vm_running)
-        timeout = 5000;
-    else {
-     /* XXX: use timeout computed from timers */
-        int64_t add;
-        int64_t delta;
-        /* Advance virtual time to the next event.  */
-	delta = qemu_icount_delta();
-        if (delta > 0) {
-            /* If virtual time is ahead of real time then just
-               wait for IO.  */
-            timeout = (delta + 999999) / 1000000;
-        } else {
-            /* Wait for either IO to occur or the next
-               timer event.  */
-            add = qemu_next_deadline();
-            /* We advance the timer before checking for IO.
-               Limit the amount we advance so that early IO
-               activity won't get the guest too far ahead.  */
-            if (add > 10000000)
-                add = 10000000;
-            delta += add;
-            qemu_icount += qemu_icount_round (add);
-            timeout = delta / 1000000;
-            if (timeout < 0)
-                timeout = 0;
-        }
-    }
-
-    return timeout;
-#else /* CONFIG_IOTHREAD */
-    return 1000;
-#endif
-}
-
 static int vm_can_run(void)
 {
     if (powerdown_requested)
-- 
1.6.5.2

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

* Re: [Qemu-devel] [PATCH 03/19] avoid dubiously clever code in win32_start_timer
  2010-01-04 19:34   ` Anthony Liguori
@ 2010-01-04 18:39     ` Paolo Bonzini
  0 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2010-01-04 18:39 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel

On 01/04/2010 08:34 PM, Anthony Liguori wrote:
> On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
>> The code is initializing an unsigned int to UINT_MAX using "-1", so that
>> the following always-true comparison seems to be always-false at a
>> first look. Just remove the if.
>
> This really needs a better comment message as at first it wasn't obvious
> to me that this was correct.
>
> start_timer is only ever called once and as such, the check here is
> unnecessary. It's not incorrect, it's just more robust than it needs to be.
>
> I'm not sure it's really worth removing to be honest.

I didn't like the comparison with -1 being always true.  I'll update 
with a comment.

Paolo

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

* Re: [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o
  2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
                   ` (18 preceding siblings ...)
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 19/19] split out qemu-timer.c Paolo Bonzini
@ 2010-01-04 19:26 ` Anthony Liguori
  19 siblings, 0 replies; 41+ messages in thread
From: Anthony Liguori @ 2010-01-04 19:26 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> This series makes a few cleanup in the timer handling code and
> splits out ~1500 lines out of the huge vl.o file.  So far I've tested
> it by booting a live CD both under Linux and by cross-compiling to
> Windows.  If the series is considered helpful, I can test further
> including actually running the Windows version (Wine doesn't work).


Wine usually works although it's been a while since I've tested.

I haven't reviewed the series yet but I love the idea.  It's been 
something I've wanted to do for a while.  More detailed review of the 
patches will follow.

Regards,

Anthony Liguori

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

* Re: [Qemu-devel] [PATCH 03/19] avoid dubiously clever code in win32_start_timer
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 03/19] avoid dubiously clever code in win32_start_timer Paolo Bonzini
@ 2010-01-04 19:34   ` Anthony Liguori
  2010-01-04 18:39     ` Paolo Bonzini
  0 siblings, 1 reply; 41+ messages in thread
From: Anthony Liguori @ 2010-01-04 19:34 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> The code is initializing an unsigned int to UINT_MAX using "-1", so that
> the following always-true comparison seems to be always-false at a
> first look.  Just remove the if.

This really needs a better comment message as at first it wasn't obvious 
to me that this was correct.

start_timer is only ever called once and as such, the check here is 
unnecessary.  It's not incorrect, it's just more robust than it needs to be.

I'm not sure it's really worth removing to be honest.

Regards,

Anthony Liguori

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

* Re: [Qemu-devel] [PATCH 11/19] use a bottom half to run timers
  2010-01-04 20:24   ` Anthony Liguori
@ 2010-01-04 19:38     ` Jamie Lokier
  2010-01-05  8:38       ` Paolo Bonzini
  2010-01-04 20:01     ` [Qemu-devel] " Michael S. Tsirkin
  2010-01-04 20:01     ` Paolo Bonzini
  2 siblings, 1 reply; 41+ messages in thread
From: Jamie Lokier @ 2010-01-04 19:38 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Paolo Bonzini, qemu-devel

Anthony Liguori wrote:
> introduces a subtle semantic change.  Previously, timers always ran 
> before bottom halves whereas after this change, timers may run after 
> some bottoms halves but before others.  While this should be okay in 
> principle, in practice, I'm sure it'll introduce regressions.  I'd be 
> very surprised if cris wasn't affected by this.

In principle, if it does affect something, it seems likely there is
already a buggy race condition.  After all, if the timer and bottom
half could trigger at the same time, which is the condition where the
order is significant, then in principle the timer could have triggered
slightly later because it depends on the host alarm behaviour.

> But more importantly, I think timer dispatch needs to be part of the 
> select loop.  malc has a git tree that replaces host alarm timers with 
> select() timeouts.  This has a lot of really nice properties like it 
> eliminates the need for signals and EINTR handling.  A move like this 
> would likely make this more difficult.

I agree that select loop is better, but don't you still need a host
alarm signal for when the guest is running for a long time without
trapping?

-- Jamie

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-04 20:24   ` Anthony Liguori
  2010-01-04 19:38     ` Jamie Lokier
@ 2010-01-04 20:01     ` Michael S. Tsirkin
  2010-01-04 23:54       ` Anthony Liguori
  2010-01-04 20:01     ` Paolo Bonzini
  2 siblings, 1 reply; 41+ messages in thread
From: Michael S. Tsirkin @ 2010-01-04 20:01 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Paolo Bonzini, qemu-devel

On Mon, Jan 04, 2010 at 02:24:53PM -0600, Anthony Liguori wrote:
> On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
>> Make the timer subsystem register its own bottom half instead of
>> placing the bottom half code in the heart of the main loop.  To
>> test if an alarm timer is pending, just check if the bottom half is
>> scheduled.
>>
>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
>
> I'm not a huge fan of this for a couple reasons.  The first is that it  
> introduces a subtle semantic change.  Previously, timers always ran  
> before bottom halves whereas after this change, timers may run after  
> some bottoms halves but before others.  While this should be okay in  
> principle, in practice, I'm sure it'll introduce regressions.  I'd be  
> very surprised if cris wasn't affected by this.
>
> But more importantly, I think timer dispatch needs to be part of the  
> select loop.  malc has a git tree that replaces host alarm timers with  
> select() timeouts.

Where is that tree?

IMO we need that, I am not sure all code is as signal-safe
as it should be. At least crashes that I saw with winxp install
seem to be related to signal handling.

>  This has a lot of really nice properties like it  
> eliminates the need for signals and EINTR handling.  A move like this  
> would likely make this more difficult.
>
> I think the opposite sort of move makes more sense.  Treating bottom  
> halves as 0-duration timer events.  Unfortunately, things like cris do  
> not handle this sort of change very well.
>
> Regards,
>
> Anthony Liguori
>

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-04 20:24   ` Anthony Liguori
  2010-01-04 19:38     ` Jamie Lokier
  2010-01-04 20:01     ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-01-04 20:01     ` Paolo Bonzini
  2010-01-04 23:59       ` Anthony Liguori
  2 siblings, 1 reply; 41+ messages in thread
From: Paolo Bonzini @ 2010-01-04 20:01 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel

On 01/04/2010 09:24 PM, Anthony Liguori wrote:
>
> I'm not a huge fan of this for a couple reasons.  The first is that
> it introduces a subtle semantic change.  Previously, timers always
> ran before bottom halves whereas after this change, timers may run
> after some bottoms halves but before others.

I see what you mean, and you are right: qemu_bh_new adds a
bottom half at the beginning of the queue, so it's pretty much
guaranteed that a ptimer's bottom half will run _before_ the alarm timer's.

There are three possible fixes:

1) make async.c use a tail queue.  Fixes the bug, but it is too clever IMHO.

2) in tcg_exec, where there is

         if (timer_alarm_pending) {
             timer_alarm_pending = 0;
             break;
         }

instead check if any bottom half is scheduled.  With this change, after 
the timers run, if the ptimer's bottom half hadn't run TCG would not 
execute code, qemu_bh_calculate_timeout would make main_loop_wait 
nonblocking, and the ptimer's bottom half would execute right away.

BTW after my series the above check will test whether the timer bottom 
half is scheduled, so in some sense this could be considered a bugfix 
that would be placed _very early_ in the series or could even go in 
independently.

3) Both of the above.  2 would provide the fix and 1 would provide a 
performance improvement by avoiding the useless looping.

> But more importantly, I think timer dispatch needs to be part of the
> select loop.  malc has a git tree that replaces host alarm timers
> with select() timeouts.  This has a lot of really nice properties
> like it eliminates the need for signals and EINTR handling.  A move
> like this would likely make this more difficult.

Not necessarily, or at least, splitting qemu-timer.c may make it 
marginally more difficult but not having a bottom half for timers.

With qemu-timer.c split you'd need something like

    if (rc == 0) host_alarm_handler ();

after the select loop.  I suppose you could basically revert this patch 
and move timer handling into host_alarm_handler, but the code should 
work independent of this patch.  This patch (modulo your other 
objection) just adds a level of indirection but doesn't change the 
overall structure of the main loop.

Paolo

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

* Re: [Qemu-devel] [PATCH 08/19] move kbd/mouse events to event.c
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 08/19] move kbd/mouse events to event.c Paolo Bonzini
@ 2010-01-04 20:19   ` Anthony Liguori
  0 siblings, 0 replies; 41+ messages in thread
From: Anthony Liguori @ 2010-01-04 20:19 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> As a side exercise, move 200 lines out of vl.c already into common
> code that only needs to be compiled once.


Maybe input.c instead of event.c?

Please split this out of the series and resubmit.  It's a pretty obvious 
patch to include on it's own.

Regards,

Anthony Liguori

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

* Re: [Qemu-devel] [PATCH 11/19] use a bottom half to run timers
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 11/19] use a bottom half to run timers Paolo Bonzini
@ 2010-01-04 20:24   ` Anthony Liguori
  2010-01-04 19:38     ` Jamie Lokier
                       ` (2 more replies)
  0 siblings, 3 replies; 41+ messages in thread
From: Anthony Liguori @ 2010-01-04 20:24 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> Make the timer subsystem register its own bottom half instead of
> placing the bottom half code in the heart of the main loop.  To
> test if an alarm timer is pending, just check if the bottom half is
> scheduled.
>
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>

I'm not a huge fan of this for a couple reasons.  The first is that it 
introduces a subtle semantic change.  Previously, timers always ran 
before bottom halves whereas after this change, timers may run after 
some bottoms halves but before others.  While this should be okay in 
principle, in practice, I'm sure it'll introduce regressions.  I'd be 
very surprised if cris wasn't affected by this.

But more importantly, I think timer dispatch needs to be part of the 
select loop.  malc has a git tree that replaces host alarm timers with 
select() timeouts.  This has a lot of really nice properties like it 
eliminates the need for signals and EINTR handling.  A move like this 
would likely make this more difficult.

I think the opposite sort of move makes more sense.  Treating bottom 
halves as 0-duration timer events.  Unfortunately, things like cris do 
not handle this sort of change very well.

Regards,

Anthony Liguori

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

* Re: [Qemu-devel] [PATCH 19/19] split out qemu-timer.c
  2009-12-21  8:09 ` [Qemu-devel] [PATCH 19/19] split out qemu-timer.c Paolo Bonzini
@ 2010-01-04 20:26   ` Anthony Liguori
  0 siblings, 0 replies; 41+ messages in thread
From: Anthony Liguori @ 2010-01-04 20:26 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> ---
>   Makefile.target |    1 +
>   cpu-all.h       |    2 +
>   qemu-timer.c    | 1218 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   qemu-timer.h    |   11 +
>   vl.c            | 1164 ----------------------------------------------------
>   5 files changed, 1232 insertions(+), 1164 deletions(-)
>   create mode 100644 qemu-timer.c


All in all, a nice cleanup.  Good work!

Regards,

Anthony Liguori

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-04 20:01     ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-01-04 23:54       ` Anthony Liguori
  2010-01-05 12:07         ` Michael S. Tsirkin
  0 siblings, 1 reply; 41+ messages in thread
From: Anthony Liguori @ 2010-01-04 23:54 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Paolo Bonzini, qemu-devel

On 01/04/2010 02:01 PM, Michael S. Tsirkin wrote:
> On Mon, Jan 04, 2010 at 02:24:53PM -0600, Anthony Liguori wrote:
>    
>> On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
>>      
>>> Make the timer subsystem register its own bottom half instead of
>>> placing the bottom half code in the heart of the main loop.  To
>>> test if an alarm timer is pending, just check if the bottom half is
>>> scheduled.
>>>
>>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
>>>        
>> I'm not a huge fan of this for a couple reasons.  The first is that it
>> introduces a subtle semantic change.  Previously, timers always ran
>> before bottom halves whereas after this change, timers may run after
>> some bottoms halves but before others.  While this should be okay in
>> principle, in practice, I'm sure it'll introduce regressions.  I'd be
>> very surprised if cris wasn't affected by this.
>>
>> But more importantly, I think timer dispatch needs to be part of the
>> select loop.  malc has a git tree that replaces host alarm timers with
>> select() timeouts.
>>      
> Where is that tree?
>    

http://repo.or.cz/w/qemu/malc.git  mtloop

> IMO we need that, I am not sure all code is as signal-safe
> as it should be. At least crashes that I saw with winxp install
> seem to be related to signal handling.
>    

Regards,

Anthony Liguori

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-04 20:01     ` Paolo Bonzini
@ 2010-01-04 23:59       ` Anthony Liguori
  2010-01-05 12:48         ` Paolo Bonzini
  0 siblings, 1 reply; 41+ messages in thread
From: Anthony Liguori @ 2010-01-04 23:59 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 01/04/2010 02:01 PM, Paolo Bonzini wrote:
> On 01/04/2010 09:24 PM, Anthony Liguori wrote:
>>
>> I'm not a huge fan of this for a couple reasons.  The first is that
>> it introduces a subtle semantic change.  Previously, timers always
>> ran before bottom halves whereas after this change, timers may run
>> after some bottoms halves but before others.
>
> I see what you mean, and you are right: qemu_bh_new adds a
> bottom half at the beginning of the queue, so it's pretty much
> guaranteed that a ptimer's bottom half will run _before_ the alarm 
> timer's.
>
> There are three possible fixes:
>
> 1) make async.c use a tail queue.  Fixes the bug, but it is too clever 
> IMHO.
>
> 2) in tcg_exec, where there is
>
>         if (timer_alarm_pending) {
>             timer_alarm_pending = 0;
>             break;
>         }
>
> instead check if any bottom half is scheduled.  With this change, 
> after the timers run, if the ptimer's bottom half hadn't run TCG would 
> not execute code, qemu_bh_calculate_timeout would make main_loop_wait 
> nonblocking, and the ptimer's bottom half would execute right away.
>
> BTW after my series the above check will test whether the timer bottom 
> half is scheduled, so in some sense this could be considered a bugfix 
> that would be placed _very early_ in the series or could even go in 
> independently.
>
> 3) Both of the above.  2 would provide the fix and 1 would provide a 
> performance improvement by avoiding the useless looping.
>
>> But more importantly, I think timer dispatch needs to be part of the
>> select loop.  malc has a git tree that replaces host alarm timers
>> with select() timeouts.  This has a lot of really nice properties
>> like it eliminates the need for signals and EINTR handling.  A move
>> like this would likely make this more difficult.
>
> Not necessarily, or at least, splitting qemu-timer.c may make it 
> marginally more difficult but not having a bottom half for timers.

When I think of a main loop, I think of something that provides the 
following three services

1) fd based events
2) time based events
3) an idle callback

The problem we have is that bottom halves aren't quite idle callbacks.  
They have some really weird properties about the guarantees of when 
they're executed.  At any rate, you really cannot express a time based 
event as an idle callback because you need to setup the select() timeout 
based on the next deadline and then dispatch timer event based on 
selects return.  idle functions have none of this ability.

> With qemu-timer.c split you'd need something like
>
>    if (rc == 0) host_alarm_handler ();
>
> after the select loop.  I suppose you could basically revert this 
> patch and move timer handling into host_alarm_handler, but the code 
> should work independent of this patch.  This patch (modulo your other 
> objection) just adds a level of indirection but doesn't change the 
> overall structure of the main loop.

Yeah, I'm not at all opposed to the qemu-timer.c split.  What I would 
suggest is just to continue calling timers explicitly instead of trying 
to make them bottoms halves.  I've played with this in the past and the 
regressions are really nasty to track down.  If we're going to make such 
a change, I'd rather spend that regression-busting effort on mtloop or 
completely changing the bottom half semantics to be idle callbacks.

Regards,

Anthony Liguori

> Paolo

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

* Re: [Qemu-devel] [PATCH 11/19] use a bottom half to run timers
  2010-01-04 19:38     ` Jamie Lokier
@ 2010-01-05  8:38       ` Paolo Bonzini
  0 siblings, 0 replies; 41+ messages in thread
From: Paolo Bonzini @ 2010-01-05  8:38 UTC (permalink / raw)
  To: Jamie Lokier; +Cc: qemu-devel

On 01/04/2010 08:38 PM, Jamie Lokier wrote:
> In principle, if it does affect something, it seems likely there is
> already a buggy race condition.  After all, if the timer and bottom
> half could trigger at the same time, which is the condition where the
> order is significant, then in principle the timer could have triggered
> slightly later because it depends on the host alarm behaviour.

No, the problem is when the timer function is _itself_ scheduling a 
bottom half.  Before my patch there was a guarantee that the bh would 
run before TCG, now there is not.  It can be fixed easily though.

Paolo

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-04 23:54       ` Anthony Liguori
@ 2010-01-05 12:07         ` Michael S. Tsirkin
  2010-01-05 15:23           ` malc
  0 siblings, 1 reply; 41+ messages in thread
From: Michael S. Tsirkin @ 2010-01-05 12:07 UTC (permalink / raw)
  To: Anthony Liguori, av1474; +Cc: Paolo Bonzini, qemu-devel

On Mon, Jan 04, 2010 at 05:54:13PM -0600, Anthony Liguori wrote:
> On 01/04/2010 02:01 PM, Michael S. Tsirkin wrote:
>> On Mon, Jan 04, 2010 at 02:24:53PM -0600, Anthony Liguori wrote:
>>    
>>> On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
>>>      
>>>> Make the timer subsystem register its own bottom half instead of
>>>> placing the bottom half code in the heart of the main loop.  To
>>>> test if an alarm timer is pending, just check if the bottom half is
>>>> scheduled.
>>>>
>>>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
>>>>        
>>> I'm not a huge fan of this for a couple reasons.  The first is that it
>>> introduces a subtle semantic change.  Previously, timers always ran
>>> before bottom halves whereas after this change, timers may run after
>>> some bottoms halves but before others.  While this should be okay in
>>> principle, in practice, I'm sure it'll introduce regressions.  I'd be
>>> very surprised if cris wasn't affected by this.
>>>
>>> But more importantly, I think timer dispatch needs to be part of the
>>> select loop.  malc has a git tree that replaces host alarm timers with
>>> select() timeouts.
>>>      
>> Where is that tree?
>>    
>
> http://repo.or.cz/w/qemu/malc.git  mtloop

Don't seem to see anything there.
malc?

>> IMO we need that, I am not sure all code is as signal-safe
>> as it should be. At least crashes that I saw with winxp install
>> seem to be related to signal handling.
>>    
>
> Regards,
>
> Anthony Liguori

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-04 23:59       ` Anthony Liguori
@ 2010-01-05 12:48         ` Paolo Bonzini
  2010-01-05 13:06           ` Anthony Liguori
  0 siblings, 1 reply; 41+ messages in thread
From: Paolo Bonzini @ 2010-01-05 12:48 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: qemu-devel

On 01/05/2010 12:59 AM, Anthony Liguori wrote:
>
> When I think of a main loop, I think of something that provides the
> following three services
>
> 1) fd based events
> 2) time based events
> 3) an idle callback
>
> The problem we have is that bottom halves aren't quite idle callbacks.
> They have some really weird properties about the guarantees of when
> they're executed.  At any rate, you really cannot express a time based
> event as an idle callback because you need to setup the select() timeout
> based on the next deadline and then dispatch timer event based on
> selects return.  idle functions have none of this ability.

I see bottom halves as a fourth service, providing the ability to 
synchronize stuff that needs to execute in a particular thread 
(guaranteeing thread-safety and especially signal-safety).

In some sense, the problem is that bottom halves conflate this service 
_and_ idle callbacks, and that is the weird property.  Idle callbacks 
are used basically only for DMA, and real-world DMA does not have at all 
the semantics that qemu_bh_schedule_idle provides (which in turn is very 
tricky and not exactly what comments in qemu_bh_update_timeout promise).

Compared to qemu_bh_schedule_idle, bottom halves have sane semantics. 
It would be nicer IMO to remove qemu_bh_schedule_idle, have first-class 
idle callbacks, and leave bottom halves as they are now.  I'll put that 
on the todo list. :-)

That said, I'll try to replace this patch with one that doesn't use a 
bottom half.

Paolo

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-05 12:48         ` Paolo Bonzini
@ 2010-01-05 13:06           ` Anthony Liguori
  2010-01-06  1:20             ` Jamie Lokier
  0 siblings, 1 reply; 41+ messages in thread
From: Anthony Liguori @ 2010-01-05 13:06 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: qemu-devel

On 01/05/2010 06:48 AM, Paolo Bonzini wrote:
> On 01/05/2010 12:59 AM, Anthony Liguori wrote:
>>
>> When I think of a main loop, I think of something that provides the
>> following three services
>>
>> 1) fd based events
>> 2) time based events
>> 3) an idle callback
>>
>> The problem we have is that bottom halves aren't quite idle callbacks.
>> They have some really weird properties about the guarantees of when
>> they're executed.  At any rate, you really cannot express a time based
>> event as an idle callback because you need to setup the select() timeout
>> based on the next deadline and then dispatch timer event based on
>> selects return.  idle functions have none of this ability.
>
> I see bottom halves as a fourth service, providing the ability to 
> synchronize stuff that needs to execute in a particular thread 
> (guaranteeing thread-safety and especially signal-safety).

Thread and signal safety are slightly different.  Scheduling an idle 
callback from a signal handler is a pretty reasonable thing to do.

Executing no a different thread is a whole different matter.  Right now, 
we really use bottom halves to run something on the io thread and have a 
different mechanism that ran on a specific thread.  Right now, we mix 
that between bottom halves and on_vcpu.

> In some sense, the problem is that bottom halves conflate this service 
> _and_ idle callbacks, and that is the weird property.  Idle callbacks 
> are used basically only for DMA, and real-world DMA does not have at 
> all the semantics that qemu_bh_schedule_idle provides (which in turn 
> is very tricky and not exactly what comments in qemu_bh_update_timeout 
> promise).

I think before we can do any major surgery, we need to redo the way that 
legacy DMA is handled so that it doesn't require constant polling as it 
does now.

Regards,

Anthony Liguori

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-05 12:07         ` Michael S. Tsirkin
@ 2010-01-05 15:23           ` malc
  2010-01-05 15:23             ` Michael S. Tsirkin
  0 siblings, 1 reply; 41+ messages in thread
From: malc @ 2010-01-05 15:23 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Paolo Bonzini, qemu-devel

On Tue, 5 Jan 2010, Michael S. Tsirkin wrote:

> On Mon, Jan 04, 2010 at 05:54:13PM -0600, Anthony Liguori wrote:
> > On 01/04/2010 02:01 PM, Michael S. Tsirkin wrote:
> >> On Mon, Jan 04, 2010 at 02:24:53PM -0600, Anthony Liguori wrote:
> >>    
> >>> On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> >>>      
> >>>> Make the timer subsystem register its own bottom half instead of
> >>>> placing the bottom half code in the heart of the main loop.  To
> >>>> test if an alarm timer is pending, just check if the bottom half is
> >>>> scheduled.
> >>>>
> >>>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> >>>>        
> >>> I'm not a huge fan of this for a couple reasons.  The first is that it
> >>> introduces a subtle semantic change.  Previously, timers always ran
> >>> before bottom halves whereas after this change, timers may run after
> >>> some bottoms halves but before others.  While this should be okay in
> >>> principle, in practice, I'm sure it'll introduce regressions.  I'd be
> >>> very surprised if cris wasn't affected by this.
> >>>
> >>> But more importantly, I think timer dispatch needs to be part of the
> >>> select loop.  malc has a git tree that replaces host alarm timers with
> >>> select() timeouts.
> >>>      
> >> Where is that tree?
> >>    
> >
> > http://repo.or.cz/w/qemu/malc.git  mtloop
> 
> Don't seem to see anything there.
> malc?

Yes?
 
> >> IMO we need that, I am not sure all code is as signal-safe
> >> as it should be. At least crashes that I saw with winxp install
> >> seem to be related to signal handling.
> >>    
> >
> > Regards,
> >
> > Anthony Liguori
> 

-- 
mailto:av1474@comtv.ru

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-05 15:23           ` malc
@ 2010-01-05 15:23             ` Michael S. Tsirkin
  2010-01-05 15:32               ` malc
  0 siblings, 1 reply; 41+ messages in thread
From: Michael S. Tsirkin @ 2010-01-05 15:23 UTC (permalink / raw)
  To: malc; +Cc: Paolo Bonzini, qemu-devel

On Tue, Jan 05, 2010 at 06:23:34PM +0300, malc wrote:
> On Tue, 5 Jan 2010, Michael S. Tsirkin wrote:
> 
> > On Mon, Jan 04, 2010 at 05:54:13PM -0600, Anthony Liguori wrote:
> > > On 01/04/2010 02:01 PM, Michael S. Tsirkin wrote:
> > >> On Mon, Jan 04, 2010 at 02:24:53PM -0600, Anthony Liguori wrote:
> > >>    
> > >>> On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> > >>>      
> > >>>> Make the timer subsystem register its own bottom half instead of
> > >>>> placing the bottom half code in the heart of the main loop.  To
> > >>>> test if an alarm timer is pending, just check if the bottom half is
> > >>>> scheduled.
> > >>>>
> > >>>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> > >>>>        
> > >>> I'm not a huge fan of this for a couple reasons.  The first is that it
> > >>> introduces a subtle semantic change.  Previously, timers always ran
> > >>> before bottom halves whereas after this change, timers may run after
> > >>> some bottoms halves but before others.  While this should be okay in
> > >>> principle, in practice, I'm sure it'll introduce regressions.  I'd be
> > >>> very surprised if cris wasn't affected by this.
> > >>>
> > >>> But more importantly, I think timer dispatch needs to be part of the
> > >>> select loop.  malc has a git tree that replaces host alarm timers with
> > >>> select() timeouts.
> > >>>      
> > >> Where is that tree?
> > >>    
> > >
> > > http://repo.or.cz/w/qemu/malc.git  mtloop
> > 
> > Don't seem to see anything there.
> > malc?
> 
> Yes?

Do you have a patch to switch from signals to select?  If yes could you
tell me where it is so  I can test whether it fixes winxp install
crashes I see?


> > >> IMO we need that, I am not sure all code is as signal-safe
> > >> as it should be. At least crashes that I saw with winxp install
> > >> seem to be related to signal handling.
> > >>    
> > >
> > > Regards,
> > >
> > > Anthony Liguori
> > 
> 
> -- 
> mailto:av1474@comtv.ru

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-05 15:23             ` Michael S. Tsirkin
@ 2010-01-05 15:32               ` malc
  2010-01-05 15:33                 ` Michael S. Tsirkin
  0 siblings, 1 reply; 41+ messages in thread
From: malc @ 2010-01-05 15:32 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Paolo Bonzini, qemu-devel

On Tue, 5 Jan 2010, Michael S. Tsirkin wrote:

> On Tue, Jan 05, 2010 at 06:23:34PM +0300, malc wrote:
> > On Tue, 5 Jan 2010, Michael S. Tsirkin wrote:
> > 
> > > On Mon, Jan 04, 2010 at 05:54:13PM -0600, Anthony Liguori wrote:
> > > > On 01/04/2010 02:01 PM, Michael S. Tsirkin wrote:
> > > >> On Mon, Jan 04, 2010 at 02:24:53PM -0600, Anthony Liguori wrote:
> > > >>    
> > > >>> On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> > > >>>      
> > > >>>> Make the timer subsystem register its own bottom half instead of
> > > >>>> placing the bottom half code in the heart of the main loop.  To
> > > >>>> test if an alarm timer is pending, just check if the bottom half is
> > > >>>> scheduled.
> > > >>>>
> > > >>>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> > > >>>>        
> > > >>> I'm not a huge fan of this for a couple reasons.  The first is that it
> > > >>> introduces a subtle semantic change.  Previously, timers always ran
> > > >>> before bottom halves whereas after this change, timers may run after
> > > >>> some bottoms halves but before others.  While this should be okay in
> > > >>> principle, in practice, I'm sure it'll introduce regressions.  I'd be
> > > >>> very surprised if cris wasn't affected by this.
> > > >>>
> > > >>> But more importantly, I think timer dispatch needs to be part of the
> > > >>> select loop.  malc has a git tree that replaces host alarm timers with
> > > >>> select() timeouts.
> > > >>>      
> > > >> Where is that tree?
> > > >>    
> > > >
> > > > http://repo.or.cz/w/qemu/malc.git  mtloop
> > > 
> > > Don't seem to see anything there.
> > > malc?
> > 
> > Yes?
> 
> Do you have a patch to switch from signals to select?  If yes could you
> tell me where it is so  I can test whether it fixes winxp install
> crashes I see?

All i have is published at:
http://repo.or.cz/w/qemu/malc.git/shortlog/refs/heads/mtloop

Specifically:
http://repo.or.cz/w/qemu/malc.git/commit/b99fef4dc2f1ce9d436791b6821cb30e6335ee9b

A small fix is needed to make it run with KVM though.

> 
> > > >> IMO we need that, I am not sure all code is as signal-safe
> > > >> as it should be. At least crashes that I saw with winxp install
> > > >> seem to be related to signal handling.
> > > >>    
> > > >
> > > > Regards,
> > > >
> > > > Anthony Liguori
> > > 
> > 
> > -- 
> > mailto:av1474@comtv.ru
> 

-- 
mailto:av1474@comtv.ru

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-05 15:32               ` malc
@ 2010-01-05 15:33                 ` Michael S. Tsirkin
  2010-01-05 15:39                   ` malc
  0 siblings, 1 reply; 41+ messages in thread
From: Michael S. Tsirkin @ 2010-01-05 15:33 UTC (permalink / raw)
  To: malc; +Cc: Paolo Bonzini, qemu-devel

On Tue, Jan 05, 2010 at 06:32:09PM +0300, malc wrote:
> On Tue, 5 Jan 2010, Michael S. Tsirkin wrote:
> 
> > On Tue, Jan 05, 2010 at 06:23:34PM +0300, malc wrote:
> > > On Tue, 5 Jan 2010, Michael S. Tsirkin wrote:
> > > 
> > > > On Mon, Jan 04, 2010 at 05:54:13PM -0600, Anthony Liguori wrote:
> > > > > On 01/04/2010 02:01 PM, Michael S. Tsirkin wrote:
> > > > >> On Mon, Jan 04, 2010 at 02:24:53PM -0600, Anthony Liguori wrote:
> > > > >>    
> > > > >>> On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> > > > >>>      
> > > > >>>> Make the timer subsystem register its own bottom half instead of
> > > > >>>> placing the bottom half code in the heart of the main loop.  To
> > > > >>>> test if an alarm timer is pending, just check if the bottom half is
> > > > >>>> scheduled.
> > > > >>>>
> > > > >>>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> > > > >>>>        
> > > > >>> I'm not a huge fan of this for a couple reasons.  The first is that it
> > > > >>> introduces a subtle semantic change.  Previously, timers always ran
> > > > >>> before bottom halves whereas after this change, timers may run after
> > > > >>> some bottoms halves but before others.  While this should be okay in
> > > > >>> principle, in practice, I'm sure it'll introduce regressions.  I'd be
> > > > >>> very surprised if cris wasn't affected by this.
> > > > >>>
> > > > >>> But more importantly, I think timer dispatch needs to be part of the
> > > > >>> select loop.  malc has a git tree that replaces host alarm timers with
> > > > >>> select() timeouts.
> > > > >>>      
> > > > >> Where is that tree?
> > > > >>    
> > > > >
> > > > > http://repo.or.cz/w/qemu/malc.git  mtloop
> > > > 
> > > > Don't seem to see anything there.
> > > > malc?
> > > 
> > > Yes?
> > 
> > Do you have a patch to switch from signals to select?  If yes could you
> > tell me where it is so  I can test whether it fixes winxp install
> > crashes I see?
> 
> All i have is published at:
> http://repo.or.cz/w/qemu/malc.git/shortlog/refs/heads/mtloop
> 
> Specifically:
> http://repo.or.cz/w/qemu/malc.git/commit/b99fef4dc2f1ce9d436791b6821cb30e6335ee9b
> 
> A small fix is needed to make it run with KVM though.

Hmm, I guess at least iothread needs to be put back for KVM?

-- 
MST

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

* [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-05 15:33                 ` Michael S. Tsirkin
@ 2010-01-05 15:39                   ` malc
  0 siblings, 0 replies; 41+ messages in thread
From: malc @ 2010-01-05 15:39 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Paolo Bonzini, qemu-devel

On Tue, 5 Jan 2010, Michael S. Tsirkin wrote:

> On Tue, Jan 05, 2010 at 06:32:09PM +0300, malc wrote:
> > On Tue, 5 Jan 2010, Michael S. Tsirkin wrote:
> > 
> > > On Tue, Jan 05, 2010 at 06:23:34PM +0300, malc wrote:
> > > > On Tue, 5 Jan 2010, Michael S. Tsirkin wrote:
> > > > 
> > > > > On Mon, Jan 04, 2010 at 05:54:13PM -0600, Anthony Liguori wrote:
> > > > > > On 01/04/2010 02:01 PM, Michael S. Tsirkin wrote:
> > > > > >> On Mon, Jan 04, 2010 at 02:24:53PM -0600, Anthony Liguori wrote:
> > > > > >>    
> > > > > >>> On 12/21/2009 02:09 AM, Paolo Bonzini wrote:
> > > > > >>>      
> > > > > >>>> Make the timer subsystem register its own bottom half instead of
> > > > > >>>> placing the bottom half code in the heart of the main loop.  To
> > > > > >>>> test if an alarm timer is pending, just check if the bottom half is
> > > > > >>>> scheduled.
> > > > > >>>>
> > > > > >>>> Signed-off-by: Paolo Bonzini<pbonzini@redhat.com>
> > > > > >>>>        
> > > > > >>> I'm not a huge fan of this for a couple reasons.  The first is that it
> > > > > >>> introduces a subtle semantic change.  Previously, timers always ran
> > > > > >>> before bottom halves whereas after this change, timers may run after
> > > > > >>> some bottoms halves but before others.  While this should be okay in
> > > > > >>> principle, in practice, I'm sure it'll introduce regressions.  I'd be
> > > > > >>> very surprised if cris wasn't affected by this.
> > > > > >>>
> > > > > >>> But more importantly, I think timer dispatch needs to be part of the
> > > > > >>> select loop.  malc has a git tree that replaces host alarm timers with
> > > > > >>> select() timeouts.
> > > > > >>>      
> > > > > >> Where is that tree?
> > > > > >>    
> > > > > >
> > > > > > http://repo.or.cz/w/qemu/malc.git  mtloop
> > > > > 
> > > > > Don't seem to see anything there.
> > > > > malc?
> > > > 
> > > > Yes?
> > > 
> > > Do you have a patch to switch from signals to select?  If yes could you
> > > tell me where it is so  I can test whether it fixes winxp install
> > > crashes I see?
> > 
> > All i have is published at:
> > http://repo.or.cz/w/qemu/malc.git/shortlog/refs/heads/mtloop
> > 
> > Specifically:
> > http://repo.or.cz/w/qemu/malc.git/commit/b99fef4dc2f1ce9d436791b6821cb30e6335ee9b
> > 
> > A small fix is needed to make it run with KVM though.
> 
> Hmm, I guess at least iothread needs to be put back for KVM?

Nope. One of the sem_waits can actually be interrupted and shouldn't
cause an untimely death of the process.

-- 
mailto:av1474@comtv.ru

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

* Re: [Qemu-devel] Re: [PATCH 11/19] use a bottom half to run timers
  2010-01-05 13:06           ` Anthony Liguori
@ 2010-01-06  1:20             ` Jamie Lokier
  0 siblings, 0 replies; 41+ messages in thread
From: Jamie Lokier @ 2010-01-06  1:20 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: Paolo Bonzini, qemu-devel

Anthony Liguori wrote:
> Thread and signal safety are slightly different.

They are very different:

Virtually all libc calls are thread safe, unless they use unsafe
static data APIs.

On the other hand, the number of libc calls that are signal safe is
very limited.

For example, calling printf() is not signal-safe; neither is malloc().

pthread functions are not safe in signal handlers either:
pthread_self, pthread_getspecific, pthread_mutex_lock and
pthread_cond_broadcast, not of them are signal-safe.

In a nutshell, it's dubious to do much inside a signal handler.

Pure system calls tend to be ok, though.

> Scheduling an idle 
> callback from a signal handler is a pretty reasonable thing to do.

Scheduling, yes, by telling the main event loop that it's time.

Running it inside the signal handler... Then you're depending on
non-portabilities among hosts.

-- Jamie

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

end of thread, other threads:[~2010-01-06  1:20 UTC | newest]

Thread overview: 41+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-21  8:09 [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 01/19] centralize handling of -icount Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 02/19] add qemu_icount_round Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 03/19] avoid dubiously clever code in win32_start_timer Paolo Bonzini
2010-01-04 19:34   ` Anthony Liguori
2010-01-04 18:39     ` Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 04/19] fix error in win32_rearm_timer Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 05/19] only one flag is needed for alarm_timer Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 06/19] more alarm timer cleanup Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 07/19] add qemu_get_clock_ns Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 08/19] move kbd/mouse events to event.c Paolo Bonzini
2010-01-04 20:19   ` Anthony Liguori
2009-12-21  8:09 ` [Qemu-devel] [PATCH 09/19] remove qemu_rearm_alarm_timer from main loop Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 10/19] add qemu_bh_scheduled Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 11/19] use a bottom half to run timers Paolo Bonzini
2010-01-04 20:24   ` Anthony Liguori
2010-01-04 19:38     ` Jamie Lokier
2010-01-05  8:38       ` Paolo Bonzini
2010-01-04 20:01     ` [Qemu-devel] " Michael S. Tsirkin
2010-01-04 23:54       ` Anthony Liguori
2010-01-05 12:07         ` Michael S. Tsirkin
2010-01-05 15:23           ` malc
2010-01-05 15:23             ` Michael S. Tsirkin
2010-01-05 15:32               ` malc
2010-01-05 15:33                 ` Michael S. Tsirkin
2010-01-05 15:39                   ` malc
2010-01-04 20:01     ` Paolo Bonzini
2010-01-04 23:59       ` Anthony Liguori
2010-01-05 12:48         ` Paolo Bonzini
2010-01-05 13:06           ` Anthony Liguori
2010-01-06  1:20             ` Jamie Lokier
2009-12-21  8:09 ` [Qemu-devel] [PATCH 12/19] new function qemu_icount_delta Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 13/19] move tcg_has_work to cpu-exec.c and rename it Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 14/19] disentangle tcg and deadline calculation Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 15/19] do not provide qemu_event_increment if iothread not used Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 16/19] tweak qemu_notify_event Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 17/19] move vmstate registration of vmstate_timers earlier Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 18/19] introduce qemu_clock_enable Paolo Bonzini
2009-12-21  8:09 ` [Qemu-devel] [PATCH 19/19] split out qemu-timer.c Paolo Bonzini
2010-01-04 20:26   ` Anthony Liguori
2010-01-04 19:26 ` [Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o Anthony Liguori

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