[Qemu-devel] [PATCH 00/19][RFC] Cleanups + split timer handling out of vl.o

[Paolo Bonzini]

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

[Qemu-devel] [PATCH 01/19] centralize handling of -icount

[Paolo Bonzini]

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

Signed-off-by: Paolo Bonzini pbonz...@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

[Qemu-devel] [PATCH 02/19] add qemu_icount_round

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini pbonz...@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  0x) ? 0x : count;
 count -= decr;
@@ -4141,9 +4144,7 @@ static int qemu_calculate_timeout(void)
 if (add  1000)
 add = 1000;
 delta += add;
-add = (add + (1  icount_time_shift) - 1)
-   icount_time_shift;
-qemu_icount += add;
+qemu_icount += qemu_icount_round (add);
 timeout = delta / 100;
 if (timeout  0)
 timeout = 0;
-- 
1.6.5.2

[Qemu-devel] [PATCH 03/19] avoid dubiously clever code in win32_start_timer

[Paolo Bonzini]

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 pbonz...@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

[Qemu-devel] [PATCH 04/19] fix error in win32_rearm_timer

[Paolo Bonzini]

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 pbonz...@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

[Qemu-devel] [PATCH 07/19] add qemu_get_clock_ns

[Paolo Bonzini]

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 pbonz...@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

[Qemu-devel] [PATCH 05/19] only one flag is needed for alarm_timer

[Paolo Bonzini]

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

Signed-off-by: Paolo Bonzini pbonz...@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

[Qemu-devel] [PATCH 09/19] remove qemu_rearm_alarm_timer from main loop

[Paolo Bonzini]

Make the timer subsystem register its own callback instead.

Signed-off-by: Paolo Bonzini pbonz...@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

[Qemu-devel] [PATCH 08/19] move kbd/mouse events to event.c

[Paolo Bonzini]

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 pbonz...@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 000..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)
+

[Qemu-devel] [PATCH 11/19] use a bottom half to run timers

[Paolo Bonzini]

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 pbonz...@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

[Qemu-devel] [PATCH 10/19] add qemu_bh_scheduled

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini pbonz...@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

[Qemu-devel] [PATCH 06/19] more alarm timer cleanup

[Paolo Bonzini]

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 pbonz...@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

[Qemu-devel] [PATCH 12/19] new function qemu_icount_delta

[Paolo Bonzini]

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

Signed-off-by: Paolo Bonzini pbonz...@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) 100;
+} 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 / 100) + 1;
+timeout = (delta + 99) / 100;
 } else {
 /* Wait for either IO to occur or the next
timer event.  */
-- 
1.6.5.2

[Qemu-devel] [PATCH 16/19] tweak qemu_notify_event

[Paolo Bonzini]

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 pbonz...@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

[Qemu-devel] [PATCH 13/19] move tcg_has_work to cpu-exec.c and rename it

[Paolo Bonzini]

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 pbonz...@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

[Qemu-devel] [PATCH 15/19] do not provide qemu_event_increment if iothread not used

[Paolo Bonzini]

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 pbonz...@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

[Qemu-devel] [PATCH 14/19] disentangle tcg and deadline calculation

[Paolo Bonzini]

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 pbonz...@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(1);
 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

[Qemu-devel] [PATCH 18/19] introduce qemu_clock_enable

[Paolo Bonzini]

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 pbonz...@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

[Qemu-devel] [PATCH 17/19] move vmstate registration of vmstate_timers earlier

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini pbonz...@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

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Dor Laor]

On 12/21/2009 09:43 AM, Gleb Natapov wrote:

On Sun, Dec 20, 2009 at 11:59:43AM -0600, Anthony Liguori wrote:

Gleb Natapov wrote:

Windows is a mystery box, so we can speculate as much as we want about it.
If you don't like something just say it may break Windows :) Losing
activation does sound like an issue too.


Downgrading seems more likely to cause problems.  Considering
running mplayer in a guest.  If it detects SSE3 in one host and
migrate to another host that doesn't have SSE3, you'll be running an
instruction stream that uses instructions the processor doesn't
support resulting in the application faulting due to an illegal
operation.

KVM's cpuid filter doesn't help here because it won't attempt to
mask things like sse3.  It would be insane to emulate sse3 too.

This is a classic problem with live migration.  You can detect this
scenario and potentially block the migration, but honestly, how
likely is it that any given guest is using an obscure feature?  This
is a classic management tool problem and the solution usually is a
set of heuristics depending on how conservative the target audience
is.


I am confused. We explicitly not talking about migration, why bring it
here? We are talking about casual user who can't care less about
migration, but he upgrades his home computer and Windows VM stops
working for him.


John's new cpu definitions are the exact solution for this issue - all 
users, whether using mgmt app or direct qemu (this is no user, this is a 
developer/hacker/other, let's do not optimize this case) should use the 
various 'real' cpu definitions like -cpu Merom | Nehalem | Penry | 
Opteron G1, 


Qemu will check the required cpuid of the cpu model on the host and 
refuse to load otherwise. When moving to this model, migration can be 
simplified too since there are fewer combination, and one can choose 
performance over migration flexibility and wise versa.
Due to the above check, the destination qemu won't load if the host does 
not support its cpu model.





--
Gleb.

[Qemu-devel] [PATCH] Use vpath directive

[Paolo Bonzini]

The vpath directive has two advantages over the VPATH variable:
1) it allows to skip searching of .o files; 2) the default semantics
are to append to the vpath, so there is no confusion between VPATH=xyz
and VPATH+=xyz.

Since vpath %.c %.h PATH is not valid, I'm introducing a wrapper
macro to append one or more directories to the vpath.

Signed-off-by: Paolo Bonzini pbonz...@redhat.com
---
This one also fixes the same race (by building cutils.o twice).
However, it is nicer in that it fixes the wrong occurrence of
vpath %.c %.h PATH (sorry for missing that on my review of
Kirill's patch).

 Makefile   |2 +-
 Makefile.hw|2 +-
 Makefile.target|   11 +++
 Makefile.user  |6 +-
 pc-bios/optionrom/Makefile |3 ++-
 rules.mak  |2 ++
 tests/Makefile |3 ++-
 7 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index ec52ee2..645ec82 100644
--- a/Makefile
+++ b/Makefile
@@ -25,7 +25,7 @@ configure: ;
 .PHONY: all clean cscope distclean dvi html info install install-doc \
recurse-all speed tar tarbin test build-all
 
-VPATH=$(SRC_PATH):$(SRC_PATH)/hw
+$(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw)
 
 LIBS+=-lz $(LIBS_TOOLS)
 
diff --git a/Makefile.hw b/Makefile.hw
index bd252f5..781c006 100644
--- a/Makefile.hw
+++ b/Makefile.hw
@@ -7,7 +7,7 @@ include $(SRC_PATH)/rules.mak
 
 .PHONY: all
 
-VPATH=$(SRC_PATH):$(SRC_PATH)/hw
+$(call set-vpath, $(SRC_PATH):$(SRC_PATH)/hw)
 
 QEMU_CFLAGS+=-I.. -I$(SRC_PATH)/fpu
 
diff --git a/Makefile.target b/Makefile.target
index 0504d3b..92c2027 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -9,7 +9,7 @@ include config-target.mak
 include $(SRC_PATH)/rules.mak
 
 TARGET_PATH=$(SRC_PATH)/target-$(TARGET_BASE_ARCH)
-VPATH=$(SRC_PATH):$(TARGET_PATH):$(SRC_PATH)/hw
+$(call set-vpath, $(SRC_PATH):$(TARGET_PATH):$(SRC_PATH)/hw)
 QEMU_CFLAGS+= -I.. -I$(TARGET_PATH) -DNEED_CPU_H
 
 ifdef CONFIG_USER_ONLY
@@ -87,7 +87,8 @@ signal.o: QEMU_CFLAGS += $(HELPER_CFLAGS)
 
 ifdef CONFIG_LINUX_USER
 
-VPATH+=:$(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
+$(call set-vpath, 
$(SRC_PATH)/linux-user:$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR))
+
 QEMU_CFLAGS+=-I$(SRC_PATH)/linux-user 
-I$(SRC_PATH)/linux-user/$(TARGET_ABI_DIR)
 obj-y = main.o syscall.o strace.o mmap.o signal.o thunk.o \
   elfload.o linuxload.o uaccess.o gdbstub.o
@@ -115,7 +116,8 @@ endif #CONFIG_LINUX_USER
 
 ifdef CONFIG_DARWIN_USER
 
-VPATH+=:$(SRC_PATH)/darwin-user
+$(call set-vpath, $(SRC_PATH)/darwin-user)
+
 QEMU_CFLAGS+=-I$(SRC_PATH)/darwin-user -I$(SRC_PATH)/darwin-user/$(TARGET_ARCH)
 
 # Leave some space for the regular program loading zone
@@ -137,7 +139,8 @@ endif #CONFIG_DARWIN_USER
 
 ifdef CONFIG_BSD_USER
 
-VPATH+=:$(SRC_PATH)/bsd-user
+$(call set-vpath, $(SRC_PATH)/bsd-user)
+
 QEMU_CFLAGS+=-I$(SRC_PATH)/bsd-user -I$(SRC_PATH)/bsd-user/$(TARGET_ARCH)
 
 obj-y = main.o bsdload.o elfload.o mmap.o signal.o strace.o syscall.o \
diff --git a/Makefile.user b/Makefile.user
index 7daedef..5df114f 100644
--- a/Makefile.user
+++ b/Makefile.user
@@ -6,11 +6,7 @@ include $(SRC_PATH)/rules.mak
 
 .PHONY: all
 
-# Do not take %.o from $(SRC_PATH), only %.c and %.h
-# All %.o for user targets should be built with -fpie, when
-# configured with --enable-user-pie, so we don't want to
-# take %.o from $(SRC_PATH), since they built without -fpie
-vpath %.c %.h $(SRC_PATH)
+$(call set-vpath, $(SRC_PATH))
 
 QEMU_CFLAGS+=-I..
 
diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index 54db882..b4be31e 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -5,7 +5,8 @@ all: build-all
 include ../../config-host.mak
 include $(SRC_PATH)/rules.mak
 
-VPATH=$(SRC_PATH)/pc-bios/optionrom
+$(call set-vpath, $(SRC_PATH)/pc-bios/optionrom)
+
 .PHONY : all clean build-all
 
 CFLAGS := -Wall -Wstrict-prototypes -Werror -fomit-frame-pointer -fno-builtin
diff --git a/rules.mak b/rules.mak
index 5d9f684..9cd67f0 100644
--- a/rules.mak
+++ b/rules.mak
@@ -39,6 +39,8 @@ quiet-command = $(if $(V),$1,$(if $(2),@echo $2  $1, @$1))
 cc-option = $(if $(shell $(CC) $1 $2 -S -o /dev/null -xc /dev/null \
   /dev/null 21  echo OK), $2, $3)
 
+set-vpath = $(if $1,$(foreach PATTERN,%.c %.h %.S, $(eval vpath $(PATTERN) 
$1)))
+
 # Generate timestamp files for .h include files
 
 %.h: %.h-timestamp
diff --git a/tests/Makefile b/tests/Makefile
index 69092e5..ff7f787 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -1,5 +1,6 @@
 -include ../config-host.mak
-VPATH=$(SRC_PATH)/tests
+
+$(call set-vpath, $(SRC_PATH)/tests)
 
 CFLAGS=-Wall -O2 -g -fno-strict-aliasing
 #CFLAGS+=-msse2
-- 
1.6.5.2

Re: [Qemu-devel] [PATCH 0/5] tcg conditional set, round 4

[Laurent Desnogues]

On Mon, Dec 21, 2009 at 3:00 AM, Richard Henderson r...@twiddle.net wrote:
[...]
 I *am* convinced that to remove either VTRUE or VFALSE as arguments to the
 movcond primitive (implementing dest = (cond ? vtrue : dest) would do too
 much violence to both the liveness analysis and the register allocator
 within TCG.  The best I can do to remove the complexity is:

 static void tcg_out_movcond(TCGContext *s, int cond, TCGArg dest,
                            TCGArg cmp1, TCGArg cmp2, int const_cmp2,
                            TCGArg vtrue, int rexw)
 {
    tcg_out_cmp(s, cond, cmp1, cmp2, const_cmp2, rexw);
    /* cmovcc */
    tcg_out_modrm(s, 0x40 | tcg_cond_to_jcc[cond] | P_EXT | rexw,
                  dest, vtrue);
 }
 ...
    { INDEX_op_movcond_i32, { r, r, ri, r, 0 } },
    { INDEX_op_movcond_i64, { r, r, re, r, 0 } },

 using matching constraints in the target and directly implement the
 conditional move.  This eliminates the code I previously had that checked
 for things like dest=vfalse and inverted the condition.  I had planned to
 simplify the i386 version similarly, even in the case where cmovcc is not
 available.

 I would appreciate some direction here, so as to avoid wasting my time with
 a solution that won't be accepted.

The question for the generalized movcond is how useful is it?
Which front-ends would need it and would the cost to generate
code for it on some (most?) back-ends be amortized?

My guess (I use that word given that I didn't do any benchmark
to sustain my claim) is that your implementation is too complex.

Of course setcond can be implemented in terms of movcond,
but my guess (again that word...) is that setcond could be
enough and even faster in most cases.

To sum up, simplicity is extremely desirable as some profiles
are highly dependent on code generation time, and we lack
a set of benchmarks to check how good or bad our ideas
and implementations are.

Regarding your patches, I would like to see setcond put in
mainline with a simplified version for i386.  From there
people could implement it for all front-ends and back-ends.
Then we could move on to movcond and see how useful
it is.  I am all in favor of doing this incrementally as if we
aim for the best solution without participation from many
people, it'll be the best way not to do anything (cf. my
previous proposal for setcond more than a year ago).


Laurent

Re: [Qemu-devel] [PATCH] PPC64: Fix timebase

[Aurelien Jarno]

On Mon, Dec 21, 2009 at 01:22:12AM +0100, Alexander Graf wrote:
 On PPC we have a 64-bit time base. Usually (PPC32) this is accessed using
 two separate 32 bit SPR accesses to SPR_TBU and SPR_TBL.
 
 On PPC64 the SPR_TBL register acts as 64 bit though, so we get the full
 64 bits as return value. If we only take the lower ones, fine. But Linux
 wants to see all 64 bits or it breaks.

Good catch! However, I think this patch it's not fully complete and can
be improved a bit
- it's probably better to return a target_ulong value from
  cpu_ppc_load_tbl() with an explicit cast here, so that we don't have
  an implicit cast from 64-bit to 32-bit on qemu-system-powerpc (GCC may
  warn on that with some flags or in future versions).
- the store function also has to be fixed.
- the same changes should be done for the alternate timebase.

 This patch makes PPC64 Linux work even after TB crossed the 32-bit boundary,
 which usually happened a few seconds after bootup.
 
 Signed-off-by: Alexander Graf ag...@suse.de
 
 ---
 
 To verify my assumptions of the above I used this test program:
 
   int main()
   {
   unsigned int tbu=0, tbl=0;
   unsigned long tb=0;
 
   asm(mftbu %0 : =r (tbu));
   asm(mftbl %0 : =r (tbl));
   asm(mftbl %0 : =r (tb));
 
   printf(TB: %#x %#x\n, tbu, tbl);
   printf(TB64: %#lx\n, tb);
   }
 
 It produces the following output on a 970MP CPU:
 
 $ ./mftb
 TB: 0x238 0xd676bd6
 TB64: 0x2380d676f75
 ---
  hw/ppc.c |4 ++--
  target-ppc/cpu.h |2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)
 
 diff --git a/hw/ppc.c b/hw/ppc.c
 index 5208039..b4bf2d3 100644
 --- a/hw/ppc.c
 +++ b/hw/ppc.c
 @@ -401,7 +401,7 @@ static inline uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, 
 uint64_t vmclk,
  return muldiv64(vmclk, tb_env-tb_freq, get_ticks_per_sec()) + tb_offset;
  }
  
 -uint32_t cpu_ppc_load_tbl (CPUState *env)
 +uint64_t cpu_ppc_load_tbl (CPUState *env)
  {
  ppc_tb_t *tb_env = env-tb_env;
  uint64_t tb;
 @@ -409,7 +409,7 @@ uint32_t cpu_ppc_load_tbl (CPUState *env)
  tb = cpu_ppc_get_tb(tb_env, qemu_get_clock(vm_clock), tb_env-tb_offset);
  LOG_TB(%s: tb %016 PRIx64 \n, __func__, tb);
  
 -return tb  0x;
 +return tb;
  }
  
  static inline uint32_t _cpu_ppc_load_tbu(CPUState *env)
 diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
 index 2535cbc..2dc301d 100644
 --- a/target-ppc/cpu.h
 +++ b/target-ppc/cpu.h
 @@ -741,7 +741,7 @@ int cpu_ppc_register_internal (CPUPPCState *env, const 
 ppc_def_t *def);
  
  /* Time-base and decrementer management */
  #ifndef NO_CPU_IO_DEFS
 -uint32_t cpu_ppc_load_tbl (CPUPPCState *env);
 +uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
  uint32_t cpu_ppc_load_tbu (CPUPPCState *env);
  void cpu_ppc_store_tbu (CPUPPCState *env, uint32_t value);
  void cpu_ppc_store_tbl (CPUPPCState *env, uint32_t value);
 -- 
 1.6.0.2
 
 
 
 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

[Qemu-devel] Re: [ANNOUNCE] qemu-kvm-0.12.0-rc2 released

[Avi Kivity]

On 12/21/2009 05:39 AM, Dustin Kirkland wrote:


What about the qemu-system-arm build break I also mentioned?  That's
currently blocking my packaging.
   


I'm looking into it, though of course patches are welcome as usual.

--
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] PPC64: Fix timebase

[Alexander Graf]

On 21.12.2009, at 10:24, Aurelien Jarno wrote:

 On Mon, Dec 21, 2009 at 01:22:12AM +0100, Alexander Graf wrote:
 On PPC we have a 64-bit time base. Usually (PPC32) this is accessed using
 two separate 32 bit SPR accesses to SPR_TBU and SPR_TBL.
 
 On PPC64 the SPR_TBL register acts as 64 bit though, so we get the full
 64 bits as return value. If we only take the lower ones, fine. But Linux
 wants to see all 64 bits or it breaks.
 
 Good catch! However, I think this patch it's not fully complete and can
 be improved a bit
 - it's probably better to return a target_ulong value from
  cpu_ppc_load_tbl() with an explicit cast here, so that we don't have
  an implicit cast from 64-bit to 32-bit on qemu-system-powerpc (GCC may
  warn on that with some flags or in future versions).

ppc.c is in hw, so I suspect it's in the target independent makefile part? 
Otherwise we should move all TB stuff to target-ppc.

 - the store function also has to be fixed.

Oh, right.

 - the same changes should be done for the alternate timebase.

Hum, probably. Right.

Alex

[Qemu-devel] Re: [PATCH 0/5] tcg conditional set, round 4

[Paolo Bonzini]

On 12/21/2009 10:13 AM, Laurent Desnogues wrote:

Which front-ends would need it and would the cost to generate
code for it on some (most?) back-ends be amortized?


The ARM front-end could definitely use a backend's ability to do 
predication (via movcond).


Paolo

[Qemu-devel] Re: [PATCH 0/5] tcg conditional set, round 4

[Laurent Desnogues]

On Mon, Dec 21, 2009 at 10:47 AM, Paolo Bonzini pbonz...@redhat.com wrote:
 On 12/21/2009 10:13 AM, Laurent Desnogues wrote:

 Which front-ends would need it and would the cost to generate
 code for it on some (most?) back-ends be amortized?

 The ARM front-end could definitely use a backend's ability to do predication
 (via movcond).

I think most things done in the ARM front-end can be done
with a simpler movcond that'd be:

   movcond cond, arg_cond1, arg_cond2, dest, vtrue

though as Richard said this could impact tcg reg alloc and
liveness analysis.

Also, I wonder what would be the impact on speed.


Laurent

Re: [Qemu-devel] Re: SVM support in 0.12?

[Jun Koi]

On Sat, Dec 19, 2009 at 2:34 AM, Alexander Graf ag...@suse.de wrote:

 Am 18.12.2009 um 17:52 schrieb Jun Koi junkoi2...@gmail.com:

 On Fri, Dec 18, 2009 at 8:35 PM, Alexander Graf ag...@suse.de wrote:

 Am 18.12.2009 um 03:39 schrieb Jun Koi junkoi2...@gmail.com

 On Fri, Dec 18, 2009 at 11:37 AM, Jun Koi junkoi2...@gmail.com wrote:

 Hi,

 I am running latest Qemu 0.12-rc. My guest VM runs Linux kernel 2.6.31.

 Because Qemu now supports SVM, I expect to see the SVM flag in
 /proc/cpuinfo, but that is not the case.

 So it seems SVM support is not enabled by default configuration??


 My host and guest are both 32 bit Linux, if that matters.

 (And this is pure Qemu, without using KVM or KQemu)

 Kqemu actually works with svm emulation.

 Have you tried -cpu qemu32,+svm? I think I only enabled svm on qemu64,
 because I'm not aware of 32-bit AMD CPUs that can do svm.

 If you use -cpu qemu32 that also means you'll get an emulated Intel CPU.
 KVM-AMD will refuse there.

 Also, back when I developed it, 32-bit kvm didn't really work properly.
 So
 please let me know what you find out!

 I tried the latest Qemu code, with the following command:

 qemu -m 500 -cpu qemu32,+svm -cdrom ubuntu.iso

 I verified that /proc/cpuinfo has no svm flag. So SVM doesnt work on 32bit
 host.

 I will try that with 64bit host to see how it goes.

 The host doesn't matter. You can easily run qemu-system-x86_64 on a 32-bit
 host.


I can confirm that SVM works well on x86-64 target, but fails on i386 target.

Thanks,
J

Re: [Qemu-devel] [PATCH 0/5] tcg conditional set, round 4

[Aurelien Jarno]

On Sun, Dec 20, 2009 at 06:00:48PM -0800, Richard Henderson wrote:
 On 12/20/2009 02:57 PM, Paul Brook wrote:
 On Saturday 19 December 2009, Richard Henderson wrote:
 Changes from round 3:

   * Drop movcond for now.
   * Only use movzbl and not xor in setcond.

 I'm still catching up on mail backlog from this thread, but I'm concerned 
 that
 we're exposing setcond to the target translation code if we're planning on
 implementing movcond later.

 I personally was only planning to use setcond in those cases where the  
 target actually wants a 0/1 value.  E.g. i386 setcc, or alpha cmp*, or  
 mips slt*.

 As for shifts and masks, that wasn't in my plans.  I had a comment in  
 there about all the tricks that gcc plays with a conditional move of two  
 constants, and the fact that I didn't think it worth the effort.

 Indeed, my plan for today was to cut down my existing movcond patch to  
 make it simpler, as that seems to be the way Aurelien wants this to go.  
 Life conspired against and I got nothing done, but still.

 I *am* convinced that to remove either VTRUE or VFALSE as arguments to  
 the movcond primitive (implementing dest = (cond ? vtrue : dest) would  
 do too much violence to both the liveness analysis and the register  
 allocator within TCG.  The best I can do to remove the complexity is:

 static void tcg_out_movcond(TCGContext *s, int cond, TCGArg dest,
 TCGArg cmp1, TCGArg cmp2, int const_cmp2,
 TCGArg vtrue, int rexw)
 {
 tcg_out_cmp(s, cond, cmp1, cmp2, const_cmp2, rexw);
 /* cmovcc */
 tcg_out_modrm(s, 0x40 | tcg_cond_to_jcc[cond] | P_EXT | rexw,
   dest, vtrue);
 }
 ...
 { INDEX_op_movcond_i32, { r, r, ri, r, 0 } },
 { INDEX_op_movcond_i64, { r, r, re, r, 0 } },

 using matching constraints in the target and directly implement the  
 conditional move.  This eliminates the code I previously had that  
 checked for things like dest=vfalse and inverted the condition.  I had  
 planned to simplify the i386 version similarly, even in the case where  
 cmovcc is not available.

 I would appreciate some direction here, so as to avoid wasting my time  
 with a solution that won't be accepted.


As Paul pointed, there is some redundancy between setcond and movcond,
the first one being a subset of the second one. I am not personally
convinced of the gain of both these new instructions on the performance
side, but at least I am convinced that setcond makes code in targets 
much simpler compared to brcond and jumps.

I have also very bad experience in generating highly optimized code, as
the code generation then becomes complex, which often results in a slow
down. It's clearly a fearing I have for movcond, but again I haven't
seen any benchmark going in one or the other direction. On the other 
hand setcond implementation for the various hosts is much simpler, even
on 32-bit hosts.

That said, we have had various setcond implementations discussed on the
mailing list for more than a year, and we haven't made any progress
on that, while it is an item from Fabrice's TODO list.

My proposal would be to drop movcond for now (meaning at least until
after next release to answer Paul), unless you can convince us that
movcond can bring a big gain on performance. This way we can focus
on setcond, have implementations for all hosts, and get it used in all
targets. After the next release or later, when setcond is really used
wherever possible, we can try to see if movcond is really necessary or
not.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] PPC64: Fix timebase

[Aurelien Jarno]

On Mon, Dec 21, 2009 at 10:39:39AM +0100, Alexander Graf wrote:
 
 On 21.12.2009, at 10:24, Aurelien Jarno wrote:
 
  On Mon, Dec 21, 2009 at 01:22:12AM +0100, Alexander Graf wrote:
  On PPC we have a 64-bit time base. Usually (PPC32) this is accessed using
  two separate 32 bit SPR accesses to SPR_TBU and SPR_TBL.
  
  On PPC64 the SPR_TBL register acts as 64 bit though, so we get the full
  64 bits as return value. If we only take the lower ones, fine. But Linux
  wants to see all 64 bits or it breaks.
  
  Good catch! However, I think this patch it's not fully complete and can
  be improved a bit
  - it's probably better to return a target_ulong value from
   cpu_ppc_load_tbl() with an explicit cast here, so that we don't have
   an implicit cast from 64-bit to 32-bit on qemu-system-powerpc (GCC may
   warn on that with some flags or in future versions).
 
 ppc.c is in hw, so I suspect it's in the target independent makefile part? 
 Otherwise we should move all TB stuff to target-ppc.

Correct. then let's return uint64_t for cpu_ppc_load_tbl(), but do the
explicit cast in the helper.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] Re: SVM support in 0.12?

[Andre Przywara]

Jun Koi wrote:

I am running latest Qemu 0.12-rc. My guest VM runs Linux kernel 2.6.31.

Because Qemu now supports SVM, I expect to see the SVM flag in
/proc/cpuinfo, but that is not the case.

So it seems SVM support is not enabled by default configuration??


My host and guest are both 32 bit Linux, if that matters.

(And this is pure Qemu, without using KVM or KQemu)

Kqemu actually works with svm emulation.


...

qemu -m 500 -cpu qemu32,+svm -cdrom ubuntu.iso

I verified that /proc/cpuinfo has no svm flag. So SVM doesnt work on 32bit
host.

I will try that with 64bit host to see how it goes.

The host doesn't matter. You can easily run qemu-system-x86_64 on a 32-bit
host.



I can confirm that SVM works well on x86-64 target, but fails on i386 target.


The Linux kernel will only detect SVM if the machine is AMD (see 
linux-2.6/arch/x86/include/asm/virtext.h:cpu_has_svm())

So please try:
$ qemu -m 500 -cpu qemu32,+svm,vendor=AuthenticAMD -cdrom ubuntu.iso
(because the default vendor for qemu32 is Intel, for qemu64 AMD)

Regards,
Andre.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [Qemu-devel] [PATCH 3/3] linux-user: Initialize Alpha FPCR register.

[Aurelien Jarno]

On Sat, Dec 19, 2009 at 03:17:16PM -0800, Richard Henderson wrote:
 Signed-off-by: Richard Henderson r...@twiddle.net
 ---
  linux-user/main.c |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
 
 diff --git a/linux-user/main.c b/linux-user/main.c
 index 12502ad..b67662c 100644
 --- a/linux-user/main.c
 +++ b/linux-user/main.c
 @@ -3052,6 +3052,8 @@ int main(int argc, char **argv, char **envp)
  env-ir[30] = regs-usp;
  env-pc = regs-pc;
  env-unique = regs-unique;
 +cpu_alpha_store_fpcr(env, (FPCR_INVD | FPCR_DZED | FPCR_OVFD
 +   | FPCR_UNFD | FPCR_INED | FPCR_DNOD));
  }

This cpu initialization which does not depends on the binary being run
is usually done in target-*/translate.c, using #if defined
(CONFIG_USER_ONLY).

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Avi Kivity]

On 12/20/2009 07:18 PM, Michael S. Tsirkin wrote:



Hmm, then, shouldn't either kvm or qemu mask features that we do not
emulate well enough to make windows not crash?
   

-cpu host does that already, no?

Alex
 

I expected so, but Avi here seems to say windows will crash if you
use a new CPU with it ...
   


No, Windows tries to detect changes in your hardware and assumes that if 
too many things change, you might be a pirate and requires you to phone 
their offices to re-authenticate.


--
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] PPC64: Fix timebase

[Alexander Graf]

On 21.12.2009, at 10:24, Aurelien Jarno wrote:

 On Mon, Dec 21, 2009 at 01:22:12AM +0100, Alexander Graf wrote:
 On PPC we have a 64-bit time base. Usually (PPC32) this is accessed using
 two separate 32 bit SPR accesses to SPR_TBU and SPR_TBL.
 
 On PPC64 the SPR_TBL register acts as 64 bit though, so we get the full
 64 bits as return value. If we only take the lower ones, fine. But Linux
 wants to see all 64 bits or it breaks.
 
 Good catch! However, I think this patch it's not fully complete and can
 be improved a bit
 - it's probably better to return a target_ulong value from
  cpu_ppc_load_tbl() with an explicit cast here, so that we don't have
  an implicit cast from 64-bit to 32-bit on qemu-system-powerpc (GCC may
  warn on that with some flags or in future versions).
 - the store function also has to be fixed.

According to Book3:

It is not possible to write the entire 64-bit Time Base using a single 
instruction. The mttbl and mttbu extended mnemonics write the lower and upper 
halves of the Time Base (TBL and TBU), respectively, preserving the other half. 
These are extended mne- monics for the mtspr instruction; see page 83.

 - the same changes should be done for the alternate timebase.

I can't find traces of the alternative timebase in the docs :-(.

Alex

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Avi Kivity]

On 12/20/2009 07:59 PM, Anthony Liguori wrote:

Gleb Natapov wrote:
Windows is a mystery box, so we can speculate as much as we want 
about it.

If you don't like something just say it may break Windows :) Losing
activation does sound like an issue too.


Downgrading seems more likely to cause problems.  Considering running 
mplayer in a guest.  If it detects SSE3 in one host and migrate to 
another host that doesn't have SSE3, you'll be running an instruction 
stream that uses instructions the processor doesn't support resulting 
in the application faulting due to an illegal operation.




Migration needs preparation beforehand.  You can't take two random hosts 
with two random qemu flag sets and expect it to work.


KVM's cpuid filter doesn't help here because it won't attempt to mask 
things like sse3.  It would be insane to emulate sse3 too.


It does expose sse3 support (called pni in Linux IIRC).  Not sure if 
qemu masks it, but the information is there.




This is a classic problem with live migration.  You can detect this 
scenario and potentially block the migration, but honestly, how likely 
is it that any given guest is using an obscure feature? 


100% likely.

This is a classic management tool problem and the solution usually is 
a set of heuristics depending on how conservative the target audience is.


Right.  My concern is with casual users upgrading their machine, not 
enterprise users who should be protected by their tools.


--
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] PPC64: Fix timebase

[Alexander Graf]

On 21.12.2009, at 10:24, Aurelien Jarno wrote:

 On Mon, Dec 21, 2009 at 01:22:12AM +0100, Alexander Graf wrote:
 On PPC we have a 64-bit time base. Usually (PPC32) this is accessed using
 two separate 32 bit SPR accesses to SPR_TBU and SPR_TBL.
 
 On PPC64 the SPR_TBL register acts as 64 bit though, so we get the full
 64 bits as return value. If we only take the lower ones, fine. But Linux
 wants to see all 64 bits or it breaks.
 
 Good catch! However, I think this patch it's not fully complete and can
 be improved a bit
 - it's probably better to return a target_ulong value from
  cpu_ppc_load_tbl() with an explicit cast here, so that we don't have
  an implicit cast from 64-bit to 32-bit on qemu-system-powerpc (GCC may
  warn on that with some flags or in future versions).
 - the store function also has to be fixed.
 - the same changes should be done for the alternate timebase.

Uuuh:

__attribute__ (( unused ))
static void spr_read_atbl (void *opaque, int gprn, int sprn)
{
gen_helper_load_atbl(cpu_gpr[gprn]);
}

And that attribute is correct. There is no caller.

Alex

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Michael S. Tsirkin]

On Mon, Dec 21, 2009 at 01:12:26PM +0200, Avi Kivity wrote:
 On 12/20/2009 07:18 PM, Michael S. Tsirkin wrote:

 Hmm, then, shouldn't either kvm or qemu mask features that we do not
 emulate well enough to make windows not crash?

 -cpu host does that already, no?

 Alex
  
 I expected so, but Avi here seems to say windows will crash if you
 use a new CPU with it ...


 No, Windows tries to detect changes in your hardware and assumes that if  
 too many things change, you might be a pirate and requires you to phone  
 their offices to re-authenticate.

How often does a casual user upgrade his host CPU?

 -- 
 error compiling committee.c: too many arguments to function

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Alexander Graf]

On 21.12.2009, at 12:18, Michael S. Tsirkin wrote:

 On Mon, Dec 21, 2009 at 01:12:26PM +0200, Avi Kivity wrote:
 On 12/20/2009 07:18 PM, Michael S. Tsirkin wrote:
 
 Hmm, then, shouldn't either kvm or qemu mask features that we do not
 emulate well enough to make windows not crash?
 
 -cpu host does that already, no?
 
 Alex
 
 I expected so, but Avi here seems to say windows will crash if you
 use a new CPU with it ...
 
 
 No, Windows tries to detect changes in your hardware and assumes that if  
 too many things change, you might be a pirate and requires you to phone  
 their offices to re-authenticate.
 
 How often does a casual user upgrade his host CPU?

I tend to have my VM images on an NFS share and expect them to work properly on 
all PCs I work on. So I guess the answer is often?

Alex

[Qemu-devel] [PATCH] PPC64: Fix timebase

[Alexander Graf]

On PPC we have a 64-bit time base. Usually (PPC32) this is accessed using
two separate 32 bit SPR accesses to SPR_TBU and SPR_TBL.

On PPC64 the SPR_TBL register acts as 64 bit though, so we get the full
64 bits as return value. If we only take the lower ones, fine. But Linux
wants to see all 64 bits or it breaks.

This patch makes PPC64 Linux work even after TB crossed the 32-bit boundary,
which usually happened a few seconds after bootup.

Signed-off-by: Alexander Graf ag...@suse.de

---

To verify my assumptions of the above I used this test program:

  int main()
  {
  unsigned int tbu=0, tbl=0;
  unsigned long tb=0;

  asm(mftbu %0 : =r (tbu));
  asm(mftbl %0 : =r (tbl));
  asm(mftbl %0 : =r (tb));

  printf(TB: %#x %#x\n, tbu, tbl);
  printf(TB64: %#lx\n, tb);
  }

It produces the following output on a 970MP CPU:

$ ./mftb
TB: 0x238 0xd676bd6
TB64: 0x2380d676f75

V1 - V2:

  - adjust user targets too
  - do an explicit cast for target_ulong
---
 darwin-user/main.c |4 ++--
 hw/ppc.c   |4 ++--
 linux-user/main.c  |4 ++--
 target-ppc/cpu.h   |2 +-
 target-ppc/op_helper.c |2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/darwin-user/main.c b/darwin-user/main.c
index 5fd314e..2f4a0f8 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -82,9 +82,9 @@ static inline uint64_t cpu_ppc_get_tb (CPUState *env)
 return 0;
 }
 
-uint32_t cpu_ppc_load_tbl (CPUState *env)
+uint64_t cpu_ppc_load_tbl (CPUState *env)
 {
-return cpu_ppc_get_tb(env)  0x;
+return cpu_ppc_get_tb(env);
 }
 
 uint32_t cpu_ppc_load_tbu (CPUState *env)
diff --git a/hw/ppc.c b/hw/ppc.c
index 5208039..b4bf2d3 100644
--- a/hw/ppc.c
+++ b/hw/ppc.c
@@ -401,7 +401,7 @@ static inline uint64_t cpu_ppc_get_tb(ppc_tb_t *tb_env, 
uint64_t vmclk,
 return muldiv64(vmclk, tb_env-tb_freq, get_ticks_per_sec()) + tb_offset;
 }
 
-uint32_t cpu_ppc_load_tbl (CPUState *env)
+uint64_t cpu_ppc_load_tbl (CPUState *env)
 {
 ppc_tb_t *tb_env = env-tb_env;
 uint64_t tb;
@@ -409,7 +409,7 @@ uint32_t cpu_ppc_load_tbl (CPUState *env)
 tb = cpu_ppc_get_tb(tb_env, qemu_get_clock(vm_clock), tb_env-tb_offset);
 LOG_TB(%s: tb %016 PRIx64 \n, __func__, tb);
 
-return tb  0x;
+return tb;
 }
 
 static inline uint32_t _cpu_ppc_load_tbu(CPUState *env)
diff --git a/linux-user/main.c b/linux-user/main.c
index 12502ad..5aa9dac 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1068,9 +1068,9 @@ static inline uint64_t cpu_ppc_get_tb (CPUState *env)
 return 0;
 }
 
-uint32_t cpu_ppc_load_tbl (CPUState *env)
+uint64_t cpu_ppc_load_tbl (CPUState *env)
 {
-return cpu_ppc_get_tb(env)  0x;
+return cpu_ppc_get_tb(env);
 }
 
 uint32_t cpu_ppc_load_tbu (CPUState *env)
diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2535cbc..2dc301d 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -741,7 +741,7 @@ int cpu_ppc_register_internal (CPUPPCState *env, const 
ppc_def_t *def);
 
 /* Time-base and decrementer management */
 #ifndef NO_CPU_IO_DEFS
-uint32_t cpu_ppc_load_tbl (CPUPPCState *env);
+uint64_t cpu_ppc_load_tbl (CPUPPCState *env);
 uint32_t cpu_ppc_load_tbu (CPUPPCState *env);
 void cpu_ppc_store_tbu (CPUPPCState *env, uint32_t value);
 void cpu_ppc_store_tbl (CPUPPCState *env, uint32_t value);
diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index e3bd29c..e7bcd71 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -68,7 +68,7 @@ void helper_store_dump_spr (uint32_t sprn)
 
 target_ulong helper_load_tbl (void)
 {
-return cpu_ppc_load_tbl(env);
+return (target_ulong)cpu_ppc_load_tbl(env);
 }
 
 target_ulong helper_load_tbu (void)
-- 
1.6.0.2

Re: [Qemu-devel] Re: SVM support in 0.12?

[Andre Przywara]

Andre Przywara wrote:

Jun Koi wrote:
I am running latest Qemu 0.12-rc. My guest VM runs Linux kernel 
2.6.31.


Because Qemu now supports SVM, I expect to see the SVM flag in
/proc/cpuinfo, but that is not the case.

So it seems SVM support is not enabled by default configuration??


My host and guest are both 32 bit Linux, if that matters.

(And this is pure Qemu, without using KVM or KQemu)

Kqemu actually works with svm emulation.


...

qemu -m 500 -cpu qemu32,+svm -cdrom ubuntu.iso

I verified that /proc/cpuinfo has no svm flag. So SVM doesnt work on 
32bit

host.

I will try that with 64bit host to see how it goes.
The host doesn't matter. You can easily run qemu-system-x86_64 on a 
32-bit

host.



I can confirm that SVM works well on x86-64 target, but fails on i386 
target.


The Linux kernel will only detect SVM if the machine is AMD (see 
linux-2.6/arch/x86/include/asm/virtext.h:cpu_has_svm())

So please try:
$ qemu -m 500 -cpu qemu32,+svm,vendor=AuthenticAMD -cdrom ubuntu.iso
(because the default vendor for qemu32 is Intel, for qemu64 AMD)


Should have checked this before the post ;-):
qemu32 has a xlevel of 0, so no AMD-defined CPUID leafs will be parsed.
Either fix this explicitly with xlevel=0xa or use athlon as your base 
CPU model:

$ qemu -m 500 -cpu athlon,+svm -cdrom ubuntu.iso

This made my Linux show the SVM flag.

Regards,
Andre.

--
Andre Przywara
AMD-OSRC (Dresden)
Tel: x29712

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Yaniv Kaul]

On 12/21/2009 1:12 PM, Avi Kivity wrote:

On 12/20/2009 07:18 PM, Michael S. Tsirkin wrote:



Hmm, then, shouldn't either kvm or qemu mask features that we do not
emulate well enough to make windows not crash?

-cpu host does that already, no?

Alex

I expected so, but Avi here seems to say windows will crash if you
use a new CPU with it ...


No, Windows tries to detect changes in your hardware and assumes that 
if too many things change, you might be a pirate and requires you to 
phone their offices to re-authenticate.




'Just' the CPU is not big deal. Might hit you with two 'bad' points.
Network interface + CPU will require re-activation.
( http://www.aumha.org/win5/a/wpa.php )
Y.

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Alexander Graf]

On 21.12.2009, at 12:38, Michael S. Tsirkin wrote:

 On Mon, Dec 21, 2009 at 12:22:53PM +0100, Alexander Graf wrote:
 
 On 21.12.2009, at 12:18, Michael S. Tsirkin wrote:
 
 On Mon, Dec 21, 2009 at 01:12:26PM +0200, Avi Kivity wrote:
 On 12/20/2009 07:18 PM, Michael S. Tsirkin wrote:
 
 Hmm, then, shouldn't either kvm or qemu mask features that we do not
 emulate well enough to make windows not crash?
 
 -cpu host does that already, no?
 
 Alex
 
 I expected so, but Avi here seems to say windows will crash if you
 use a new CPU with it ...
 
 
 No, Windows tries to detect changes in your hardware and assumes that if  
 too many things change, you might be a pirate and requires you to phone  
 their offices to re-authenticate.
 
 How often does a casual user upgrade his host CPU?
 
 I tend to have my VM images on an NFS share and expect them to work properly 
 on all PCs I work on.
 So I guess the answer is often?
 
 Alex
 
 Yes but you are not a casual user, are you?  

Well, we have two groups:

1) Casual user w/o management app
2) Enterprise user w/ management app

So I clearly belong to the first group.

 Consider a 64 bit guest to
 see why moving a VM across machines needs some planning.

-ENOPARSE

We're still talking about bootup on different machines, not migration / loadvm.

Alex

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Avi Kivity]

On 12/21/2009 01:18 PM, Michael S. Tsirkin wrote:



No, Windows tries to detect changes in your hardware and assumes that if
too many things change, you might be a pirate and requires you to phone
their offices to re-authenticate.
 

How often does a casual user upgrade his host CPU?
   


I don't know.  Does it matter?

--
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Michael S. Tsirkin]

On Mon, Dec 21, 2009 at 12:45:26PM +0100, Alexander Graf wrote:
 
 On 21.12.2009, at 12:38, Michael S. Tsirkin wrote:
 
  On Mon, Dec 21, 2009 at 12:22:53PM +0100, Alexander Graf wrote:
  
  On 21.12.2009, at 12:18, Michael S. Tsirkin wrote:
  
  On Mon, Dec 21, 2009 at 01:12:26PM +0200, Avi Kivity wrote:
  On 12/20/2009 07:18 PM, Michael S. Tsirkin wrote:
  
  Hmm, then, shouldn't either kvm or qemu mask features that we do not
  emulate well enough to make windows not crash?
  
  -cpu host does that already, no?
  
  Alex
  
  I expected so, but Avi here seems to say windows will crash if you
  use a new CPU with it ...
  
  
  No, Windows tries to detect changes in your hardware and assumes that if 
   
  too many things change, you might be a pirate and requires you to phone  
  their offices to re-authenticate.
  
  How often does a casual user upgrade his host CPU?
  
  I tend to have my VM images on an NFS share and expect them to work 
  properly on all PCs I work on.
  So I guess the answer is often?
  
  Alex
  
  Yes but you are not a casual user, are you?  
 
 Well, we have two groups:
 
 1) Casual user w/o management app
 2) Enterprise user w/ management app
 
 So I clearly belong to the first group.

No, you are in
3) virtualization hacker without management app
:)

  Consider a 64 bit guest to
  see why moving a VM across machines needs some planning.
 
 -ENOPARSE
 
 We're still talking about bootup on different machines, not migration / 
 loadvm.
 
 Alex

Translation: your requirement work on all PCs I work on might only be 
satisfied
by specifying the set of PCs you work on.

Bootup on different machines has some of the same issues as migration.
Consider a 64 bit guest as an example, I think it can not boot on a 32
bit host OS with kvm. I think you can use tcg, but it will be slower.
Same thing applies to other CPU features.

Many guests reconfigure themselves when you swap harware under them.
This makes it easier to move such guests between machines, or change
qemu configuration and still reuse guest image.  Others might record
hardware state on setup (64 bit above is one such example) making such
changes more painful.

I do think it would be good for us to supply management with utilities
that analyse system hardware features relevant to qemu in a portable
way. Could be some qemu monitor command, even.  Management would be able
to decide between using least common denominator and not supporting some
hosts.

-- 
MST

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Avi Kivity]

On 12/21/2009 01:45 PM, Alexander Graf wrote:


Well, we have two groups:

1) Casual user w/o management app
2) Enterprise user w/ management app

So I clearly belong to the first group.
   


3) Developer/power user who knows what he's about.

You could simply add -cpu qemu64 for those guests that care about it.

--
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Michael S. Tsirkin]

On Mon, Dec 21, 2009 at 02:04:47PM +0200, Avi Kivity wrote:
 On 12/21/2009 01:18 PM, Michael S. Tsirkin wrote:

 No, Windows tries to detect changes in your hardware and assumes that if
 too many things change, you might be a pirate and requires you to phone
 their offices to re-authenticate.
  
 How often does a casual user upgrade his host CPU?


 I don't know.  Does it matter?

I think so - if this does not happen a lot, it's not a problem to phone home,
right?

 -- 
 error compiling committee.c: too many arguments to function

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Avi Kivity]

On 12/21/2009 02:04 PM, Michael S. Tsirkin wrote:


I think so - if this does not happen a lot, it's not a problem to phone home,
right?
   


I'm sure it's very annoying when it happens.

--
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Michael S. Tsirkin]

On Mon, Dec 21, 2009 at 02:09:31PM +0200, Avi Kivity wrote:
 On 12/21/2009 02:04 PM, Michael S. Tsirkin wrote:

 I think so - if this does not happen a lot, it's not a problem to phone home,
 right?


 I'm sure it's very annoying when it happens.

It could well be less annoying than having your guest be slow every day
because qemu masked CPU features that guest needs to be fast.

At least with a CPU upgrade, casual user can easily disagnose the
problem: the system was changed. If qemu is slow by default, there's no
way to find out why without reading the manual.  And we don't have one
:)

 -- 
 error compiling committee.c: too many arguments to function

Re: [Qemu-devel] ne2k_isa: how to specify a custom iobase and irq?

[Markus Armbruster]

Sebastian Herbszt herb...@gmx.de writes:

 Anthony Liguori wrote:
 Sebastian Herbszt wrote:
 Markus Armbruster wrote:
 Sebastian Herbszt herb...@gmx.de writes:

 The default iobase and irq for the ne2k_isa card are 0x300 and 9.
 It should be possible to override both using the -net syntax like
 -net nic,model=ne2k_isa,irq=5,iobase=0x280.

 -device ne2k_isa,irq=5,iobase=0x280

 If i specify -net nic,model=pcnet i end up only with a pcnet nic. With
 the above syntax i get a e1000 and a ne2k_isa. It also loads the
 e1000 rom.

 Can you supply the full command lines.  I'm a little confused about
 what you're reporting.

 qemu -device ne2k_isa,irq=10 and info qtree has e1000 and ne2k_isa.
 qemu -net nic,model=pcnet and info qtree has only pcnet.

Unlike -net nic, -device doesn't suppress the default NIC.  Unfortunate.
You can get rid of it with -nodefaults, but that also rids you of other
default devices.

[...]

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Andre Przywara]

Avi Kivity wrote:

On 12/20/2009 07:59 PM, Anthony Liguori wrote:

Gleb Natapov wrote:
Windows is a mystery box, so we can speculate as much as we want 
about it.

If you don't like something just say it may break Windows :) Losing
activation does sound like an issue too.


Downgrading seems more likely to cause problems.  Considering running 
mplayer in a guest.  If it detects SSE3 in one host and migrate to 
another host that doesn't have SSE3, you'll be running an instruction 
stream that uses instructions the processor doesn't support resulting 
in the application faulting due to an illegal operation.




Migration needs preparation beforehand.  You can't take two random hosts 
with two random qemu flag sets and expect it to work.


KVM's cpuid filter doesn't help here because it won't attempt to mask 
things like sse3.  It would be insane to emulate sse3 too.


It does expose sse3 support (called pni in Linux IIRC).  Not sure if 
qemu masks it, but the information is there.

What about using -cpu kvm64?
This has SSE3 as well as CMPXCHG16, both are available in all VMX/SVM 
capable machines (as far as I could find out) and in TCG.
This gives a pretty descent base without loosing many relevant 
performance features.


... 
This is a classic management tool problem and the solution usually is 
a set of heuristics depending on how conservative the target audience is.


Right.  My concern is with casual users upgrading their machine, not 
enterprise users who should be protected by their tools.
Then what about fixing the CPUID bits to those returned by the KVM 
module the first time the guest is started? Later you would only use 
those bits (which may have been cropped) for later restarts of the same 
guest. If you only upgrade your machine, then there should be no problems.


Regards,
Andre.

P.S. What feature bits do we talk about?
Maybe we should group them and use aliases for those.
SS_S_E3 and SSE4.x come to mind, are any other bits really relevant? (Or 
do they justify the pain we have when propagating them?)

Then we would only need: Athlon64, Core2, Nehalem (and maybe Phenom).

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448 3567 12
to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Karl-Hammerschmidt-Str. 34, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Andrew Bowd; Thomas M. McCoy; Giuliano Meroni
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[Qemu-devel] [PATCH] PPC: Make DCR uint32_t

[Alexander Graf]

For what I know DCR is always 32 bits wide, so we should also use uint32_t to
pass it along the stacks.

This fixes a warning when compiling qemu-system-ppc64 with KVM enabled, making
it compile without --disable-werror

Signed-off-by: Alexander Graf ag...@suse.de
---
 darwin-user/main.c |4 ++--
 hw/ppc.c   |4 ++--
 hw/ppc.h   |4 ++--
 hw/ppc405_uc.c |   46 +++---
 hw/ppc4xx_devs.c   |   14 +++---
 linux-user/main.c  |4 ++--
 target-ppc/cpu.h   |4 ++--
 target-ppc/helper.h|4 ++--
 target-ppc/op_helper.c |   10 +-
 9 files changed, 47 insertions(+), 47 deletions(-)

diff --git a/darwin-user/main.c b/darwin-user/main.c
index 5fd314e..f97b215 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -113,12 +113,12 @@ uint32_t cpu_ppc601_load_rtcl (CPUState *env)
 }
 
 /* XXX: to be fixed */
-int ppc_dcr_read (ppc_dcr_t *dcr_env, int dcrn, target_ulong *valp)
+int ppc_dcr_read (ppc_dcr_t *dcr_env, int dcrn, uint32_t *valp)
 {
 return -1;
 }
 
-int ppc_dcr_write (ppc_dcr_t *dcr_env, int dcrn, target_ulong val)
+int ppc_dcr_write (ppc_dcr_t *dcr_env, int dcrn, uint32_t val)
 {
 return -1;
 }
diff --git a/hw/ppc.c b/hw/ppc.c
index 5208039..415b70b 100644
--- a/hw/ppc.c
+++ b/hw/ppc.c
@@ -1009,7 +1009,7 @@ struct ppc_dcr_t {
 int (*write_error)(int dcrn);
 };
 
-int ppc_dcr_read (ppc_dcr_t *dcr_env, int dcrn, target_ulong *valp)
+int ppc_dcr_read (ppc_dcr_t *dcr_env, int dcrn, uint32_t *valp)
 {
 ppc_dcrn_t *dcr;
 
@@ -1029,7 +1029,7 @@ int ppc_dcr_read (ppc_dcr_t *dcr_env, int dcrn, 
target_ulong *valp)
 return -1;
 }
 
-int ppc_dcr_write (ppc_dcr_t *dcr_env, int dcrn, target_ulong val)
+int ppc_dcr_write (ppc_dcr_t *dcr_env, int dcrn, uint32_t val)
 {
 ppc_dcrn_t *dcr;
 
diff --git a/hw/ppc.h b/hw/ppc.h
index 4b481af..bbf3a98 100644
--- a/hw/ppc.h
+++ b/hw/ppc.h
@@ -13,8 +13,8 @@ static inline void clk_setup (clk_setup_t *clk, uint32_t freq)
 
 clk_setup_cb cpu_ppc_tb_init (CPUState *env, uint32_t freq);
 /* Embedded PowerPC DCR management */
-typedef target_ulong (*dcr_read_cb)(void *opaque, int dcrn);
-typedef void (*dcr_write_cb)(void *opaque, int dcrn, target_ulong val);
+typedef uint32_t (*dcr_read_cb)(void *opaque, int dcrn);
+typedef void (*dcr_write_cb)(void *opaque, int dcrn, uint32_t val);
 int ppc_dcr_init (CPUState *env, int (*dcr_read_error)(int dcrn),
   int (*dcr_write_error)(int dcrn));
 int ppc_dcr_register (CPUState *env, int dcrn, void *opaque,
diff --git a/hw/ppc405_uc.c b/hw/ppc405_uc.c
index 052f902..bfcb791 100644
--- a/hw/ppc405_uc.c
+++ b/hw/ppc405_uc.c
@@ -107,10 +107,10 @@ struct ppc4xx_plb_t {
 uint32_t besr;
 };
 
-static target_ulong dcr_read_plb (void *opaque, int dcrn)
+static uint32_t dcr_read_plb (void *opaque, int dcrn)
 {
 ppc4xx_plb_t *plb;
-target_ulong ret;
+uint32_t ret;
 
 plb = opaque;
 switch (dcrn) {
@@ -132,7 +132,7 @@ static target_ulong dcr_read_plb (void *opaque, int dcrn)
 return ret;
 }
 
-static void dcr_write_plb (void *opaque, int dcrn, target_ulong val)
+static void dcr_write_plb (void *opaque, int dcrn, uint32_t val)
 {
 ppc4xx_plb_t *plb;
 
@@ -189,10 +189,10 @@ struct ppc4xx_pob_t {
 uint32_t besr[2];
 };
 
-static target_ulong dcr_read_pob (void *opaque, int dcrn)
+static uint32_t dcr_read_pob (void *opaque, int dcrn)
 {
 ppc4xx_pob_t *pob;
-target_ulong ret;
+uint32_t ret;
 
 pob = opaque;
 switch (dcrn) {
@@ -212,7 +212,7 @@ static target_ulong dcr_read_pob (void *opaque, int dcrn)
 return ret;
 }
 
-static void dcr_write_pob (void *opaque, int dcrn, target_ulong val)
+static void dcr_write_pob (void *opaque, int dcrn, uint32_t val)
 {
 ppc4xx_pob_t *pob;
 
@@ -410,10 +410,10 @@ enum {
 EBC0_CFGDATA = 0x013,
 };
 
-static target_ulong dcr_read_ebc (void *opaque, int dcrn)
+static uint32_t dcr_read_ebc (void *opaque, int dcrn)
 {
 ppc4xx_ebc_t *ebc;
-target_ulong ret;
+uint32_t ret;
 
 ebc = opaque;
 switch (dcrn) {
@@ -494,7 +494,7 @@ static target_ulong dcr_read_ebc (void *opaque, int dcrn)
 return ret;
 }
 
-static void dcr_write_ebc (void *opaque, int dcrn, target_ulong val)
+static void dcr_write_ebc (void *opaque, int dcrn, uint32_t val)
 {
 ppc4xx_ebc_t *ebc;
 
@@ -627,7 +627,7 @@ struct ppc405_dma_t {
 uint32_t pol;
 };
 
-static target_ulong dcr_read_dma (void *opaque, int dcrn)
+static uint32_t dcr_read_dma (void *opaque, int dcrn)
 {
 ppc405_dma_t *dma;
 
@@ -636,7 +636,7 @@ static target_ulong dcr_read_dma (void *opaque, int dcrn)
 return 0;
 }
 
-static void dcr_write_dma (void *opaque, int dcrn, target_ulong val)
+static void dcr_write_dma (void *opaque, int dcrn, uint32_t val)
 {
 ppc405_dma_t *dma;
 
@@ -914,10 +914,10 @@ static void ocm_update_mappings (ppc405_ocm_t *ocm,
 }
 }
 
-static target_ulong dcr_read_ocm (void *opaque, int dcrn)
+static 

[Qemu-devel] Re: cpuid problem in upstream qemu with kvm

[Paolo Bonzini]

On 12/21/2009 08:48 AM, Gleb Natapov wrote:

  Well, I wouldn't be too sure on that one. Software may use SSE3 instructions 
to access MMIO in which case we do have to emulate it.

Unfortunately this is already a reality. Look here:
http://groups.google.com/group/linux.kernel/browse_thread/thread/ada9fa2d2574a8af


Emulating 16-byte moves is a tad simpler than emulating the whole of 
SSE3 though. :-)


Paolo

[Qemu-devel] Re: cpuid problem in upstream qemu with kvm

[Paolo Bonzini]

No, Windows tries to detect changes in your hardware and assumes that
if too many things change, you might be a pirate and requires you to
phone their offices to re-authenticate.


'Just' the CPU is not big deal. Might hit you with two 'bad' points.
Network interface + CPU will require re-activation.


I think the problem is only for networked images.  For normal images, it 
would be the same if you were running Windows natively---upgrade CPU, 
and you risk having to do reactivation.


For networked images, does changing CPU twice count as changing one 
thing?  If so that's not too bad, not many things are likely to change 
in a VM beyond the CPU.


Besides, if you have networked images, it's relatively likely that 
you're doing it for a company, hence that you have some kind of MSDN 
subscription which grants you unlimited activations for a given product key.


Paolo

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[David S. Ahern]

On 12/21/2009 05:05 AM, Avi Kivity wrote:
 On 12/21/2009 01:45 PM, Alexander Graf wrote:

 Well, we have two groups:

 1) Casual user w/o management app
 2) Enterprise user w/ management app

 So I clearly belong to the first group.

 
 3) Developer/power user who knows what he's about.
 
 You could simply add -cpu qemu64 for those guests that care about it.
 

4) embedded virtualization where the use of a management app provides
little to no added benefit and everything has to be
automated (ie., no user).

My point is there are other use cases besides data center deployments
(aka enterprise) and workstation (casual/power user). There are use
cases where virtualization is just yet another tool to achieve a product.

David

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Michael S. Tsirkin]

On Mon, Dec 21, 2009 at 06:45:22AM -0700, David S. Ahern wrote:
 
 On 12/21/2009 05:05 AM, Avi Kivity wrote:
  On 12/21/2009 01:45 PM, Alexander Graf wrote:
 
  Well, we have two groups:
 
  1) Casual user w/o management app
  2) Enterprise user w/ management app
 
  So I clearly belong to the first group.
 
  
  3) Developer/power user who knows what he's about.
  
  You could simply add -cpu qemu64 for those guests that care about it.
  
 
 4) embedded virtualization where the use of a management app provides
 little to no added benefit and everything has to be
 automated (ie., no user).
 
 My point is there are other use cases besides data center deployments
 (aka enterprise) and workstation (casual/power user). There are use
 cases where virtualization is just yet another tool to achieve a product.
 
 David

Yes, but unless someone runs qemu directly, default value for any flag
does not matter much.

-- 
MST

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[David S. Ahern]

On 12/21/2009 06:51 AM, Michael S. Tsirkin wrote:
 On Mon, Dec 21, 2009 at 06:45:22AM -0700, David S. Ahern wrote:

 On 12/21/2009 05:05 AM, Avi Kivity wrote:
 On 12/21/2009 01:45 PM, Alexander Graf wrote:

 Well, we have two groups:

 1) Casual user w/o management app
 2) Enterprise user w/ management app

 So I clearly belong to the first group.


 3) Developer/power user who knows what he's about.

 You could simply add -cpu qemu64 for those guests that care about it.


 4) embedded virtualization where the use of a management app provides
 little to no added benefit and everything has to be
 automated (ie., no user).

 My point is there are other use cases besides data center deployments
 (aka enterprise) and workstation (casual/power user). There are use
 cases where virtualization is just yet another tool to achieve a product.

 David
 
 Yes, but unless someone runs qemu directly, default value for any flag
 does not matter much.
 

That's what I was getting at - direct invocation of qemu in an
automated sense without a libvirt layer. Defaults that do the right
thing on the host would be nice, with tweaking from the defaults only if
something extra is wanted.

David

Re: [Qemu-devel] [PATCH] PPC64: Fix timebase

[Aurelien Jarno]

On Mon, Dec 21, 2009 at 12:15:42PM +0100, Alexander Graf wrote:
 
 On 21.12.2009, at 10:24, Aurelien Jarno wrote:
 
  On Mon, Dec 21, 2009 at 01:22:12AM +0100, Alexander Graf wrote:
  On PPC we have a 64-bit time base. Usually (PPC32) this is accessed using
  two separate 32 bit SPR accesses to SPR_TBU and SPR_TBL.
  
  On PPC64 the SPR_TBL register acts as 64 bit though, so we get the full
  64 bits as return value. If we only take the lower ones, fine. But Linux
  wants to see all 64 bits or it breaks.
  
  Good catch! However, I think this patch it's not fully complete and can
  be improved a bit
  - it's probably better to return a target_ulong value from
   cpu_ppc_load_tbl() with an explicit cast here, so that we don't have
   an implicit cast from 64-bit to 32-bit on qemu-system-powerpc (GCC may
   warn on that with some flags or in future versions).
  - the store function also has to be fixed.
  - the same changes should be done for the alternate timebase.

They are defined in the Book II, and corresponds to atbl and atbu
functions.

 Uuuh:
 
 __attribute__ (( unused ))
 static void spr_read_atbl (void *opaque, int gprn, int sprn)
 {
 gen_helper_load_atbl(cpu_gpr[gprn]);
 }
 
 And that attribute is correct. There is no caller.
 

Ok. I have committed a fix anyway, so that if someone enable it later,
he/she doesn't spend to much time fixing the bug.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

[Qemu-devel] [PATCHv2-RFC 0/2] qemu: properties for feature compatibility

[Michael S. Tsirkin]

Here's what I came up with for solving the problem of differences in
features in virtio between 0.12 and 0.11.  This also enables migration
between different backends, e.g.  between host where tap supports virtio
net header and where it does not: management just needs to set features
appropriately.

Changes since RFC:
- add symbolic names for properties
- only optional features can be changed
Comments? Gerd, what do you think?



Michael S. Tsirkin (2):
  qdev: add bit property type
  virtio: add features as qdev properties

 hw/qdev-properties.c |   70 +-
 hw/qdev.h|9 ++
 hw/syborg_virtio.c   |   12 +---
 hw/virtio-balloon.c  |4 +-
 hw/virtio-blk.c  |   10 ++-
 hw/virtio-blk.h  |   10 +++
 hw/virtio-console.c  |4 +-
 hw/virtio-net.c  |   39 +++
 hw/virtio-net.h  |   20 ++
 hw/virtio-pci.c  |   25 -
 hw/virtio.c  |2 +-
 hw/virtio.h  |7 -
 12 files changed, 156 insertions(+), 56 deletions(-)

[Qemu-devel] [PATCHv2-RFC 1/2] qdev: add bit property type

[Michael S. Tsirkin]

This adds bit property type, which is a boolean stored in a 32 bit
integer field, with legal values on and off.  Will be used by virtio for
feature bits.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/qdev-properties.c |   70 +-
 hw/qdev.h|9 ++
 2 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index fb07279..3782609 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -9,6 +9,67 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
 return ptr;
 }
 
+static void *qdev_bit_prop_ptr(DeviceState *dev, Property *prop)
+{
+void *ptr = dev;
+assert(prop-info-type == PROP_TYPE_BIT);
+ptr += prop-offset / 32;
+return ptr;
+}
+
+static uint32_t qdev_bit_prop_mask(Property *prop)
+{
+assert(prop-info-type == PROP_TYPE_BIT);
+return 0x1  (prop-offset % 32);
+}
+
+static void bit_prop_set(DeviceState *dev, Property *props, bool val)
+{
+uint32_t *p = qdev_bit_prop_ptr(dev, props);
+uint32_t mask = qdev_bit_prop_mask(props);
+if (val)
+*p |= ~mask;
+else
+*p = ~mask;
+}
+
+static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src)
+{
+if (props-info-type == PROP_TYPE_BIT) {
+bool *defval = src;
+bit_prop_set(dev, props, *defval);
+} else {
+char *dst = qdev_get_prop_ptr(dev, props);
+memcpy(dst, src, props-info-size);
+}
+}
+
+/* Bit */
+static int parse_bit(DeviceState *dev, Property *prop, const char *str)
+{
+if (!strncasecmp(str, on, 2))
+bit_prop_set(dev, prop, true);
+else if (!strncasecmp(str, off, 3))
+bit_prop_set(dev, prop, false);
+else
+return -1;
+return 0;
+}
+
+static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+uint8_t *p = qdev_bit_prop_ptr(dev, prop);
+return snprintf(dest, len, (*p  qdev_bit_prop_mask(prop)) ? on : off);
+}
+
+PropertyInfo qdev_prop_bit = {
+.name  = on/off,
+.type  = PROP_TYPE_BIT,
+.size  = sizeof(uint32_t),
+.parse = parse_bit,
+.print = print_bit,
+};
+
 /* --- 8bit integer --- */
 
 static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
@@ -506,7 +567,6 @@ int qdev_prop_parse(DeviceState *dev, const char *name, 
const char *value)
 void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum 
PropertyType type)
 {
 Property *prop;
-void *dst;
 
 prop = qdev_prop_find(dev, name);
 if (!prop) {
@@ -519,8 +579,7 @@ void qdev_prop_set(DeviceState *dev, const char *name, void 
*src, enum PropertyT
 __FUNCTION__, dev-info-name, name);
 abort();
 }
-dst = qdev_get_prop_ptr(dev, prop);
-memcpy(dst, src, prop-info-size);
+qdev_prop_cpy(dev, prop, src);
 }
 
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
@@ -580,14 +639,11 @@ void qdev_prop_set_ptr(DeviceState *dev, const char 
*name, void *value)
 
 void qdev_prop_set_defaults(DeviceState *dev, Property *props)
 {
-char *dst;
-
 if (!props)
 return;
 while (props-name) {
 if (props-defval) {
-dst = qdev_get_prop_ptr(dev, props);
-memcpy(dst, props-defval, props-info-size);
+qdev_prop_cpy(dev, props, props-defval);
 }
 props++;
 }
diff --git a/hw/qdev.h b/hw/qdev.h
index bbcdba1..26853e0 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -82,6 +82,7 @@ enum PropertyType {
 PROP_TYPE_NETDEV,
 PROP_TYPE_VLAN,
 PROP_TYPE_PTR,
+PROP_TYPE_BIT,
 };
 
 struct PropertyInfo {
@@ -173,6 +174,7 @@ void do_device_del(Monitor *mon, const QDict *qdict);
 
 /*** qdev-properties.c ***/
 
+extern PropertyInfo qdev_prop_bit;
 extern PropertyInfo qdev_prop_uint8;
 extern PropertyInfo qdev_prop_uint16;
 extern PropertyInfo qdev_prop_uint32;
@@ -202,6 +204,13 @@ extern PropertyInfo qdev_prop_pci_devfn;
 + type_check(_type,typeof_field(_state, _field)),   \
 .defval= (_type[]) { _defval }, \
 }
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) { \
+.name  = (_name), \
+.info  = (qdev_prop_bit),\
+.offset= offsetof(_state, _field) * 32 + (_bit)\
++ type_check(uint32_t,typeof_field(_state, _field)), \
+.defval= (bool[]) { (_defval) }, \
+}
 
 #define DEFINE_PROP_UINT8(_n, _s, _f, _d)   \
 DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
-- 
1.6.6.rc1.43.gf55cc

[Qemu-devel] [PATCHv2-RFC 2/2] virtio: add features as qdev properties

[Michael S. Tsirkin]

Add feature bits as properties to virtio. This makes it possible to e.g. define
machine without indirect buffer support, which is required for 0.10
compatibility, or without hardware checksum support, which is required for 0.11
compatibility.  Since default values for optional features are now set by qdev,
get_features callback has been modified: it sets non-optional bits, and clears
bits not supported by host.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/syborg_virtio.c  |   12 +++-
 hw/virtio-balloon.c |4 ++--
 hw/virtio-blk.c |   10 +++---
 hw/virtio-blk.h |   10 ++
 hw/virtio-console.c |4 ++--
 hw/virtio-net.c |   39 ---
 hw/virtio-net.h |   20 
 hw/virtio-pci.c |   25 +
 hw/virtio.c |2 +-
 hw/virtio.h |7 ++-
 10 files changed, 84 insertions(+), 49 deletions(-)

diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index fe6fc23..b3c2734 100644
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -66,6 +66,7 @@ typedef struct {
 uint32_t int_enable;
 uint32_t id;
 NICConf nic;
+uint32_t host_features;
 } SyborgVirtIOProxy;
 
 static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset)
@@ -86,8 +87,7 @@ static uint32_t syborg_virtio_readl(void *opaque, 
target_phys_addr_t offset)
 ret = s-id;
 break;
 case SYBORG_VIRTIO_HOST_FEATURES:
-ret = vdev-get_features(vdev);
-ret |= vdev-binding-get_features(s);
+   ret = s-host_features;
 break;
 case SYBORG_VIRTIO_GUEST_FEATURES:
 ret = vdev-guest_features;
@@ -244,9 +244,8 @@ static void syborg_virtio_update_irq(void *opaque, uint16_t 
vector)
 
 static unsigned syborg_virtio_get_features(void *opaque)
 {
-unsigned ret = 0;
-ret |= (1  VIRTIO_F_NOTIFY_ON_EMPTY);
-return ret;
+SyborgVirtIOProxy *proxy = opaque;
+return proxy-host_features;
 }
 
 static VirtIOBindings syborg_virtio_bindings = {
@@ -272,6 +271,8 @@ static int syborg_virtio_init(SyborgVirtIOProxy *proxy, 
VirtIODevice *vdev)
 qemu_register_reset(virtio_reset, vdev);
 
 virtio_bind_device(vdev, syborg_virtio_bindings, proxy);
+proxy-host_features |= (0x1  VIRTIO_F_NOTIFY_ON_EMPTY);
+proxy-host_features = vdev-get_features(vdev, proxy-host_features);
 return 0;
 }
 
@@ -292,6 +293,7 @@ static SysBusDeviceInfo syborg_virtio_net_info = {
 .qdev.size  = sizeof(SyborgVirtIOProxy),
 .qdev.props = (Property[]) {
 DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic),
+DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features),
 DEFINE_PROP_END_OF_LIST(),
 }
 };
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index cfd3b41..e17880f 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -124,9 +124,9 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
 dev-actual = config.actual;
 }
 
-static uint32_t virtio_balloon_get_features(VirtIODevice *vdev)
+static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 {
-return 0;
+return f;
 }
 
 static ram_addr_t virtio_balloon_to_target(void *opaque, ram_addr_t target)
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index a2f0639..72970ef 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -432,19 +432,15 @@ static void virtio_blk_update_config(VirtIODevice *vdev, 
uint8_t *config)
 memcpy(config, blkcfg, s-config_size);
 }
 
-static uint32_t virtio_blk_get_features(VirtIODevice *vdev)
+static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
 {
 VirtIOBlock *s = to_virtio_blk(vdev);
-uint32_t features = 0;
 
 features |= (1  VIRTIO_BLK_F_SEG_MAX);
 features |= (1  VIRTIO_BLK_F_GEOMETRY);
 
-if (bdrv_enable_write_cache(s-bs))
-features |= (1  VIRTIO_BLK_F_WCACHE);
-#ifdef __linux__
-features |= (1  VIRTIO_BLK_F_SCSI);
-#endif
+if (!bdrv_enable_write_cache(s-bs))
+features = ~(1  VIRTIO_BLK_F_WCACHE);
 if (strcmp(s-serial_str, 0))
 features |= 1  VIRTIO_BLK_F_IDENTIFY;
 
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index 23ad74c..3dee5c2 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -92,4 +92,14 @@ struct virtio_scsi_inhdr
 uint32_t residual;
 };
 
+#ifdef __linux__
+#define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
+DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
+DEFINE_PROP_BIT(scsi, _state, _field, VIRTIO_BLK_F_SCSI, true), \
+DEFINE_PROP_BIT(wcache, _state, _field, VIRTIO_BLK_F_WCACHE, true)
+#else
+#define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
+DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
+DEFINE_PROP_BIT(wcache, _state, _field, VIRTIO_BLK_F_WCACHE, true)
+#endif
 #endif
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 57f8f89..4f18ef2 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -51,9 +51,9 @@ 

Re: [Qemu-devel] [PATCH] PPC64: Fix timebase

[Alexander Graf]

Aurelien Jarno wrote:
 On Mon, Dec 21, 2009 at 12:15:42PM +0100, Alexander Graf wrote:
   
 On 21.12.2009, at 10:24, Aurelien Jarno wrote:

 
 On Mon, Dec 21, 2009 at 01:22:12AM +0100, Alexander Graf wrote:
   
 On PPC we have a 64-bit time base. Usually (PPC32) this is accessed using
 two separate 32 bit SPR accesses to SPR_TBU and SPR_TBL.

 On PPC64 the SPR_TBL register acts as 64 bit though, so we get the full
 64 bits as return value. If we only take the lower ones, fine. But Linux
 wants to see all 64 bits or it breaks.
 
 Good catch! However, I think this patch it's not fully complete and can
 be improved a bit
 - it's probably better to return a target_ulong value from
  cpu_ppc_load_tbl() with an explicit cast here, so that we don't have
  an implicit cast from 64-bit to 32-bit on qemu-system-powerpc (GCC may
  warn on that with some flags or in future versions).
 - the store function also has to be fixed.
 - the same changes should be done for the alternate timebase.
   

 They are defined in the Book II, and corresponds to atbl and atbu
 functions.

   
 Uuuh:

 __attribute__ (( unused ))
 static void spr_read_atbl (void *opaque, int gprn, int sprn)
 {
 gen_helper_load_atbl(cpu_gpr[gprn]);
 }

 And that attribute is correct. There is no caller.

 

 Ok. I have committed a fix anyway, so that if someone enable it later,
 he/she doesn't spend to much time fixing the bug.
   

Thanks :-).

Alex

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Avi Kivity]

On 12/21/2009 03:45 PM, David S. Ahern wrote:

On 12/21/2009 05:05 AM, Avi Kivity wrote:
   

On 12/21/2009 01:45 PM, Alexander Graf wrote:
 

Well, we have two groups:

1) Casual user w/o management app
2) Enterprise user w/ management app

So I clearly belong to the first group.

   

3) Developer/power user who knows what he's about.

You could simply add -cpu qemu64 for those guests that care about it.

 

4) embedded virtualization where the use of a management app provides
little to no added benefit and everything has to be
automated (ie., no user).

My point is there are other use cases besides data center deployments
(aka enterprise) and workstation (casual/power user). There are use
cases where virtualization is just yet another tool to achieve a product.
   


If there is no user, then a management app exists.  It may be as simple 
as execing qemu from initrd, but there is still something that is not a 
user that can set the correct parameters.  It is up to the author of 
this one-line management app to decide what the correct parameters are 
(-cpu host or some other cpu baseline that allows VM portability, if 
needed).


--
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[Avi Kivity]

On 12/21/2009 02:59 PM, Andre Przywara wrote:


KVM's cpuid filter doesn't help here because it won't attempt to 
mask things like sse3.  It would be insane to emulate sse3 too.


It does expose sse3 support (called pni in Linux IIRC).  Not sure if 
qemu masks it, but the information is there.


What about using -cpu kvm64?
This has SSE3 as well as CMPXCHG16, both are available in all VMX/SVM 
capable machines (as far as I could find out) and in TCG.
This gives a pretty descent base without loosing many relevant 
performance features.


Sure.  This gives more performance to users and preserves compatibility 
pretty well.  The disadvantage is that the baseline will recede as more 
processor features are introduced.



...
This is a classic management tool problem and the solution usually 
is a set of heuristics depending on how conservative the target 
audience is.


Right.  My concern is with casual users upgrading their machine, not 
enterprise users who should be protected by their tools.
Then what about fixing the CPUID bits to those returned by the KVM 
module the first time the guest is started? Later you would only use 
those bits (which may have been cropped) for later restarts of the 
same guest. If you only upgrade your machine, then there should be no 
problems.


First, even an upgrade may cause loss of cpuid bits (moving to a 
different vendor).  Second, where do you store those bits?




P.S. What feature bits do we talk about?
Maybe we should group them and use aliases for those.
SS_S_E3 and SSE4.x come to mind, are any other bits really relevant? 
(Or do they justify the pain we have when propagating them?)

Then we would only need: Athlon64, Core2, Nehalem (and maybe Phenom).


Future bits too, for example AVX.

--
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: stable-0.12 versus master branching?

[Michael S. Tsirkin]

On Sun, Dec 20, 2009 at 08:35:53AM -0600, Anthony Liguori wrote:
 Michael S. Tsirkin wrote:
 I am really confused wrt stable 0.12 branching policy.
 For example I see this, in master:

  commit d587e0787153f0224a6140c5015609963ceaabfb
  Author: Anthony Liguori aligu...@us.ibm.com
  Date:   Mon Dec 14 11:36:53 2009 -0600

  Revert pci: interrupt disable bit support

  This reverts commit 0ea5709a32085f7d14901a09d12bd35f9b267607.

  Per discussion with Michael Tsirkin, this is too risky for 0.12

  Signed-off-by: Anthony Liguori aligu...@us.ibm.com

 (this was cherry-picked in stable-0.12 as well).
 From the comment, this revert makes sense for stable-0.12
 but not for master.  Is this an attempt to keep stable and master
 as close as possible? I expected master development to proceed while
 stable-0.12 would get just bugfixes. No?
   

 It was a mistake.  I've already got it recommitted in master.  Just need  
 to push.

 Regards,

 Anthony Liguori

 Thanks,


I see. So I think we should revert
f9bf77dd1f838b0061172fe41709b221956da2f5
on stable-0.12 as well: interrupt disable and interrupt
status appeared in the same spec revision,
so it might surprise guests to see one but not the other.

-- 
MST

[Qemu-devel] [PATCHv3-RFC 0/2] qemu: properties for feature compatibility

[Michael S. Tsirkin]

Here's what I came up with for solving the problem of differences in
features in virtio between 0.12 and 0.11.  This also enables migration
between different backends, e.g.  between host where tap supports virtio
net header and where it does not: management just needs to set features
appropriately.

Changes since RFC v2:
- we already have a flag to control write cache feature,
  let's not add another one

Changes since RFC:
- add symbolic names for properties
- only optional features can be changed
Comments? Gerd, what do you think?



Michael S. Tsirkin (2):
  qdev: add bit property type
  virtio: add features as qdev properties

 hw/qdev-properties.c |   70 +-
 hw/qdev.h|9 ++
 hw/syborg_virtio.c   |   12 +---
 hw/virtio-balloon.c  |4 +-
 hw/virtio-blk.c  |6 +---
 hw/virtio-blk.h  |8 +
 hw/virtio-console.c  |4 +-
 hw/virtio-net.c  |   39 +++
 hw/virtio-net.h  |   20 ++
 hw/virtio-pci.c  |   25 -
 hw/virtio.c  |2 +-
 hw/virtio.h  |7 -
 12 files changed, 152 insertions(+), 54 deletions(-)

[Qemu-devel] [PATCHv3-RFC 1/2] qdev: add bit property type

[Michael S. Tsirkin]

This adds bit property type, which is a boolean stored in a 32 bit
integer field, with legal values on and off.  Will be used by virtio for
feature bits.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/qdev-properties.c |   70 +-
 hw/qdev.h|9 ++
 2 files changed, 72 insertions(+), 7 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index fb07279..3782609 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -9,6 +9,67 @@ void *qdev_get_prop_ptr(DeviceState *dev, Property *prop)
 return ptr;
 }
 
+static void *qdev_bit_prop_ptr(DeviceState *dev, Property *prop)
+{
+void *ptr = dev;
+assert(prop-info-type == PROP_TYPE_BIT);
+ptr += prop-offset / 32;
+return ptr;
+}
+
+static uint32_t qdev_bit_prop_mask(Property *prop)
+{
+assert(prop-info-type == PROP_TYPE_BIT);
+return 0x1  (prop-offset % 32);
+}
+
+static void bit_prop_set(DeviceState *dev, Property *props, bool val)
+{
+uint32_t *p = qdev_bit_prop_ptr(dev, props);
+uint32_t mask = qdev_bit_prop_mask(props);
+if (val)
+*p |= ~mask;
+else
+*p = ~mask;
+}
+
+static void qdev_prop_cpy(DeviceState *dev, Property *props, void *src)
+{
+if (props-info-type == PROP_TYPE_BIT) {
+bool *defval = src;
+bit_prop_set(dev, props, *defval);
+} else {
+char *dst = qdev_get_prop_ptr(dev, props);
+memcpy(dst, src, props-info-size);
+}
+}
+
+/* Bit */
+static int parse_bit(DeviceState *dev, Property *prop, const char *str)
+{
+if (!strncasecmp(str, on, 2))
+bit_prop_set(dev, prop, true);
+else if (!strncasecmp(str, off, 3))
+bit_prop_set(dev, prop, false);
+else
+return -1;
+return 0;
+}
+
+static int print_bit(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+uint8_t *p = qdev_bit_prop_ptr(dev, prop);
+return snprintf(dest, len, (*p  qdev_bit_prop_mask(prop)) ? on : off);
+}
+
+PropertyInfo qdev_prop_bit = {
+.name  = on/off,
+.type  = PROP_TYPE_BIT,
+.size  = sizeof(uint32_t),
+.parse = parse_bit,
+.print = print_bit,
+};
+
 /* --- 8bit integer --- */
 
 static int parse_uint8(DeviceState *dev, Property *prop, const char *str)
@@ -506,7 +567,6 @@ int qdev_prop_parse(DeviceState *dev, const char *name, 
const char *value)
 void qdev_prop_set(DeviceState *dev, const char *name, void *src, enum 
PropertyType type)
 {
 Property *prop;
-void *dst;
 
 prop = qdev_prop_find(dev, name);
 if (!prop) {
@@ -519,8 +579,7 @@ void qdev_prop_set(DeviceState *dev, const char *name, void 
*src, enum PropertyT
 __FUNCTION__, dev-info-name, name);
 abort();
 }
-dst = qdev_get_prop_ptr(dev, prop);
-memcpy(dst, src, prop-info-size);
+qdev_prop_cpy(dev, prop, src);
 }
 
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
@@ -580,14 +639,11 @@ void qdev_prop_set_ptr(DeviceState *dev, const char 
*name, void *value)
 
 void qdev_prop_set_defaults(DeviceState *dev, Property *props)
 {
-char *dst;
-
 if (!props)
 return;
 while (props-name) {
 if (props-defval) {
-dst = qdev_get_prop_ptr(dev, props);
-memcpy(dst, props-defval, props-info-size);
+qdev_prop_cpy(dev, props, props-defval);
 }
 props++;
 }
diff --git a/hw/qdev.h b/hw/qdev.h
index bbcdba1..26853e0 100644
--- a/hw/qdev.h
+++ b/hw/qdev.h
@@ -82,6 +82,7 @@ enum PropertyType {
 PROP_TYPE_NETDEV,
 PROP_TYPE_VLAN,
 PROP_TYPE_PTR,
+PROP_TYPE_BIT,
 };
 
 struct PropertyInfo {
@@ -173,6 +174,7 @@ void do_device_del(Monitor *mon, const QDict *qdict);
 
 /*** qdev-properties.c ***/
 
+extern PropertyInfo qdev_prop_bit;
 extern PropertyInfo qdev_prop_uint8;
 extern PropertyInfo qdev_prop_uint16;
 extern PropertyInfo qdev_prop_uint32;
@@ -202,6 +204,13 @@ extern PropertyInfo qdev_prop_pci_devfn;
 + type_check(_type,typeof_field(_state, _field)),   \
 .defval= (_type[]) { _defval }, \
 }
+#define DEFINE_PROP_BIT(_name, _state, _field, _bit, _defval) { \
+.name  = (_name), \
+.info  = (qdev_prop_bit),\
+.offset= offsetof(_state, _field) * 32 + (_bit)\
++ type_check(uint32_t,typeof_field(_state, _field)), \
+.defval= (bool[]) { (_defval) }, \
+}
 
 #define DEFINE_PROP_UINT8(_n, _s, _f, _d)   \
 DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_uint8, uint8_t)
-- 
1.6.6.rc1.43.gf55cc

[Qemu-devel] [PATCHv3-RFC 2/2] virtio: add features as qdev properties

[Michael S. Tsirkin]

Add feature bits as properties to virtio. This makes it possible to e.g. define
machine without indirect buffer support, which is required for 0.10
compatibility, or without hardware checksum support, which is required for 0.11
compatibility.  Since default values for optional features are now set by qdev,
get_features callback has been modified: it sets non-optional bits, and clears
bits not supported by host.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/syborg_virtio.c  |   12 +++-
 hw/virtio-balloon.c |4 ++--
 hw/virtio-blk.c |6 +-
 hw/virtio-blk.h |8 
 hw/virtio-console.c |4 ++--
 hw/virtio-net.c |   39 ---
 hw/virtio-net.h |   20 
 hw/virtio-pci.c |   25 +
 hw/virtio.c |2 +-
 hw/virtio.h |7 ++-
 10 files changed, 80 insertions(+), 47 deletions(-)

diff --git a/hw/syborg_virtio.c b/hw/syborg_virtio.c
index fe6fc23..b3c2734 100644
--- a/hw/syborg_virtio.c
+++ b/hw/syborg_virtio.c
@@ -66,6 +66,7 @@ typedef struct {
 uint32_t int_enable;
 uint32_t id;
 NICConf nic;
+uint32_t host_features;
 } SyborgVirtIOProxy;
 
 static uint32_t syborg_virtio_readl(void *opaque, target_phys_addr_t offset)
@@ -86,8 +87,7 @@ static uint32_t syborg_virtio_readl(void *opaque, 
target_phys_addr_t offset)
 ret = s-id;
 break;
 case SYBORG_VIRTIO_HOST_FEATURES:
-ret = vdev-get_features(vdev);
-ret |= vdev-binding-get_features(s);
+   ret = s-host_features;
 break;
 case SYBORG_VIRTIO_GUEST_FEATURES:
 ret = vdev-guest_features;
@@ -244,9 +244,8 @@ static void syborg_virtio_update_irq(void *opaque, uint16_t 
vector)
 
 static unsigned syborg_virtio_get_features(void *opaque)
 {
-unsigned ret = 0;
-ret |= (1  VIRTIO_F_NOTIFY_ON_EMPTY);
-return ret;
+SyborgVirtIOProxy *proxy = opaque;
+return proxy-host_features;
 }
 
 static VirtIOBindings syborg_virtio_bindings = {
@@ -272,6 +271,8 @@ static int syborg_virtio_init(SyborgVirtIOProxy *proxy, 
VirtIODevice *vdev)
 qemu_register_reset(virtio_reset, vdev);
 
 virtio_bind_device(vdev, syborg_virtio_bindings, proxy);
+proxy-host_features |= (0x1  VIRTIO_F_NOTIFY_ON_EMPTY);
+proxy-host_features = vdev-get_features(vdev, proxy-host_features);
 return 0;
 }
 
@@ -292,6 +293,7 @@ static SysBusDeviceInfo syborg_virtio_net_info = {
 .qdev.size  = sizeof(SyborgVirtIOProxy),
 .qdev.props = (Property[]) {
 DEFINE_NIC_PROPERTIES(SyborgVirtIOProxy, nic),
+DEFINE_VIRTIO_NET_FEATURES(SyborgVirtIOProxy, host_features),
 DEFINE_PROP_END_OF_LIST(),
 }
 };
diff --git a/hw/virtio-balloon.c b/hw/virtio-balloon.c
index cfd3b41..e17880f 100644
--- a/hw/virtio-balloon.c
+++ b/hw/virtio-balloon.c
@@ -124,9 +124,9 @@ static void virtio_balloon_set_config(VirtIODevice *vdev,
 dev-actual = config.actual;
 }
 
-static uint32_t virtio_balloon_get_features(VirtIODevice *vdev)
+static uint32_t virtio_balloon_get_features(VirtIODevice *vdev, uint32_t f)
 {
-return 0;
+return f;
 }
 
 static ram_addr_t virtio_balloon_to_target(void *opaque, ram_addr_t target)
diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index a2f0639..cb1ae1f 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -432,19 +432,15 @@ static void virtio_blk_update_config(VirtIODevice *vdev, 
uint8_t *config)
 memcpy(config, blkcfg, s-config_size);
 }
 
-static uint32_t virtio_blk_get_features(VirtIODevice *vdev)
+static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
 {
 VirtIOBlock *s = to_virtio_blk(vdev);
-uint32_t features = 0;
 
 features |= (1  VIRTIO_BLK_F_SEG_MAX);
 features |= (1  VIRTIO_BLK_F_GEOMETRY);
 
 if (bdrv_enable_write_cache(s-bs))
 features |= (1  VIRTIO_BLK_F_WCACHE);
-#ifdef __linux__
-features |= (1  VIRTIO_BLK_F_SCSI);
-#endif
 if (strcmp(s-serial_str, 0))
 features |= 1  VIRTIO_BLK_F_IDENTIFY;
 
diff --git a/hw/virtio-blk.h b/hw/virtio-blk.h
index 23ad74c..c28f776 100644
--- a/hw/virtio-blk.h
+++ b/hw/virtio-blk.h
@@ -92,4 +92,12 @@ struct virtio_scsi_inhdr
 uint32_t residual;
 };
 
+#ifdef __linux__
+#define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
+DEFINE_VIRTIO_COMMON_FEATURES(_state, _field), \
+DEFINE_PROP_BIT(scsi, _state, _field, VIRTIO_BLK_F_SCSI, true)
+#else
+#define DEFINE_VIRTIO_BLK_FEATURES(_state, _field) \
+DEFINE_VIRTIO_COMMON_FEATURES(_state, _field)
+#endif
 #endif
diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index 57f8f89..4f18ef2 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -51,9 +51,9 @@ static void virtio_console_handle_input(VirtIODevice *vdev, 
VirtQueue *vq)
 {
 }
 
-static uint32_t virtio_console_get_features(VirtIODevice *vdev)
+static uint32_t virtio_console_get_features(VirtIODevice *vdev, uint32_t f)
 {
-return 0;
+return 

Re: [Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loading overhaul.

[Gleb Natapov]

On Mon, Dec 21, 2009 at 10:40:56AM -0600, Anthony Liguori wrote:
 On 12/21/2009 01:32 AM, Gleb Natapov wrote:
 On Sun, Dec 20, 2009 at 08:59:48PM -0500, Kevin O'Connor wrote:
 On Sun, Dec 20, 2009 at 11:48:20AM -0600, Anthony Liguori wrote:
 I think we have two ways to view firmware.  The first would be to treat
 guest firmware as part of the guest.  What that means it that we should
 store all firmware in an nvram file, migrate the nvram file during
 migration
 [...]
 The other option would be to treat guest firmware as part of the machine
 state.
 How about mixing the two?  Store the firmware in an nvram file and
 migrate it during migration, but clear the nvram and reload the
 firmware on each start-up and qemu reset.
 
 That's precisely what I propose.
 
 There are some really ugly corner cases here.  For instance, guest
 is running and the user does a yum update which upgrades the qemu
 package.  This includes laying down a new bios.
 
 User eventually restarts guest, now we re-read BIOS and we're on a
 newer BIOS than the device model.  Badness ensues.
 
My package manager warns me that certain application need to be
restarted to work correctly after upgrade. This is hardly qemu specific
problem.

 And more importantly, what is the end-user benefit of doing this?
 
Working migration?

--
Gleb.

[Qemu-devel] Re: [PATCH] PPC: Make DCR uint32_t

[Hollis Blanchard]

On Mon, Dec 21, 2009 at 5:02 AM, Alexander Graf ag...@suse.de wrote:
 For what I know DCR is always 32 bits wide, so we should also use uint32_t to
 pass it along the stacks.

 This fixes a warning when compiling qemu-system-ppc64 with KVM enabled, making
 it compile without --disable-werror

 Signed-off-by: Alexander Graf ag...@suse.de

Acked-by: Hollis Blanchard hol...@penguinppc.org

Re: [Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loading overhaul.

[Anthony Liguori]

On 12/21/2009 10:43 AM, Gleb Natapov wrote:

There are some really ugly corner cases here.  For instance, guest
is running and the user does a yum update which upgrades the qemu
package.  This includes laying down a new bios.

User eventually restarts guest, now we re-read BIOS and we're on a
newer BIOS than the device model.  Badness ensues.

 

My package manager warns me that certain application need to be
restarted to work correctly after upgrade. This is hardly qemu specific
problem.
   


But again, I don't see when this is ever a feature that a user actually 
wants.  Unless you change restart to fork/exec+exit, you'll never have 
reset equivalent to power off + startup.  Can you advocate rereading 
roms and not advocate re-execing qemu?



And more importantly, what is the end-user benefit of doing this?

 

Working migration?
   


How does it fix migration?  Migration needs to transfer the current roms 
in order to work.  A new version of qemu must support interacting with 
the old version of the firmware for migration to work.  What happens 
after reset has nothing to do with migration but because of the last 
requirement, the guest will obviously continue to work after reboot too.


Regards,

Anthony Liguori

[Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loading overhaul.

[Michael S. Tsirkin]

On Mon, Dec 21, 2009 at 09:40:47AM +0200, Gleb Natapov wrote:
 On Sun, Dec 20, 2009 at 11:48:20AM -0600, Anthony Liguori wrote:
  system_reset _is_ hard shutdown/start-up. If it is not it is a bug, we
  just arguing if the same applies for the case that migration was done
  between boot and reset.
  
  It's not and it will never be completely.
 There are always bugs left. As long as we agree on what is right that is
 OK.
 
 By this logic, we should
  close the file descriptors for the block devices and try to reopen
  them.  It's certainly possible that someone does a mv of the block
  device and replaces it under the covers.
 What is the analogue in real HW? BTW it is not a bad idea to get
 notification when disk file has changed and issue change device
 automatically.
 
  
  Likewise, if a user upgrades the firmware independently of the qemu
  version, it would need to reread the firmware from disk each boot.
  
 That's the idea. Qemu should reread firmware from disk on each hard reset.

We could have another copy in memory if we are concerned about being consistent,
reset firmware to that on reboot.

 --
   Gleb.
 

Re: [Qemu-devel] M68K Or PPC Status Update to Run Mac OS in Qemu

[G 3]

On Dec 21, 2009, at 2:13 AM, qemu-devel-requ...@nongnu.org wrote:


On 20.12.2009, at 21:32, Leo B wrote:

A while back I know someone was working on PPC Support in Qemu to  
be able to run Mac OS9-Mac OS X in Qemu on X86 PC's not sure about  
M68K support though so is anybody still working on M68K support or  
PPC Support and how far is it from being included in a stable Qemu  
release,I notice Power PC Support still says Testing on the status  
page how far is it from being complete?


M68k with non-colfire targets is pretty much non-existing. At least  
when it comes to system emulation (which is the one you're  
interested in).


As for PPC, the CPU and device emulation code should work fine for  
OSX up to 10.2 or 10.3. Then Apple dropped support for the G3 Beige.


The major bit missing to get those to work is OpenBIOS. It's  
missing some Forth extension that the OSX bootloader uses, so it  
fails to load the kernel and boot.


Alex


What exactly is missing in OpenBIOS. I am currently studying the IEEE  
1275 document (OpenBIOS specifications). I would like to add this  
system if I can.

Re: [Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loading overhaul.

[Gleb Natapov]

On Mon, Dec 21, 2009 at 11:26:12AM -0600, Anthony Liguori wrote:
 On 12/21/2009 10:43 AM, Gleb Natapov wrote:
 There are some really ugly corner cases here.  For instance, guest
 is running and the user does a yum update which upgrades the qemu
 package.  This includes laying down a new bios.
 
 User eventually restarts guest, now we re-read BIOS and we're on a
 newer BIOS than the device model.  Badness ensues.
 
 My package manager warns me that certain application need to be
 restarted to work correctly after upgrade. This is hardly qemu specific
 problem.
 
 But again, I don't see when this is ever a feature that a user
 actually wants.  Unless you change restart to fork/exec+exit, you'll
 never have reset equivalent to power off + startup.  Can you
 advocate rereading roms and not advocate re-execing qemu?
Why I will never have reset equivalent to power off + startup? Are you
saying we are not capable of implementing spec correctly?

 And more importantly, what is the end-user benefit of doing this?
 
 Working migration?
 
 How does it fix migration? Migration needs to transfer the current
 roms in order to work.  A new version of qemu must support
 interacting with the old version of the firmware for migration to
 work.  What happens after reset has nothing to do with migration but
 because of the last requirement, the guest will obviously continue
 to work after reboot too.
I don't agree with your last requirement. Firmware goes hand in hand with
HW. Change that is only FW visible should not be backwards compatible.

--
Gleb.

Re: [Qemu-devel] M68K Or PPC Status Update to Run Mac OS in Qemu

[Alexander Graf]

G 3 wrote:

 On Dec 21, 2009, at 2:13 AM, qemu-devel-requ...@nongnu.org
 mailto:qemu-devel-requ...@nongnu.org wrote:

 On 20.12.2009, at 21:32, Leo B wrote:


 A while back I know someone was working on PPC Support in Qemu to be
 able to run Mac OS9-Mac OS X in Qemu on X86 PC's not sure about M68K
 support though so is anybody still working on M68K support or PPC
 Support and how far is it from being included in a stable Qemu
 release,I notice Power PC Support still says Testing on the status
 page how far is it from being complete?


 M68k with non-colfire targets is pretty much non-existing. At least
 when it comes to system emulation (which is the one you're interested
 in).


 As for PPC, the CPU and device emulation code should work fine for
 OSX up to 10.2 or 10.3. Then Apple dropped support for the G3 Beige.


 The major bit missing to get those to work is OpenBIOS. It's missing
 some Forth extension that the OSX bootloader uses, so it fails to
 load the kernel and boot.


 Alex


 What exactly is missing in OpenBIOS. I am currently studying the IEEE
 1275 document (OpenBIOS specifications). I would like to add this
 system if I can. 


OpenBIOS is missing an implementation for local variables. See this
thread:
http://www.openfirmware.info/pipermail/openbios/2009-July/003796.html

Alex

[Qemu-devel] [slirp] guest program's tcp connection hang on close_wait.

[Sam Liao]

Hi All,

I am using qemu-arm running linux (guest), I start a server
program on the linux(guest); then I start a program on host machine to
connect to the server(on qemu linux guest) to a specified tcp port.
(the qemu is started with tcp redir configuration).

Then I got a problem when the client program on host closed the
tcp connection when the server is busy sending data to client. The
server program will hang on close_wait state.  I captured the tcp
packets between the host - qemu and qemu-guest linux. It seems
like the connection is closed like:

1. host - qemu

   FIN --
   -- ACK
   -- More data with PUSH
   RST --

2. qemu - guest

   FIN --
   --  ACK
   -- More data
   ACK --
   -- More data
   ACK --
   ...
   ACK, zero window-
- Keep-Alive
   ACK, zero window-
- keep alive

So, here the connection between the qemu and guest did not closed
correctly. After the qemu's FIN is acked by guest os, the qemu still
receives data until it's
window came to be zero(which I think maybe the receive buf is filled),
then it came to a zerowindow-keepalive loop, and the connection is
hanging there. I checked the slirp's source code, from the packet
between host and qemu, the host connection is reset, and the problem
is the connection between qemu and guest is not correctly closed by
slirp.

I'm not sure this is a slirp bug or something else, any suggestion
would be appreciated.

Thanks.

-Sam

[Qemu-devel] Re: Re: [SeaBIOS] [PATCH 0/8] option rom loadingoverhaul.

[Gleb Natapov]

On Mon, Dec 21, 2009 at 07:24:43PM +0100, Sebastian Herbszt wrote:
 Gleb Natapov wrote:
 On Mon, Dec 21, 2009 at 11:26:12AM -0600, Anthony Liguori wrote:
 On 12/21/2009 10:43 AM, Gleb Natapov wrote:
 There are some really ugly corner cases here.  For instance, guest
 is running and the user does a yum update which upgrades the qemu
 package.  This includes laying down a new bios.
 
 User eventually restarts guest, now we re-read BIOS and we're on a
 newer BIOS than the device model.  Badness ensues.
 
 My package manager warns me that certain application need to be
 restarted to work correctly after upgrade. This is hardly qemu specific
 problem.
 
 But again, I don't see when this is ever a feature that a user
 actually wants.  Unless you change restart to fork/exec+exit, you'll
 never have reset equivalent to power off + startup.  Can you
 advocate rereading roms and not advocate re-execing qemu?
 Why I will never have reset equivalent to power off + startup? Are you
 saying we are not capable of implementing spec correctly?
 
 In the POST failure (loop) with isapc and seabios thread we have concluded
 that the system_reset monitor command should trigger a power cycle, but
 it currently doesn't. This same power cycle logic has to be implemented for 
 ACPI
 reset.
 
It currently tries to do that, but state after reset is not exactly the
same as after qemu start. Each and every such difference is an instance
of a bug.

 And more importantly, what is the end-user benefit of doing this?
 
 Working migration?
 
 How does it fix migration? Migration needs to transfer the current
 roms in order to work.  A new version of qemu must support
 interacting with the old version of the firmware for migration to
 work.  What happens after reset has nothing to do with migration but
 because of the last requirement, the guest will obviously continue
 to work after reboot too.
 I don't agree with your last requirement. Firmware goes hand in hand with
 HW. Change that is only FW visible should not be backwards compatible.
 
 As stated before i don't like the idea of automagically upgrading the firmware
 on reset, e.g. after a live migration to a newer qemu version. You have 
 explained
 that qemu-kvm needs this in order to work with live migration and changed hw
 support because of bug fixes. Is this only needed in the kvm case?
 
There basically two approaches. One is you allow HW to change in such a
way that new FW is needed to init it and then you have to reload newer
FW after migration or you disallow HW changes that will prevent
old FW from configuring it and then you can use old FW after migration.

This is nothing special for KVM.

 Does any OS (Windows?) depend on the tables the bios creates (e.g. smbios)
 for licensing? It would be ugly if Windows wants you to re-activate after a 
 reboot
 following a migration to newer qemu version and therefore possibly changed 
 tables
 due to newer bios.
 
I am not talking about OS visible stuff.

--
Gleb.

Re: [Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loading overhaul.

[Anthony Liguori]

On 12/21/2009 11:43 AM, Gleb Natapov wrote:

Why I will never have reset equivalent to power off + startup? Are you
saying we are not capable of implementing spec correctly?
   


No, I'm saying that if you want reset absolutely equivalent to power off 
+ startup, then you need to fork() and re-exec qemu at startup since the 
qemu binary may have changed while the guest was running.


My point behind this is I think that's equivalent to re-reading rom 
contents from disk.



And more importantly, what is the end-user benefit of doing this?

 

Working migration?
   

How does it fix migration? Migration needs to transfer the current
roms in order to work.  A new version of qemu must support
interacting with the old version of the firmware for migration to
work.  What happens after reset has nothing to do with migration but
because of the last requirement, the guest will obviously continue
to work after reboot too.
 

I don't agree with your last requirement. Firmware goes hand in hand with
HW. Change that is only FW visible should not be backwards compatible.
   


I think we're papering over this issue.  FW interfaces are guest visible 
even though we've been pretending like they aren't.


SeaBIOS does not implement every possible BIOS function.  I'm sure that 
it will implement new functions over time.  The presence of those 
functions are certainly visible to the guest.  Likewise, any bug or 
added feature is visible to a guest.


More concretely, we have an internal OS that interacts very closely with 
PXE roms (it makes use of UNDI).  It's very aware of the difference 
between etherboot and gPXE and it is also aware of different versions of 
those two.


The arguments for saying FW is not guest visible is that FW interfaces 
are well defined and do not change.  The same is true for 99% of the 
hardware we emulate.  The reason we have guest visible changes is that 
we're constantly improving and increasing the functionality of the 
hardware.  The same is true for FW.


I'm starting to think having an nvram with saved firmware and user 
driven upgrades is the best approach for compatibility by far.  I'm 
still concerned though about the relatively complexity of having to 
force users to upgrade their firmware.


Regards,

Anthony Liguori

[Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loadingoverhaul.

[Anthony Liguori]

On 12/21/2009 12:24 PM, Sebastian Herbszt wrote:
As stated before i don't like the idea of automagically upgrading the 
firmware
on reset, e.g. after a live migration to a newer qemu version. You 
have explained
that qemu-kvm needs this in order to work with live migration and 
changed hw

support because of bug fixes. Is this only needed in the kvm case?


It's not needed, it's desired.  The same case can be made for real 
hardware (automated firmware updates).


Does any OS (Windows?) depend on the tables the bios creates (e.g. 
smbios)
for licensing? It would be ugly if Windows wants you to re-activate 
after a reboot
following a migration to newer qemu version and therefore possibly 
changed tables

due to newer bios.


Yes, and this is a good point.  ACPI table changes can absolutely cause 
re-activation.  If we migrate from 0.12 - 0.13 and make major changes 
to the ACPI tables in 0.13, then it's very likely that will result in 
problems for Windows guests.


I really think that we need to snapshot the FW and store it with the 
guest state.  If we switch all FW to be allocated with qemu_ram_alloc() 
and we use an id mechanism, then this will Just Work for savevm based 
snapshots and live migration.  However, for it to work with -M pc-0.11 
started from a cold boot, we need an nvram file.  We probably want to 
make available versioned nvram files from each release too.


Regards,

Anthony Liguori



- Sebastian

Re: [Qemu-devel] [PATCH 3/7] QMP: Assure that returned data is a QDict

[Nathan Baum]

On Fri, 2009-12-18 at 13:25 -0200, Luiz Capitulino wrote:
 This is for debug purposes only.

This breaks quite a lot of commands where the returned data is a QList,
e.g. query-commands, query-mice, query-cpus. Is the assert wrong, or are
such commands meant to be returning a QDict?

 Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
 ---
  monitor.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/monitor.c b/monitor.c
 index d238660..8ef1582 100644
 --- a/monitor.c
 +++ b/monitor.c
 @@ -283,6 +283,7 @@ static void monitor_protocol_emitter(Monitor *mon, 
 QObject *data)
  if (!monitor_has_error(mon)) {
  /* success response */
  if (data) {
 +assert(qobject_type(data) == QTYPE_QDICT);
  qobject_incref(data);
  qdict_put_obj(qmp, return, data);
  } else {

[Qemu-devel] Re: Re: [SeaBIOS] [PATCH 0/8] option rom loadingoverhaul.

[Sebastian Herbszt]

Gleb Natapov wrote:

On Mon, Dec 21, 2009 at 07:24:43PM +0100, Sebastian Herbszt wrote:

Does any OS (Windows?) depend on the tables the bios creates (e.g. smbios)
for licensing? It would be ugly if Windows wants you to re-activate after a 
reboot
following a migration to newer qemu version and therefore possibly changed 
tables
due to newer bios.


I am not talking about OS visible stuff.


How do you want to avoid OS visible changes if the bios is reread on each 
reset? People
can upgrade/change their bios in the pc-bios directory and bad things could 
happen if its
reloaded on each reset.

- Sebastian

[Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loadingoverhaul.

[Sebastian Herbszt]

Anthony Liguori wrote:

On 12/21/2009 12:24 PM, Sebastian Herbszt wrote:
As stated before i don't like the idea of automagically upgrading the 
firmware
on reset, e.g. after a live migration to a newer qemu version. You 
have explained
that qemu-kvm needs this in order to work with live migration and 
changed hw

support because of bug fixes. Is this only needed in the kvm case?


It's not needed, it's desired.  The same case can be made for real 
hardware (automated firmware updates).


Tho on real hardware those updates are initiated by someone and not
automagic.

Does any OS (Windows?) depend on the tables the bios creates (e.g. 
smbios)
for licensing? It would be ugly if Windows wants you to re-activate 
after a reboot
following a migration to newer qemu version and therefore possibly 
changed tables

due to newer bios.


Yes, and this is a good point.  ACPI table changes can absolutely cause 
re-activation.  If we migrate from 0.12 - 0.13 and make major changes 
to the ACPI tables in 0.13, then it's very likely that will result in 
problems for Windows guests.


Another problem could be on guest resume from S3 after migration if the
bios or acpi tables change.

I really think that we need to snapshot the FW and store it with the 
guest state.  If we switch all FW to be allocated with qemu_ram_alloc() 
and we use an id mechanism, then this will Just Work for savevm based 
snapshots and live migration.  However, for it to work with -M pc-0.11 
started from a cold boot, we need an nvram file.  We probably want to 
make available versioned nvram files from each release too.


So the idea is to store the bios/option roms in the guest state and read them
from there on reset or power cycle?

- Sebastian

Re: [Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loading overhaul.

[Gleb Natapov]

On Mon, Dec 21, 2009 at 01:13:10PM -0600, Anthony Liguori wrote:
 On 12/21/2009 11:43 AM, Gleb Natapov wrote:
 Why I will never have reset equivalent to power off + startup? Are you
 saying we are not capable of implementing spec correctly?
 
 No, I'm saying that if you want reset absolutely equivalent to power
 off + startup, then you need to fork() and re-exec qemu at startup
 since the qemu binary may have changed while the guest was running.
 
 My point behind this is I think that's equivalent to re-reading rom
 contents from disk.
 
You are exaggerating. If user want to update qemu it should exit old
one and re-run new one. Power off + startup, on the other hand, is defied
to be equivalent to ACPI reset by ACPI spec. In other words if devices
state as seen by vmstate after qemu start is different by even one bit
from the state after system_restart this is bug. This is completely
orthogonal to our firmware loading discussion.

 And more importantly, what is the end-user benefit of doing this?
 
 Working migration?
 How does it fix migration? Migration needs to transfer the current
 roms in order to work.  A new version of qemu must support
 interacting with the old version of the firmware for migration to
 work.  What happens after reset has nothing to do with migration but
 because of the last requirement, the guest will obviously continue
 to work after reboot too.
 I don't agree with your last requirement. Firmware goes hand in hand with
 HW. Change that is only FW visible should not be backwards compatible.
 
 I think we're papering over this issue.  FW interfaces are guest
 visible even though we've been pretending like they aren't.
Not all of it. We can completely change extboot interface, for instance,
and support migration if we will handle things right. We can fix bugs in
tpr patching without losing migration capability.

 
 SeaBIOS does not implement every possible BIOS function.  I'm sure
 that it will implement new functions over time.  The presence of
 those functions are certainly visible to the guest.  Likewise, any
 bug or added feature is visible to a guest.
 
No, not any bug is visible to the guest. Example: kvm currently configures
lvt0 to virtual wire inside qemu/kvm acpi code, but this should be done
by the BIOS on real HW. So suppose we fixed this bug and moved
initialization into the BIOS If we'll try to init new kvm with old BIOS
next OS boot will fail.

 More concretely, we have an internal OS that interacts very closely
 with PXE roms (it makes use of UNDI).  It's very aware of the
 difference between etherboot and gPXE and it is also aware of
 different versions of those two.
 
 The arguments for saying FW is not guest visible is that FW
 interfaces are well defined and do not change.  The same is true for
 99% of the hardware we emulate.  The reason we have guest visible
 changes is that we're constantly improving and increasing the
 functionality of the hardware.  The same is true for FW.
 
We can't do guest visible changes in devices and FW and support
migration from older qemu. I think we agree on this. Non guest visible
changes are allowed for device emulation, you are trying to disallow it for
FW.

 I'm starting to think having an nvram with saved firmware and user
 driven upgrades is the best approach for compatibility by far.  I'm
 still concerned though about the relatively complexity of having to
 force users to upgrade their firmware.
 
Please stop thinking so :) Especially about user driven upgrades.
Why combine disadvantages of vitalization with pain of physical HW
management? Or may be next step will be to require uploading of CPU
microcode to take advantage of KVM/tcg bug fixes?

--
Gleb.

[Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loadingoverhaul.

[Gleb Natapov]

On Mon, Dec 21, 2009 at 01:17:23PM -0600, Anthony Liguori wrote:
 Does any OS (Windows?) depend on the tables the bios creates (e.g.
 smbios)
 for licensing? It would be ugly if Windows wants you to
 re-activate after a reboot
 following a migration to newer qemu version and therefore possibly
 changed tables
 due to newer bios.
 
 Yes, and this is a good point.  ACPI table changes can absolutely
 cause re-activation.  If we migrate from 0.12 - 0.13 and make major
 changes to the ACPI tables in 0.13, then it's very likely that will
 result in problems for Windows guests.
 
On the contrary. This is very unlikely. Otherwise BIOS upgrade would
cause Windows reactivation.

 I really think that we need to snapshot the FW and store it with the
 guest state.  If we switch all FW to be allocated with
 qemu_ram_alloc() and we use an id mechanism, then this will Just
 Work for savevm based snapshots and live migration.  However, for it
 to work with -M pc-0.11 started from a cold boot, we need an nvram
 file.  We probably want to make available versioned nvram files from
 each release too.
 
Yes, firmware should be part of machine description.

--
Gleb.

[Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loadingoverhaul.

[Gleb Natapov]

On Mon, Dec 21, 2009 at 08:39:03PM +0100, Sebastian Herbszt wrote:
 Anthony Liguori wrote:
 On 12/21/2009 12:24 PM, Sebastian Herbszt wrote:
 As stated before i don't like the idea of automagically
 upgrading the firmware
 on reset, e.g. after a live migration to a newer qemu version.
 You have explained
 that qemu-kvm needs this in order to work with live migration
 and changed hw
 support because of bug fixes. Is this only needed in the kvm case?
 
 It's not needed, it's desired.  The same case can be made for
 real hardware (automated firmware updates).
 
 Tho on real hardware those updates are initiated by someone and not
 automagic.
 
Because on real hardware it is impossible to do it differently may be?
My cable TV provider upgrades FW on my set-top-box automatically.

 Does any OS (Windows?) depend on the tables the bios creates
 (e.g. smbios)
 for licensing? It would be ugly if Windows wants you to
 re-activate after a reboot
 following a migration to newer qemu version and therefore
 possibly changed tables
 due to newer bios.
 
 Yes, and this is a good point.  ACPI table changes can absolutely
 cause re-activation.  If we migrate from 0.12 - 0.13 and make
 major changes to the ACPI tables in 0.13, then it's very likely
 that will result in problems for Windows guests.
 
 Another problem could be on guest resume from S3 after migration if the
 bios or acpi tables change.
On resume from S3 BIOS doesn't recreate ACPI tables. ACPI tables are not
part of a BIOS image and in fact OS can reuse memory ACPI tables reside
in. So such problem definitely does not exist.

 
 I really think that we need to snapshot the FW and store it with
 the guest state.  If we switch all FW to be allocated with
 qemu_ram_alloc() and we use an id mechanism, then this will Just
 Work for savevm based snapshots and live migration.  However, for
 it to work with -M pc-0.11 started from a cold boot, we need an
 nvram file.  We probably want to make available versioned nvram
 files from each release too.
 
 So the idea is to store the bios/option roms in the guest state and read them
 from there on reset or power cycle?
 
 - Sebastian

--
Gleb.

[Qemu-devel] Re: Re: [SeaBIOS] [PATCH 0/8] option rom loadingoverhaul.

[Gleb Natapov]

On Mon, Dec 21, 2009 at 08:28:20PM +0100, Sebastian Herbszt wrote:
 Gleb Natapov wrote:
 On Mon, Dec 21, 2009 at 07:24:43PM +0100, Sebastian Herbszt wrote:
 Does any OS (Windows?) depend on the tables the bios creates (e.g. smbios)
 for licensing? It would be ugly if Windows wants you to re-activate after a 
 reboot
 following a migration to newer qemu version and therefore possibly changed 
 tables
 due to newer bios.
 
 I am not talking about OS visible stuff.
 
 How do you want to avoid OS visible changes if the bios is reread on each 
 reset? People
 can upgrade/change their bios in the pc-bios directory and bad things could 
 happen if its
 reloaded on each reset.
 
The fact that people can doesn't mean people should. Arbitrary
combination of qemu + BIOS is not supported. If people do what you say
the bad things will happen anyway after restarting qemu instead of
reboot.

--
Gleb.

[Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loadingoverhaul.

[Sebastian Herbszt]

Gleb Natapov wrote:

On Mon, Dec 21, 2009 at 08:39:03PM +0100, Sebastian Herbszt wrote:

Anthony Liguori wrote:
On 12/21/2009 12:24 PM, Sebastian Herbszt wrote:
As stated before i don't like the idea of automagically
upgrading the firmware
on reset, e.g. after a live migration to a newer qemu version.
You have explained
that qemu-kvm needs this in order to work with live migration
and changed hw
support because of bug fixes. Is this only needed in the kvm case?

It's not needed, it's desired.  The same case can be made for
real hardware (automated firmware updates).

Tho on real hardware those updates are initiated by someone and not
automagic.


Because on real hardware it is impossible to do it differently may be?
My cable TV provider upgrades FW on my set-top-box automatically.


Your cable TV provider does likely also control what beside the FW (if anything)
runs on your set-top-box. So he can verify the FW upgrade doesn't break anything
in the field. That pre-deployment verification is not possible in non closed
environments.


Does any OS (Windows?) depend on the tables the bios creates
(e.g. smbios)
for licensing? It would be ugly if Windows wants you to
re-activate after a reboot
following a migration to newer qemu version and therefore
possibly changed tables
due to newer bios.

Yes, and this is a good point.  ACPI table changes can absolutely
cause re-activation.  If we migrate from 0.12 - 0.13 and make
major changes to the ACPI tables in 0.13, then it's very likely
that will result in problems for Windows guests.

Another problem could be on guest resume from S3 after migration if the
bios or acpi tables change.

On resume from S3 BIOS doesn't recreate ACPI tables. ACPI tables are not
part of a BIOS image and in fact OS can reuse memory ACPI tables reside
in. So such problem definitely does not exist.


If the OS recycles the whole memory which holds the ACPI tables i am not sure 
how
the BIOS will find the firmware_waking_vector. Maybe the OS can only use the 
memory
which holds the DSDT? Anyway, will the guest even resume from S3 if the hw 
changed
on migration and the bios doesn't know how to init it?

- Sebastian

Re: [Qemu-devel] [PATCH] PPC64: Fix timebase

[Andreas Färber]

Am 21.12.2009 um 11:15 schrieb Aurelien Jarno:


On Mon, Dec 21, 2009 at 10:39:39AM +0100, Alexander Graf wrote:


On 21.12.2009, at 10:24, Aurelien Jarno wrote:


On Mon, Dec 21, 2009 at 01:22:12AM +0100, Alexander Graf wrote:
On PPC we have a 64-bit time base. Usually (PPC32) this is  
accessed using

two separate 32 bit SPR accesses to SPR_TBU and SPR_TBL.

On PPC64 the SPR_TBL register acts as 64 bit though, so we get  
the full
64 bits as return value. If we only take the lower ones, fine.  
But Linux

wants to see all 64 bits or it breaks.


Good catch! However, I think this patch it's not fully complete  
and can

be improved a bit
- it's probably better to return a target_ulong value from
cpu_ppc_load_tbl() with an explicit cast here, so that we don't have
an implicit cast from 64-bit to 32-bit on qemu-system-powerpc (GCC  
may

warn on that with some flags or in future versions).


ppc.c is in hw, so I suspect it's in the target independent  
makefile part? Otherwise we should move all TB stuff to target-ppc.


Correct.


No. Just like hw/ppc_newworld.c, it is target dependent and built from  
Makefile.target (obj-ppc-y).


Andreas


then let's return uint64_t for cpu_ppc_load_tbl(), but do the
explicit cast in the helper.

--
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH 0/5] tcg conditional set, round 4

[Richard Henderson]

On 12/21/2009 01:13 AM, Laurent Desnogues wrote:

The question for the generalized movcond is how useful is it?
Which front-ends would need it and would the cost to generate
code for it on some (most?) back-ends be amortized?


... Any front end that has a conditional move instruction?
Sparcv9, Mips32, Alpha, ARM...

That said, I think the *biggest* gains are to be had because with 
movcond -- at least on some targets -- we can have one BB per TB, and 
avoid any intermediate spilling of global registers back to memory.



My guess (I use that word given that I didn't do any benchmark
to sustain my claim) is that your implementation is too complex.


Too complex for what?  The message against which you are quoting has an 
implementation of 2 lines.



Of course setcond can be implemented in terms of movcond,
but my guess (again that word...) is that setcond could be
enough and even faster in most cases.


To implement condition codes, yes, to implement compare instructions 
(e.g. mips slt, alpha cmp{eq,lt,lte}), yes.  To implement conditional 
moves, no.  At least not without using 5 instructions where 1 would suffice.



Regarding your patches, I would like to see setcond put in
mainline with a simplified version for i386.


Again, simplified from what?  The last setcond implementation was 2 lines.


r~

Re: [Qemu-devel] [PATCH 3/3] linux-user: Initialize Alpha FPCR register.

[Richard Henderson]

On 12/21/2009 02:33 AM, Aurelien Jarno wrote:

On Sat, Dec 19, 2009 at 03:17:16PM -0800, Richard Henderson wrote:

Signed-off-by: Richard Hendersonr...@twiddle.net
---
  linux-user/main.c |2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 12502ad..b67662c 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3052,6 +3052,8 @@ int main(int argc, char **argv, char **envp)
  env-ir[30] = regs-usp;
  env-pc = regs-pc;
  env-unique = regs-unique;
+cpu_alpha_store_fpcr(env, (FPCR_INVD | FPCR_DZED | FPCR_OVFD
+   | FPCR_UNFD | FPCR_INED | FPCR_DNOD));
  }


This cpu initialization which does not depends on the binary being run
is usually done in target-*/translate.c, using #if defined
(CONFIG_USER_ONLY).


I didn't want to assume that bsd-user initializes the fpcr to the same 
value.  However, they probably do.  This appears to be what you wanted.



r~
commit 033df7a558acdf39d81681f3858f13ebe2a92a6e
Author: Richard Henderson r...@twiddle.net
Date:   Mon Dec 21 12:48:43 2009 -0800

target-alpha: Initialize fpcr.

Linux, at least, disables exceptions by default.

Signed-off-by: Richard Henderson r...@twiddle.net

diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 5e0647b..87813e7 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -2748,6 +2748,8 @@ CPUAlphaState * cpu_alpha_init (const char *cpu_model)
 env-ps = 0x1F00;
 #if defined (CONFIG_USER_ONLY)
 env-ps |= 1  3;
+cpu_alpha_store_fpcr(env, (FPCR_INVD | FPCR_DZED | FPCR_OVFD
+   | FPCR_UNFD | FPCR_INED | FPCR_DNOD));
 #endif
 pal_init(env);
 /* Initialize IPR */

Re: [Qemu-devel] [PATCH 0/5] tcg conditional set, round 4

[Laurent Desnogues]

On Mon, Dec 21, 2009 at 9:28 PM, Richard Henderson r...@twiddle.net wrote:
 On 12/21/2009 01:13 AM, Laurent Desnogues wrote:

 The question for the generalized movcond is how useful is it?
 Which front-ends would need it and would the cost to generate
 code for it on some (most?) back-ends be amortized?

 ... Any front end that has a conditional move instruction?
 Sparcv9, Mips32, Alpha, ARM...

As far as I know these CPU's don't need the full movcond but
only the variant with vtrue. Even if movcond was quick to generate
host code, for instance for ARM, you'd have to explicitly detect
conditional moves, which probably wouldn't be worth the cost;
I might be wrong, since no one has given it a try.

 That said, I think the *biggest* gains are to be had because with movcond --
 at least on some targets -- we can have one BB per TB, and avoid any
 intermediate spilling of global registers back to memory.

I can't count the number of times I thought some branch removal
could only bring improvement, only to see QEMU slow down.
The balance between simplicity and good generated code is
very hard to achieve (and in that particular case, benchmarking
on an Intel just shows how Intel engineers are good at designing
branch predictors :-).

 My guess (I use that word given that I didn't do any benchmark
 to sustain my claim) is that your implementation is too complex.

 Too complex for what?  The message against which you are quoting has an
 implementation of 2 lines.

Well I answered to this mail after seeing the SPARC
implementation :)  Indeed your implementation for i386 setcond2
(setcond is trivial) is not that complex.

 Of course setcond can be implemented in terms of movcond,
 but my guess (again that word...) is that setcond could be
 enough and even faster in most cases.

 To implement condition codes, yes, to implement compare instructions (e.g.
 mips slt, alpha cmp{eq,lt,lte}), yes.  To implement conditional moves, no.
  At least not without using 5 instructions where 1 would suffice.

How many instructions would you need to generate one
host instruction?  If the block is not executed that often, it could
be a waste.  If you want I can provide you with dynamic counts
of ARM conditional mov when running SPEC;  but that wouldn't
be enough, someone would need to do that for the kernel
boot too.

I'm not saying movcond is useless, I'm just wondering if it
would bring improvements.  That's why I would prefer to do
all of that stuff incrementally:  setcond, then movcond.

 Regarding your patches, I would like to see setcond put in
 mainline with a simplified version for i386.

 Again, simplified from what?  The last setcond implementation was 2 lines.

I was wrong sorry, I mixed several of your patches.


Laurent

Re: [Qemu-devel] [PATCH 0/5] tcg conditional set, round 4

[Laurent Desnogues]

On Mon, Dec 21, 2009 at 11:50 PM, Richard Henderson r...@twiddle.net wrote:
[...]
 Even if movcond was quick to generate
 host code, for instance for ARM, you'd have to explicitly detect
 conditional moves

 One of us is confused.  Why would I have to explicitly detect conditional
 moves?

Most ARM instructions are conditional, with condition code being the
top 4 bits of the instruction.  So the front-end does it the simplest
way possible:

if (cond != 0xe) {
/* if not always execute, we generate a conditional jump to
   next instruction */
s-condlabel = gen_new_label();
gen_test_cc(cond ^ 1, s-condlabel);
s-condjmp = 1;
}

and then generates the code for the instruction as if it wasn't
conditional.  If you wanted to use movcond, you'd have to make
cond + move a special case, which would add some cost to
all conditional instructions.  OTOH that cost could be amortized
by generating less TCG ops for that instruction, and by a
potentially faster generated code.  But if this isn't measured I
won't bet which one is faster, I have been wrong too often.


Laurent

Re: [Qemu-devel] [PATCH 0/5] tcg conditional set, round 4

[Richard Henderson]

On 12/21/2009 02:21 PM, Laurent Desnogues wrote:

As far as I know these CPU's don't need the full movcond but
only the variant with vtrue.


I know that.  And I looked into TCG very closely to figure out how to 
implement just that.  Except then I have to modify TCG to special-case 
movcond to know that DEST is both input and output.  This is *much* more 
work than simply having the back-end use a matching constraint on VFALSE.



Even if movcond was quick to generate
host code, for instance for ARM, you'd have to explicitly detect
conditional moves


One of us is confused.  Why would I have to explicitly detect 
conditional moves?



r~

Re: [Qemu-devel] Re: [SeaBIOS] [PATCH 0/8] option rom loading overhaul.

[Anthony Liguori]

On 12/21/2009 01:43 PM, Gleb Natapov wrote:

Please stop thinking so :) Especially about user driven upgrades.
Why combine disadvantages of vitalization with pain of physical HW
management? Or may be next step will be to require uploading of CPU
microcode to take advantage of KVM/tcg bug fixes?
   


I think your real argument boils down to that we can be better than real 
hardware therefore we should.  I actually agree with you for 90% of 
users.  Let me summarize where I'm at.


 - We are currently horribly broken with respect to how we handle roms 
particularly with respect to backwards compatibility
 - We must support running older roms on newer qemu at least within a 
stable series (because of live migration)

 - We need a mechanism to include roms specific to a machine type
 - Probably by default, we want new roms to be used by guests at some 
well defined time (either first reset or first power-down/start-up)
 - We ought to make all roms live in qemu_ram_alloc()'d memory and we 
ought to change that api to contain contexts, this will make sure rom 
live migration works properly.


The best approach I can think of is to introduce an nvram mechanism.  A 
tar file probably work really well.  If a user doesn't supply an 
explicit -nvram, we could create a temporary file and delete it.  If the 
nvram is empty, we populate it with whatever the appropriate roms are 
for the machine type.


I would also suggest that we support the guest updating roms on its own 
(through fw_cfg).  I can think of a number of reasons for this.  For 
instance, why shouldn't a guest be able to update the gPXE associated 
with the network card?  The code runs entirely in the guest so there's 
no harm to the host.  Allow a guest to do this sort of thing takes 
pressure off of the management system so particularly in an environment 
like a cloud, I think this could prove very useful.


It could be possible to add an option to -nvram to make it re-read roms 
from disk every reboot.  I really think this is a bad idea though, I'd 
like to hear more people comment on it but it's certainly technically 
feasible.


Regards,

Anthony Liguori

--
Gleb.
   

Re: [Qemu-devel] [PATCH 0/5] tcg conditional set, round 4

[Richard Henderson]

On 12/21/2009 03:08 PM, Laurent Desnogues wrote:

If you wanted to use movcond, you'd have to make
cond + move a special case...


You'd certainly want the ARM front-end to use movcond more often than 
that.  For instance:


  addeq r1,r2,r3
--
  add_i32 tmp,r2,r3
  movcond_i32 r1,ZF,0,tmp,r1,eq

You'd want to continue to use a branch around if the instruction has 
side effects like cpu fault (e.g. load, store) or updating flags.


It ought not be very hard to arrange for something like

  if (cond != 0xe) {
if (may_use_movcond(insn)) {
  s-condlabel = -1;
  /* Save the true destination register.  */
  s-conddest = cpu_R[dest];
  /* Implement the instruction into a temporary.  */
  cpu_R[dest] = tcg_temp_new();
} else {
  s-condlabel = gen_new_label();
  ArmConditional cmp = gen_test_cc(cond ^ 1);
  tcg_gen_brcondi_i32(cmp.cond, cmp.reg, 0, s-condlabel);
}
s-condjmp = 1;
  }

  // ... implement the instruction as we currently do.

  if (s-condjmp) {
if (s-condlabel == -1) {
  /* Conditionally move the temporary result into the
 true destination register.  */
  ArmConditional cmp = gen_test_cc(cond);
  tcg_gen_movcond_i32(cmp.cond, s-conddest, cmp.reg, 0,
  cpu_R[dest], s-conddest);
  tcg_temp_free(cpu_R[dest]);
  /* Restore the true destination register.  */
  cpu_R[dest] = s-conddest;
} else {
  tcg_set_label(d-condlabel);
}
  }


r~

Re: [Qemu-devel] cpuid problem in upstream qemu with kvm

[john cooper]

Dor Laor wrote:

Qemu will check the required cpuid of the cpu model on the host and 
refuse to load otherwise. When moving to this model, migration can be 
simplified too since there are fewer combination, and one can choose 
performance over migration flexibility and wise versa.
Due to the above check, the destination qemu won't load if the host does 
not support its cpu model.


If you're referring to the check in my patch, that's
currently advisory only.

The existing cpu model encoding of CPUID tosses flags
in to the soup on speculation they may be available
on the host.  If not it assumes they will be quietly
disabled on their way to the guest along with whatever
flags were enabled via +flag on the command line. 


This mixed treatment for model implied vs. user
specified flags could be partly an artifact of trying
to adapt the legacy qemu64 model to whatever host
silicon it finds itself upon.

-john


--
john.coo...@third-harmonic.com

Re: [Qemu-devel] [PATCH 3/7] QMP: Assure that returned data is a QDict

[Luiz Capitulino]

On Mon, 21 Dec 2009 19:21:18 +
Nathan Baum nat...@parenthephobia.org.uk wrote:

 On Fri, 2009-12-18 at 13:25 -0200, Luiz Capitulino wrote:
  This is for debug purposes only.
 
 This breaks quite a lot of commands where the returned data is a QList,
 e.g. query-commands, query-mice, query-cpus. Is the assert wrong, or are
 such commands meant to be returning a QDict?

 The assert is wrong, as we've defined that returning a QList of
QDicts is ok.

 We could check for a QList too and check its contents but I think that
only dropping the assert is ok for now.

 Will submit a patch and thanks for testing QMP.

[Qemu-devel] [STABLE 0.12] QMP: Drop wrong assert()

[Luiz Capitulino]

Some commands return a QList of QDicts, which is valid,
but will trig the assert().

Just drop it.

Reported-by: Nathan Baum nat...@parenthephobia.org.uk
Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
---
 monitor.c |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/monitor.c b/monitor.c
index c0dc48e..3af1d5c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -283,7 +283,6 @@ static void monitor_protocol_emitter(Monitor *mon, QObject 
*data)
 if (!monitor_has_error(mon)) {
 /* success response */
 if (data) {
-assert(qobject_type(data) == QTYPE_QDICT);
 qobject_incref(data);
 qdict_put_obj(qmp, return, data);
 } else {
-- 
1.6.6.rc3

[Qemu-devel] Please help the daughter of a member of open source community

[yajin]

I am very very sorry for sending this mail to qemu-dev list. Please
please help panjet, who loves the open source software and makes
contribution to the open source community. He is the core member of
the open source project jz-hacking and virtualmips. And even more, he
created the open source foundation - Shangri-La, a foundation intends
to help the open source software development in China.

His daughter Yi Fan is 4 years old and she is dying She has pulmonary
hypertension. She loves poetry, her bunny and her Mom and Dad. Her
dreams include walking in the park, roller skating and someday going
to school.

We need your help to make this dream come true...

http://yifanfund.com/  (English)
http://help-yifan.org   (Chinese)

---
yajin

http://vm-kernel.org/blog

Re: [Qemu-devel] [PATCH 0/5] tcg conditional set, round 4

[Aurelien Jarno]

On Mon, Dec 21, 2009 at 12:28:03PM -0800, Richard Henderson wrote:
 On 12/21/2009 01:13 AM, Laurent Desnogues wrote:
 The question for the generalized movcond is how useful is it?
 Which front-ends would need it and would the cost to generate
 code for it on some (most?) back-ends be amortized?

 ... Any front end that has a conditional move instruction?
 Sparcv9, Mips32, Alpha, ARM...

 That said, I think the *biggest* gains are to be had because with  
   ^
This word is the main reason why I am not convinced at all by movcond.

 movcond -- at least on some targets -- we can have one BB per TB, and  
 avoid any intermediate spilling of global registers back to memory.

Memory load/stores are far more frequent, and also spill global
registers back to memory.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [STABLE 0.12] QMP: Drop wrong assert()

[Markus Armbruster]

Luiz Capitulino lcapitul...@redhat.com writes:

 Some commands return a QList of QDicts, which is valid,
 but will trig the assert().

 Just drop it.

 Reported-by: Nathan Baum nat...@parenthephobia.org.uk
 Signed-off-by: Luiz Capitulino lcapitul...@redhat.com
 ---
  monitor.c |1 -
  1 files changed, 0 insertions(+), 1 deletions(-)

 diff --git a/monitor.c b/monitor.c
 index c0dc48e..3af1d5c 100644
 --- a/monitor.c
 +++ b/monitor.c
 @@ -283,7 +283,6 @@ static void monitor_protocol_emitter(Monitor *mon, 
 QObject *data)
  if (!monitor_has_error(mon)) {
  /* success response */
  if (data) {
 -assert(qobject_type(data) == QTYPE_QDICT);
  qobject_incref(data);
  qdict_put_obj(qmp, return, data);
  } else {

Yes, let's drop the assertion for now.

qmp-spec.txt specifies json-object, so it needs fixing as well.
I figure what we want is either a dictionary (json-object), or a list of
dictionaries (json-array of json-object).