[PATCH v6 0/5] prevent QEMU from waking up needlessly

[PATCH v6 0/5] prevent QEMU from waking up needlessly

[Stefano Stabellini]

Hi all,
this small patch series prevents QEMU from waking up needlessly on Xen
several times a second in order to check some timers.



The first patch stops QEMU from emulating the PIT on Xen, the second
patch disables the rtc_clock entirely.

The third patch fixes win32_rearm_timer and mm_rearm_timer that
risk an overflow if INT64_MAX is passed as delta.

The fourth patch changes qemu_next_alarm_deadline only to check the
expire time of a clock if it is enabled.

Finally the last patch makes main_loop_wait wait indefinitely on select
unless we actually need a timeout.



Changes in v6:

- rebase on 7672725d41d1a04195affc1a7bd5676ba6314b14;

- dropped the buffered IO patch, that has been handled separately from
this series.


Changes in v5:

- remove the last patch to increase the timeout to 1h, replace it with a
patch to wait indefinitely on select unless we need a timeout.


Changes in v4:

- do not initialize pcspk on xen;

- disable rtc_clock only when it points to the host_clock (the default);

- make sure it compiles on older xen versions.


Changes in v3:

- added a new patch to fix win32_rearm_timer and mm_rearm_timer, that
risk an overflow if INT64_MAX is passed as delta.



Shortlog and diffstat follow:

Stefano Stabellini (5):
      xen: do not initialize the interval timer and PCSPK emulator
      xen: disable rtc_clock
      timers: the rearm function should be able to handle delta = INT64_MAX
      qemu_next_alarm_deadline: check the expire time of a clock only if it is enabled
      main_loop_wait: block indefinitely

 async.c          |    2 +-
 hw/pc.c          |    9 ++++++---
 main-loop.c      |   23 ++++++++++++++---------
 main-loop.h      |    2 +-
 qemu-timer.c     |   33 +++++++++++++++++----------------
 qemu-timer.h     |    1 -
 qemu-tool.c      |    4 ++++
 slirp/libslirp.h |    1 +
 slirp/slirp.c    |    7 +++++++
 xen-all.c        |    4 ++++
 10 files changed, 55 insertions(+), 31 deletions(-)


A git tree, based on 7672725d41d1a04195affc1a7bd5676ba6314b14, is available here:

git://xenbits.xen.org/people/sstabellini/qemu-dm.git timers-6

Cheers,

Stefano

[PATCH v6 1/5] xen: do not initialize the interval timer and PCSPK emulator

[Stefano Stabellini]

PIT and PCSPK are emulated by the hypervisor so we don't need to emulate
them in Qemu: this patch prevents Qemu from waking up needlessly at
PIT_FREQ on Xen.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 hw/pc.c |    9 ++++++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 67f0479..08c69e9 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -46,6 +46,7 @@
 #include "ui/qemu-spice.h"
 #include "memory.h"
 #include "exec-memory.h"
+#include "arch_init.h"
 
 /* output Bochs bios info messages */
 //#define DEBUG_BIOS
@@ -1089,7 +1090,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
     qemu_irq pit_alt_irq = NULL;
     qemu_irq rtc_irq = NULL;
     qemu_irq *a20_line;
-    ISADevice *i8042, *port92, *vmmouse, *pit;
+    ISADevice *i8042, *port92, *vmmouse, *pit = NULL;
     qemu_irq *cpu_exit_irq;
 
     register_ioport_write(0x80, 1, 1, ioport80_write, NULL);
@@ -1120,14 +1121,16 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi,
 
     if (kvm_irqchip_in_kernel()) {
         pit = kvm_pit_init(isa_bus, 0x40);
-    } else {
+    } else if (!xen_available()) {
         pit = pit_init(isa_bus, 0x40, pit_isa_irq, pit_alt_irq);
     }
     if (hpet) {
         /* connect PIT to output control line of the HPET */
         qdev_connect_gpio_out(hpet, 0, qdev_get_gpio_in(&pit->qdev, 0));
     }
-    pcspk_init(isa_bus, pit);
+    if (!xen_available()) {
+        pcspk_init(isa_bus, pit);
+    }
 
     for(i = 0; i < MAX_SERIAL_PORTS; i++) {
         if (serial_hds[i]) {
-- 
1.7.2.5

[PATCH v6 2/5] xen: disable rtc_clock

[Stefano Stabellini]

rtc_clock is only used by the RTC emulator (mc146818rtc.c), however Xen
has its own RTC emulator in the hypervisor so we can disable it.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 xen-all.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/xen-all.c b/xen-all.c
index 3e6de41..d1c4e72 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -593,6 +593,10 @@ void xen_vcpu_init(void)
         qemu_register_reset(xen_reset_vcpu, first_cpu);
         xen_reset_vcpu(first_cpu);
     }
+    /* if rtc_clock is left to default (host_clock), disable it */
+    if (rtc_clock == host_clock) {
+        qemu_clock_enable(rtc_clock, false);
+    }
 }
 
 /* get the ioreq packets from share mem */
-- 
1.7.2.5

[PATCH v6 3/5] timers: the rearm function should be able to handle delta = INT64_MAX

[Stefano Stabellini]

Fix win32_rearm_timer and mm_rearm_timer: they should be able to handle
INT64_MAX as a delta parameter without overflowing.
Also, the next deadline in ms should be calculated rounding down rather
than up (see unix_rearm_timer and dynticks_rearm_timer).

Finally ChangeTimerQueueTimer takes an unsigned long and timeSetEvent
takes an unsigned int as delta, so cast the ms delta to the appropriate
unsigned integer.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 qemu-timer.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 80bcc56..fe4cd6f 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -696,13 +696,17 @@ static void mm_stop_timer(struct qemu_alarm_timer *t)
 
 static void mm_rearm_timer(struct qemu_alarm_timer *t, int64_t delta)
 {
-    int nearest_delta_ms = (delta + 999999) / 1000000;
+    int64_t nearest_delta_ms = delta / 1000000;
     if (nearest_delta_ms < 1) {
         nearest_delta_ms = 1;
     }
+    /* UINT_MAX can be 32 bit */
+    if (nearest_delta_ms > UINT_MAX) {
+        nearest_delta_ms = UINT_MAX;
+    }
 
     timeKillEvent(mm_timer);
-    mm_timer = timeSetEvent(nearest_delta_ms,
+    mm_timer = timeSetEvent((unsigned int) nearest_delta_ms,
                             mm_period,
                             mm_alarm_handler,
                             (DWORD_PTR)t,
@@ -757,16 +761,20 @@ static void win32_rearm_timer(struct qemu_alarm_timer *t,
                               int64_t nearest_delta_ns)
 {
     HANDLE hTimer = t->timer;
-    int nearest_delta_ms;
+    int64_t nearest_delta_ms;
     BOOLEAN success;
 
-    nearest_delta_ms = (nearest_delta_ns + 999999) / 1000000;
+    nearest_delta_ms = nearest_delta_ns / 1000000;
     if (nearest_delta_ms < 1) {
         nearest_delta_ms = 1;
     }
+    /* ULONG_MAX can be 32 bit */
+    if (nearest_delta_ms > ULONG_MAX) {
+        nearest_delta_ms = ULONG_MAX;
+    }
     success = ChangeTimerQueueTimer(NULL,
                                     hTimer,
-                                    nearest_delta_ms,
+                                    (unsigned long) nearest_delta_ms,
                                     3600000);
 
     if (!success) {
-- 
1.7.2.5

[PATCH v6 4/5] qemu_next_alarm_deadline: check the expire time of a clock only if it is enabled

[Stefano Stabellini]

Also delta in qemu_next_alarm_deadline is a 64 bit value so set the
default to INT64_MAX instead of INT32_MAX.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 qemu-timer.c |   10 ++++------
 1 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index fe4cd6f..1166beb 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -106,23 +106,21 @@ static inline int alarm_has_dynticks(struct qemu_alarm_timer *t)
 
 static int64_t qemu_next_alarm_deadline(void)
 {
-    int64_t delta;
+    int64_t delta = INT64_MAX;
     int64_t rtdelta;
 
-    if (!use_icount && vm_clock->active_timers) {
+    if (!use_icount && vm_clock->enabled && vm_clock->active_timers) {
         delta = vm_clock->active_timers->expire_time -
                      qemu_get_clock_ns(vm_clock);
-    } else {
-        delta = INT32_MAX;
     }
-    if (host_clock->active_timers) {
+    if (host_clock->enabled && host_clock->active_timers) {
         int64_t hdelta = host_clock->active_timers->expire_time -
                  qemu_get_clock_ns(host_clock);
         if (hdelta < delta) {
             delta = hdelta;
         }
     }
-    if (rt_clock->active_timers) {
+    if (rt_clock->enabled && rt_clock->active_timers) {
         rtdelta = (rt_clock->active_timers->expire_time -
                  qemu_get_clock_ns(rt_clock));
         if (rtdelta < delta) {
-- 
1.7.2.5

[PATCH v6 5/5] main_loop_wait: block indefinitely

[Stefano Stabellini]

- remove qemu_calculate_timeout;

- explicitly size timeout to uint32_t;

- introduce slirp_update_timeout;

- pass NULL as timeout argument to select in case timeout is the maximum
value;

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Paul Brook <paul@codesourcery.com>
---
 async.c          |    2 +-
 main-loop.c      |   23 ++++++++++++++---------
 main-loop.h      |    2 +-
 qemu-timer.c     |    5 -----
 qemu-timer.h     |    1 -
 qemu-tool.c      |    4 ++++
 slirp/libslirp.h |    1 +
 slirp/slirp.c    |    7 +++++++
 8 files changed, 28 insertions(+), 17 deletions(-)

diff --git a/async.c b/async.c
index 332d511..ecdaf15 100644
--- a/async.c
+++ b/async.c
@@ -120,7 +120,7 @@ void qemu_bh_delete(QEMUBH *bh)
     bh->deleted = 1;
 }
 
-void qemu_bh_update_timeout(int *timeout)
+void qemu_bh_update_timeout(uint32_t *timeout)
 {
     QEMUBH *bh;
 
diff --git a/main-loop.c b/main-loop.c
index 1ebdc4b..f363748 100644
--- a/main-loop.c
+++ b/main-loop.c
@@ -226,7 +226,7 @@ static int max_priority;
 
 #ifndef _WIN32
 static void glib_select_fill(int *max_fd, fd_set *rfds, fd_set *wfds,
-                             fd_set *xfds, int *cur_timeout)
+                             fd_set *xfds, uint32_t *cur_timeout)
 {
     GMainContext *context = g_main_context_default();
     int i;
@@ -288,20 +288,24 @@ static void glib_select_poll(fd_set *rfds, fd_set *wfds, fd_set *xfds,
     }
 }
 
-static int os_host_main_loop_wait(int timeout)
+static int os_host_main_loop_wait(uint32_t timeout)
 {
-    struct timeval tv;
+    struct timeval tv, *tvarg = NULL;
     int ret;
 
     glib_select_fill(&nfds, &rfds, &wfds, &xfds, &timeout);
 
+    if (timeout < UINT32_MAX) {
+        tvarg = &tv;
+        tv.tv_sec = timeout / 1000;
+        tv.tv_usec = (timeout % 1000) * 1000;
+    }
+
     if (timeout > 0) {
         qemu_mutex_unlock_iothread();
     }
 
-    tv.tv_sec = timeout / 1000;
-    tv.tv_usec = (timeout % 1000) * 1000;
-    ret = select(nfds + 1, &rfds, &wfds, &xfds, &tv);
+    ret = select(nfds + 1, &rfds, &wfds, &xfds, tvarg);
 
     if (timeout > 0) {
         qemu_mutex_lock_iothread();
@@ -400,7 +404,7 @@ void qemu_fd_register(int fd)
                    FD_CONNECT | FD_WRITE | FD_OOB);
 }
 
-static int os_host_main_loop_wait(int timeout)
+static int os_host_main_loop_wait(uint32_t timeout)
 {
     GMainContext *context = g_main_context_default();
     int ret, i;
@@ -463,12 +467,12 @@ static int os_host_main_loop_wait(int timeout)
 
 int main_loop_wait(int nonblocking)
 {
-    int ret, timeout;
+    int ret;
+    uint32_t timeout = UINT32_MAX;
 
     if (nonblocking) {
         timeout = 0;
     } else {
-        timeout = qemu_calculate_timeout();
         qemu_bh_update_timeout(&timeout);
     }
 
@@ -480,6 +484,7 @@ int main_loop_wait(int nonblocking)
     FD_ZERO(&xfds);
 
 #ifdef CONFIG_SLIRP
+    slirp_update_timeout(&timeout);
     slirp_select_fill(&nfds, &rfds, &wfds, &xfds);
 #endif
     qemu_iohandler_fill(&nfds, &rfds, &wfds, &xfds);
diff --git a/main-loop.h b/main-loop.h
index e743aa0..c06b8bc 100644
--- a/main-loop.h
+++ b/main-loop.h
@@ -365,6 +365,6 @@ void qemu_iohandler_poll(fd_set *readfds, fd_set *writefds, fd_set *xfds, int rc
 
 void qemu_bh_schedule_idle(QEMUBH *bh);
 int qemu_bh_poll(void);
-void qemu_bh_update_timeout(int *timeout);
+void qemu_bh_update_timeout(uint32_t *timeout);
 
 #endif
diff --git a/qemu-timer.c b/qemu-timer.c
index 1166beb..887babf 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -821,8 +821,3 @@ fail:
     return err;
 }
 
-int qemu_calculate_timeout(void)
-{
-    return 1000;
-}
-
diff --git a/qemu-timer.h b/qemu-timer.h
index 661bbe7..094e730 100644
--- a/qemu-timer.h
+++ b/qemu-timer.h
@@ -63,7 +63,6 @@ void qemu_run_timers(QEMUClock *clock);
 void qemu_run_all_timers(void);
 int qemu_alarm_pending(void);
 void configure_alarms(char const *opt);
-int qemu_calculate_timeout(void);
 void init_clocks(void);
 int init_timer_alarm(void);
 
diff --git a/qemu-tool.c b/qemu-tool.c
index edb84f5..a0544d7 100644
--- a/qemu-tool.c
+++ b/qemu-tool.c
@@ -91,6 +91,10 @@ int qemu_init_main_loop(void)
     return main_loop_init();
 }
 
+void slirp_update_timeout(uint32_t *timeout)
+{
+}
+
 void slirp_select_fill(int *pnfds, fd_set *readfds,
                        fd_set *writefds, fd_set *xfds)
 {
diff --git a/slirp/libslirp.h b/slirp/libslirp.h
index 890fd86..77527ad 100644
--- a/slirp/libslirp.h
+++ b/slirp/libslirp.h
@@ -15,6 +15,7 @@ Slirp *slirp_init(int restricted, struct in_addr vnetwork,
                   struct in_addr vnameserver, void *opaque);
 void slirp_cleanup(Slirp *slirp);
 
+void slirp_update_timeout(uint32_t *timeout);
 void slirp_select_fill(int *pnfds,
                        fd_set *readfds, fd_set *writefds, fd_set *xfds);
 
diff --git a/slirp/slirp.c b/slirp/slirp.c
index 1502830..90473eb 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -258,6 +258,13 @@ void slirp_cleanup(Slirp *slirp)
 #define CONN_CANFRCV(so) (((so)->so_state & (SS_FCANTRCVMORE|SS_ISFCONNECTED)) == SS_ISFCONNECTED)
 #define UPD_NFDS(x) if (nfds < (x)) nfds = (x)
 
+void slirp_update_timeout(uint32_t *timeout)
+{
+    if (!QTAILQ_EMPTY(&slirp_instances)) {
+        *timeout = MIN(1000, *timeout);
+    }
+}
+
 void slirp_select_fill(int *pnfds,
                        fd_set *readfds, fd_set *writefds, fd_set *xfds)
 {
-- 
1.7.2.5