[PATCH 0/9] More integration with qemu.git

[PATCH 0/9] More integration with qemu.git

[Glauber Costa]

Marcelo:

Attention: this is _not_ a resent of my last series. I'm deferring
resending that until we reach consensus on the ioctl error values.

This one is another one I had on trigger. I tested it across reboots,
system_resets, shutdown, migration, and normal use.

It tries to glue together a lot of code that looked like the same. It
has a quite visible side effect, which is that from this point on, 
we'll need --enabled-kvm for running a kvm guest. Although it is a big
change, I argue that it would happen soon or later.

[PATCH 1/9] require --enable-kvm

[Glauber Costa]

Also following upstream behaviour, we exit if not able to service this
request. As we target move forward on merging with qemu, this would have to
happen. So make it sooner, rather than later

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm.c |    2 +-
 vl.c       |    6 ------
 2 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 3c892e6..db28126 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -37,7 +37,7 @@
 #error libkvm: userspace and kernel version mismatch
 #endif
 
-int kvm_allowed = 1;
+int kvm_allowed = 0;
 int kvm_irqchip = 1;
 int kvm_pit = 1;
 int kvm_pit_reinject = 1;
diff --git a/vl.c b/vl.c
index b3df596..7701488 100644
--- a/vl.c
+++ b/vl.c
@@ -5434,13 +5434,11 @@ int main(int argc, char **argv, char **envp)
                 break;
 #endif
 #ifdef CONFIG_KVM
-#ifdef KVM_UPSTREAM
             case QEMU_OPTION_enable_kvm:
                 kvm_allowed = 1;
 #ifdef CONFIG_KQEMU
                 kqemu_allowed = 0;
 #endif
-#endif
                 break;
 	    case QEMU_OPTION_no_kvm:
 		kvm_allowed = 0;
@@ -5754,11 +5752,7 @@ int main(int argc, char **argv, char **envp)
     if (kvm_enabled()) {
 	if (kvm_init(smp_cpus) < 0) {
 	    fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
-#ifdef NO_CPU_EMULATION
-	    fprintf(stderr, "Compiled with --disable-cpu-emulation, exiting.\n");
 	    exit(1);
-#endif
-	    kvm_allowed = 0;
 	}
     }
 #endif
-- 
1.6.2.2

[PATCH 2/9] embed kvm_create_context into kvm_init

[Glauber Costa]

There is no reason why kvm_create_context is placed outside kvm_init().
After we call kvm_init(), no extra initialization step should be necessary.

This patch folds kvm_create_context into it, simplifying vl.c code.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm.c |    6 ++++--
 qemu-kvm.h |    1 -
 vl.c       |    7 -------
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index db28126..c3af59c 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -419,6 +419,8 @@ int kvm_dirty_pages_log_reset(kvm_context_t kvm)
 }
 
 
+static int kvm_create_context(void);
+
 int kvm_init(int smp_cpus)
 {
 	int fd;
@@ -478,7 +480,7 @@ int kvm_init(int smp_cpus)
 	}
 
     pthread_mutex_lock(&qemu_mutex);
-	return 0;
+    return kvm_create_context();
 
  out_close:
 	close(fd);
@@ -2249,7 +2251,7 @@ int kvm_arch_init_irq_routing(void)
 }
 #endif
 
-int kvm_qemu_create_context(void)
+static int kvm_create_context()
 {
     int r;
 
diff --git a/qemu-kvm.h b/qemu-kvm.h
index d5291a3..4fc2847 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -930,7 +930,6 @@ struct kvm_pit_state { };
 
 int kvm_main_loop(void);
 int kvm_qemu_init(void);
-int kvm_qemu_create_context(void);
 int kvm_init_ap(void);
 int kvm_vcpu_inited(CPUState *env);
 void kvm_load_registers(CPUState *env);
diff --git a/vl.c b/vl.c
index 7701488..f4e4d0f 100644
--- a/vl.c
+++ b/vl.c
@@ -5838,13 +5838,6 @@ int main(int argc, char **argv, char **envp)
     if (ram_size == 0)
         ram_size = DEFAULT_RAM_SIZE * 1024 * 1024;
 
-    if (kvm_enabled()) {
-	    if (kvm_qemu_create_context() < 0) {
-		    fprintf(stderr, "Could not create KVM context\n");
-		    exit(1);
-	    }
-    }
-
 #ifdef CONFIG_KQEMU
     /* FIXME: This is a nasty hack because kqemu can't cope with dynamic
        guest ram allocation.  It needs to go away.  */
-- 
1.6.2.2

[PATCH 3/9] change order of kvm_init call.

[Glauber Costa]

The goal is to get rid of the call to kvm_init. But those things
are subtle, and often break. So do it in a separate patch, to help
finding potential issues in future bisections.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 vl.c |   18 +++++++++---------
 1 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/vl.c b/vl.c
index f4e4d0f..86a6d70 100644
--- a/vl.c
+++ b/vl.c
@@ -5748,15 +5748,6 @@ int main(int argc, char **argv, char **envp)
         signal(SIGTTIN, SIG_IGN);
     }
 
-#ifdef CONFIG_KVM
-    if (kvm_enabled()) {
-	if (kvm_init(smp_cpus) < 0) {
-	    fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
-	    exit(1);
-	}
-    }
-#endif
-
     if (pid_file && qemu_create_pidfile(pid_file) != 0) {
         if (daemonize) {
             uint8_t status = 1;
@@ -5956,6 +5947,15 @@ int main(int argc, char **argv, char **envp)
     }
 #endif
 
+#ifdef CONFIG_KVM
+    if (kvm_enabled()) {
+	if (kvm_init(smp_cpus) < 0) {
+	    fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
+	    exit(1);
+	}
+    }
+#endif
+
     if (monitor_device) {
         monitor_hd = qemu_chr_open("monitor", monitor_device, NULL);
         if (!monitor_hd) {
-- 
1.6.2.2

[PATCH 4/9] use qemu version of kvm initialization

[Glauber Costa]

Our code now looks like qemu's a lot. Remove ours, and leave just
qemu's.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 vl.c |   11 -----------
 1 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/vl.c b/vl.c
index 86a6d70..ae9e0b4 100644
--- a/vl.c
+++ b/vl.c
@@ -5935,7 +5935,6 @@ int main(int argc, char **argv, char **envp)
         }
     }
 
-#ifdef KVM_UPSTREAM
     if (kvm_enabled()) {
         int ret;
 
@@ -5945,16 +5944,6 @@ int main(int argc, char **argv, char **envp)
             exit(1);
         }
     }
-#endif
-
-#ifdef CONFIG_KVM
-    if (kvm_enabled()) {
-	if (kvm_init(smp_cpus) < 0) {
-	    fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
-	    exit(1);
-	}
-    }
-#endif
 
     if (monitor_device) {
         monitor_hd = qemu_chr_open("monitor", monitor_device, NULL);
-- 
1.6.2.2

[PATCH 5/9] fold second pass of kvm initialization

[Glauber Costa]

From: Glauber Costa <glommer@t60.(none)>

There is no reason why kvm_init_ap() and friends are placed outside kvm_init().
After we call kvm_init(), no extra initialization step should be necessary.
There are now no references to KVM_UPSTREAM outside of kvm*.c files

Signed-off-by: Glauber Costa <glommer@t60.(none)>
---
 qemu-kvm.c |   20 ++++++++++++++++++++
 vl.c       |   22 ----------------------
 2 files changed, 20 insertions(+), 22 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index c3af59c..26cac25 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -2251,6 +2251,8 @@ int kvm_arch_init_irq_routing(void)
 }
 #endif
 
+extern int no_hpet;
+
 static int kvm_create_context()
 {
     int r;
@@ -2283,6 +2285,24 @@ static int kvm_create_context()
         return r;
     }
 
+    kvm_init_ap();
+    if (kvm_irqchip) {
+        if (!qemu_kvm_has_gsi_routing()) {
+            irq0override = 0;
+#ifdef TARGET_I386
+            /* if kernel can't do irq routing, interrupt source
+             * override 0->2 can not be set up as required by hpet,
+             * so disable hpet.
+             */
+            no_hpet=1;
+        } else  if (!qemu_kvm_has_pit_state2()) {
+            no_hpet=1;
+        }
+#else
+        }
+#endif
+    }
+
     return 0;
 }
 
diff --git a/vl.c b/vl.c
index ae9e0b4..92ae881 100644
--- a/vl.c
+++ b/vl.c
@@ -5997,28 +5997,6 @@ int main(int argc, char **argv, char **envp)
 
     module_call_init(MODULE_INIT_DEVICE);
 
-    if (kvm_enabled()) {
-        kvm_init_ap();
-#ifdef CONFIG_KVM
-        if (kvm_irqchip) {
-            if (!qemu_kvm_has_gsi_routing()) {
-                irq0override = 0;
-#ifdef TARGET_I386
-                /* if kernel can't do irq routing, interrupt source
-                 * override 0->2 can not be set up as required by hpet,
-                 * so disable hpet.
-                 */
-                no_hpet=1;
-            } else  if (!qemu_kvm_has_pit_state2()) {
-                no_hpet=1;
-            }
-#else
-            }
-#endif
-        }
-#endif
-    }
-
     machine->init(ram_size, boot_devices,
                   kernel_filename, kernel_cmdline, initrd_filename, cpu_model);
 
-- 
1.6.2.2

[PATCH 6/9] remove kvm_in* functions

[Glauber Costa]

We can use plain qemu's here, and save a couple of lines/complexity.
I'm leaving outb for later, because the SMM thing makes it a little bit
less trivial.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm.c |   25 ++++---------------------
 1 files changed, 4 insertions(+), 21 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 26cac25..58d5de2 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -97,24 +97,6 @@ static int kvm_debug(void *opaque, void *data,
 }
 #endif
 
-static int kvm_inb(void *opaque, uint16_t addr, uint8_t *data)
-{
-    *data = cpu_inb(0, addr);
-    return 0;
-}
-
-static int kvm_inw(void *opaque, uint16_t addr, uint16_t *data)
-{
-    *data = cpu_inw(0, addr);
-    return 0;
-}
-
-static int kvm_inl(void *opaque, uint16_t addr, uint32_t *data)
-{
-    *data = cpu_inl(0, addr);
-    return 0;
-}
-
 #define PM_IO_BASE 0xb000
 
 static int kvm_outb(void *opaque, uint16_t addr, uint8_t data)
@@ -855,15 +837,16 @@ static int handle_io(kvm_vcpu_context_t vcpu)
 	for (i = 0; i < run->io.count; ++i) {
 		switch (run->io.direction) {
 		case KVM_EXIT_IO_IN:
+			r = 0;
 			switch (run->io.size) {
 			case 1:
-				r = kvm_inb(kvm->opaque, addr, p);
+				*(uint8_t *)p = cpu_inb(kvm->opaque, addr);
 				break;
 			case 2:
-				r = kvm_inw(kvm->opaque, addr, p);
+				*(uint16_t *)p = cpu_inw(kvm->opaque, addr);
 				break;
 			case 4:
-				r = kvm_inl(kvm->opaque, addr, p);
+				*(uint32_t *)p = cpu_inl(kvm->opaque, addr);
 				break;
 			default:
 				fprintf(stderr, "bad I/O size %d\n", run->io.size);
-- 
1.6.2.2

[PATCH 7/9] reuse env stop and stopped states

[Glauber Costa]

qemu CPUState already provides "stop" and "stopped" states. And they
mean exactly that. There is no need for us to provide our own.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 cpu-defs.h |    2 --
 qemu-kvm.c |   30 ++++++++++++------------------
 vl.c       |    2 +-
 3 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index 7570096..fce366f 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -142,8 +142,6 @@ struct qemu_work_item;
 struct KVMCPUState {
     pthread_t thread;
     int signalled;
-    int stop;
-    int stopped;
     int created;
     void *vcpu_ctx;
     struct qemu_work_item *queued_work_first, *queued_work_last;
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 58d5de2..6d556b8 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -91,7 +91,7 @@ static int kvm_debug(void *opaque, void *data,
 
     if (handle) {
 	kvm_debug_cpu_requested = env;
-	env->kvm_cpu_state.stopped = 1;
+	env->stopped = 1;
     }
     return handle;
 }
@@ -979,7 +979,7 @@ int handle_halt(kvm_vcpu_context_t vcpu)
 int handle_shutdown(kvm_context_t kvm, CPUState *env)
 {
     /* stop the current vcpu from going back to guest mode */
-    env->kvm_cpu_state.stopped = 1;
+    env->stopped = 1;
 
     qemu_system_reset_request();
     return 1;
@@ -1817,7 +1817,7 @@ int kvm_cpu_exec(CPUState *env)
 
 static int is_cpu_stopped(CPUState *env)
 {
-    return !vm_running || env->kvm_cpu_state.stopped;
+    return !vm_running || env->stopped;
 }
 
 static void flush_queued_work(CPUState *env)
@@ -1863,9 +1863,9 @@ static void kvm_main_loop_wait(CPUState *env, int timeout)
     cpu_single_env = env;
     flush_queued_work(env);
 
-    if (env->kvm_cpu_state.stop) {
-	env->kvm_cpu_state.stop = 0;
-	env->kvm_cpu_state.stopped = 1;
+    if (env->stop) {
+	env->stop = 0;
+	env->stopped = 1;
 	pthread_cond_signal(&qemu_pause_cond);
     }
 
@@ -1877,7 +1877,7 @@ static int all_threads_paused(void)
     CPUState *penv = first_cpu;
 
     while (penv) {
-        if (penv->kvm_cpu_state.stop)
+        if (penv->stop)
             return 0;
         penv = (CPUState *)penv->next_cpu;
     }
@@ -1891,11 +1891,11 @@ static void pause_all_threads(void)
 
     while (penv) {
         if (penv != cpu_single_env) {
-            penv->kvm_cpu_state.stop = 1;
+            penv->stop = 1;
             pthread_kill(penv->kvm_cpu_state.thread, SIG_IPI);
         } else {
-            penv->kvm_cpu_state.stop = 0;
-            penv->kvm_cpu_state.stopped = 1;
+            penv->stop = 0;
+            penv->stopped = 1;
             cpu_exit(penv);
         }
         penv = (CPUState *)penv->next_cpu;
@@ -1912,8 +1912,8 @@ static void resume_all_threads(void)
     assert(!cpu_single_env);
 
     while (penv) {
-        penv->kvm_cpu_state.stop = 0;
-        penv->kvm_cpu_state.stopped = 0;
+        penv->stop = 0;
+        penv->stopped = 0;
         pthread_kill(penv->kvm_cpu_state.thread, SIG_IPI);
         penv = (CPUState *)penv->next_cpu;
     }
@@ -2698,12 +2698,6 @@ int kvm_log_stop(target_phys_addr_t phys_addr, target_phys_addr_t len)
     return 0;
 }
 
-void qemu_kvm_cpu_stop(CPUState *env)
-{
-    if (kvm_enabled())
-        env->kvm_cpu_state.stopped = 1;
-}
-
 int kvm_set_boot_cpu_id(uint32_t id)
 {
 	return kvm_set_boot_vcpu_id(kvm_context, id);
diff --git a/vl.c b/vl.c
index 92ae881..32134a2 100644
--- a/vl.c
+++ b/vl.c
@@ -3553,7 +3553,7 @@ void qemu_system_reset_request(void)
         reset_requested = 1;
     }
     if (cpu_single_env) {
-        qemu_kvm_cpu_stop(cpu_single_env);
+        cpu_single_env->stopped = 1;
     }
     qemu_notify_event();
 }
-- 
1.6.2.2

[PATCH 8/9] kvm_send_ipi

[Glauber Costa]

Provide a wrapper to pthread kill. It is more elegant
because we only need to pass a CPUState pointer, meaning
"signall this cpu"

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 6d556b8..c89146d 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -66,6 +66,11 @@ static CPUState *kvm_debug_cpu_requested;
 
 static uint64_t phys_ram_size;
 
+static inline void kvm_send_ipi(CPUState *env)
+{
+    pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
+}
+
 /* The list of ioperm_data */
 static LIST_HEAD(, ioperm_data) ioperm_head;
 
@@ -1712,7 +1717,7 @@ static void on_vcpu(CPUState *env, void (*func)(void *data), void *data)
     wi.next = NULL;
     wi.done = false;
 
-    pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
+    kvm_send_ipi(env);
     while (!wi.done)
         qemu_cond_wait(&qemu_work_cond);
 }
@@ -1744,7 +1749,7 @@ void kvm_update_interrupt_request(CPUState *env)
         if (signal) {
             env->kvm_cpu_state.signalled = 1;
             if (env->kvm_cpu_state.thread)
-                pthread_kill(env->kvm_cpu_state.thread, SIG_IPI);
+                kvm_send_ipi(env);
         }
     }
 }
@@ -1892,7 +1897,7 @@ static void pause_all_threads(void)
     while (penv) {
         if (penv != cpu_single_env) {
             penv->stop = 1;
-            pthread_kill(penv->kvm_cpu_state.thread, SIG_IPI);
+            kvm_send_ipi(penv);
         } else {
             penv->stop = 0;
             penv->stopped = 1;
@@ -1914,7 +1919,7 @@ static void resume_all_threads(void)
     while (penv) {
         penv->stop = 0;
         penv->stopped = 0;
-        pthread_kill(penv->kvm_cpu_state.thread, SIG_IPI);
+        kvm_send_ipi(penv);
         penv = (CPUState *)penv->next_cpu;
     }
 }
-- 
1.6.2.2

[PATCH 9/9] remove kvm_abi variable

[Glauber Costa]

We're not using this for anything

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu-kvm.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index c89146d..b9eed8e 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -43,7 +43,6 @@ int kvm_pit = 1;
 int kvm_pit_reinject = 1;
 int kvm_nested = 0;
 
-
 static KVMState *kvm_state;
 kvm_context_t kvm_context;
 
@@ -84,7 +83,6 @@ static LIST_HEAD(, ioperm_data) ioperm_head;
 
 #define ALIGN(x, y) (((x)+(y)-1) & ~((y)-1))
 
-int kvm_abi = EXPECTED_KVM_API_VERSION;
 int kvm_page_size;
 
 #ifdef KVM_CAP_SET_GUEST_DEBUG
@@ -436,7 +434,6 @@ int kvm_init(int smp_cpus)
 	    fprintf(stderr, "kvm userspace version too old\n");
 	    goto out_close;
 	}
-	kvm_abi = r;
 	kvm_page_size = getpagesize();
 	kvm_state = qemu_mallocz(sizeof(*kvm_state));
     kvm_context = &kvm_state->kvm_context;
-- 
1.6.2.2

Re: [PATCH 6/9] remove kvm_in* functions

[Gleb Natapov]

On Mon, Jul 20, 2009 at 07:10:13PM -0400, Glauber Costa wrote:
> We can use plain qemu's here, and save a couple of lines/complexity.
> I'm leaving outb for later, because the SMM thing makes it a little bit
> less trivial.
> 
I think you can remove all this black SMM magic from kvm_outb(). It is
handled in acpi.c now. Just booted WindowsXP with all this crap deleted.

> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  qemu-kvm.c |   25 ++++---------------------
>  1 files changed, 4 insertions(+), 21 deletions(-)
> 
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 26cac25..58d5de2 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -97,24 +97,6 @@ static int kvm_debug(void *opaque, void *data,
>  }
>  #endif
>  
> -static int kvm_inb(void *opaque, uint16_t addr, uint8_t *data)
> -{
> -    *data = cpu_inb(0, addr);
> -    return 0;
> -}
> -
> -static int kvm_inw(void *opaque, uint16_t addr, uint16_t *data)
> -{
> -    *data = cpu_inw(0, addr);
> -    return 0;
> -}
> -
> -static int kvm_inl(void *opaque, uint16_t addr, uint32_t *data)
> -{
> -    *data = cpu_inl(0, addr);
> -    return 0;
> -}
> -
>  #define PM_IO_BASE 0xb000
>  
>  static int kvm_outb(void *opaque, uint16_t addr, uint8_t data)
> @@ -855,15 +837,16 @@ static int handle_io(kvm_vcpu_context_t vcpu)
>  	for (i = 0; i < run->io.count; ++i) {
>  		switch (run->io.direction) {
>  		case KVM_EXIT_IO_IN:
> +			r = 0;
>  			switch (run->io.size) {
>  			case 1:
> -				r = kvm_inb(kvm->opaque, addr, p);
> +				*(uint8_t *)p = cpu_inb(kvm->opaque, addr);
>  				break;
>  			case 2:
> -				r = kvm_inw(kvm->opaque, addr, p);
> +				*(uint16_t *)p = cpu_inw(kvm->opaque, addr);
>  				break;
>  			case 4:
> -				r = kvm_inl(kvm->opaque, addr, p);
> +				*(uint32_t *)p = cpu_inl(kvm->opaque, addr);
>  				break;
>  			default:
>  				fprintf(stderr, "bad I/O size %d\n", run->io.size);
> -- 
> 1.6.2.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.

Re: [PATCH 6/9] remove kvm_in* functions

[Gleb Natapov]

On Tue, Jul 21, 2009 at 09:23:24AM -0300, Glauber Costa wrote:
> On Tue, Jul 21, 2009 at 09:11:06AM +0300, Gleb Natapov wrote:
> > On Mon, Jul 20, 2009 at 07:10:13PM -0400, Glauber Costa wrote:
> > > We can use plain qemu's here, and save a couple of lines/complexity.
> > > I'm leaving outb for later, because the SMM thing makes it a little bit
> > > less trivial.
> > > 
> > I think you can remove all this black SMM magic from kvm_outb(). It is
> > handled in acpi.c now. Just booted WindowsXP with all this crap deleted.
> Cool, I figured it out, but as I said, it is much less trivial.
> If you agree with removing it, a new patch will follow after this series is
> in
> 
I agree, but I am not the maintainer :)

--
			Gleb.

Re: [PATCH 6/9] remove kvm_in* functions

[Glauber Costa]

On Tue, Jul 21, 2009 at 09:11:06AM +0300, Gleb Natapov wrote:
> On Mon, Jul 20, 2009 at 07:10:13PM -0400, Glauber Costa wrote:
> > We can use plain qemu's here, and save a couple of lines/complexity.
> > I'm leaving outb for later, because the SMM thing makes it a little bit
> > less trivial.
> > 
> I think you can remove all this black SMM magic from kvm_outb(). It is
> handled in acpi.c now. Just booted WindowsXP with all this crap deleted.
Cool, I figured it out, but as I said, it is much less trivial.
If you agree with removing it, a new patch will follow after this series is
in

> 
> > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > ---
> >  qemu-kvm.c |   25 ++++---------------------
> >  1 files changed, 4 insertions(+), 21 deletions(-)
> > 
> > diff --git a/qemu-kvm.c b/qemu-kvm.c
> > index 26cac25..58d5de2 100644
> > --- a/qemu-kvm.c
> > +++ b/qemu-kvm.c
> > @@ -97,24 +97,6 @@ static int kvm_debug(void *opaque, void *data,
> >  }
> >  #endif
> >  
> > -static int kvm_inb(void *opaque, uint16_t addr, uint8_t *data)
> > -{
> > -    *data = cpu_inb(0, addr);
> > -    return 0;
> > -}
> > -
> > -static int kvm_inw(void *opaque, uint16_t addr, uint16_t *data)
> > -{
> > -    *data = cpu_inw(0, addr);
> > -    return 0;
> > -}
> > -
> > -static int kvm_inl(void *opaque, uint16_t addr, uint32_t *data)
> > -{
> > -    *data = cpu_inl(0, addr);
> > -    return 0;
> > -}
> > -
> >  #define PM_IO_BASE 0xb000
> >  
> >  static int kvm_outb(void *opaque, uint16_t addr, uint8_t data)
> > @@ -855,15 +837,16 @@ static int handle_io(kvm_vcpu_context_t vcpu)
> >  	for (i = 0; i < run->io.count; ++i) {
> >  		switch (run->io.direction) {
> >  		case KVM_EXIT_IO_IN:
> > +			r = 0;
> >  			switch (run->io.size) {
> >  			case 1:
> > -				r = kvm_inb(kvm->opaque, addr, p);
> > +				*(uint8_t *)p = cpu_inb(kvm->opaque, addr);
> >  				break;
> >  			case 2:
> > -				r = kvm_inw(kvm->opaque, addr, p);
> > +				*(uint16_t *)p = cpu_inw(kvm->opaque, addr);
> >  				break;
> >  			case 4:
> > -				r = kvm_inl(kvm->opaque, addr, p);
> > +				*(uint32_t *)p = cpu_inl(kvm->opaque, addr);
> >  				break;
> >  			default:
> >  				fprintf(stderr, "bad I/O size %d\n", run->io.size);
> > -- 
> > 1.6.2.2
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe kvm" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> 			Gleb.

Re: [PATCH 3/9] change order of kvm_init call.

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

Glauber Costa wrote:
> The goal is to get rid of the call to kvm_init. But those things
> are subtle, and often break. So do it in a separate patch, to help
> finding potential issues in future bisections.

Found such an issued: This patch triggers a segfault if no kvm modules
are loaded and you start qemu without -no-kvm. Please have a look.

Jan

> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  vl.c |   18 +++++++++---------
>  1 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index f4e4d0f..86a6d70 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -5748,15 +5748,6 @@ int main(int argc, char **argv, char **envp)
>          signal(SIGTTIN, SIG_IGN);
>      }
>  
> -#ifdef CONFIG_KVM
> -    if (kvm_enabled()) {
> -	if (kvm_init(smp_cpus) < 0) {
> -	    fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
> -	    exit(1);
> -	}
> -    }
> -#endif
> -
>      if (pid_file && qemu_create_pidfile(pid_file) != 0) {
>          if (daemonize) {
>              uint8_t status = 1;
> @@ -5956,6 +5947,15 @@ int main(int argc, char **argv, char **envp)
>      }
>  #endif
>  
> +#ifdef CONFIG_KVM
> +    if (kvm_enabled()) {
> +	if (kvm_init(smp_cpus) < 0) {
> +	    fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
> +	    exit(1);
> +	}
> +    }
> +#endif
> +
>      if (monitor_device) {
>          monitor_hd = qemu_chr_open("monitor", monitor_device, NULL);
>          if (!monitor_hd) {



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [PATCH 3/9] change order of kvm_init call.

[Glauber Costa]

On Sun, Jul 26, 2009 at 08:59:44PM +0200, Jan Kiszka wrote:
> Glauber Costa wrote:
> > The goal is to get rid of the call to kvm_init. But those things
> > are subtle, and often break. So do it in a separate patch, to help
> > finding potential issues in future bisections.
> 
> Found such an issued: This patch triggers a segfault if no kvm modules
> are loaded and you start qemu without -no-kvm. Please have a look.
> 
> Jan

ok, the culprit seems to be a 

    if (kvm_enabled())
        return;

in the beginning of code_gen_alloc.

It is 7f3d0cbe, by Avi, and according to changelog, suggested by anthony.
I however, fail to realise the purpose of this optimization. For one thing,
it totally dictates that kvm has absolutely to be enabled or disabled prior
to this point. No mind changing later. Also, the real deal is to be able
to compile out tcg entirely. The strategy of just disabling the code gen
alloc is a minor nitpick that just papers over this.


So I'd say let's revert it. But I'm open to suggestions.

> 
> > 
> > Signed-off-by: Glauber Costa <glommer@redhat.com>
> > ---
> >  vl.c |   18 +++++++++---------
> >  1 files changed, 9 insertions(+), 9 deletions(-)
> > 
> > diff --git a/vl.c b/vl.c
> > index f4e4d0f..86a6d70 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -5748,15 +5748,6 @@ int main(int argc, char **argv, char **envp)
> >          signal(SIGTTIN, SIG_IGN);
> >      }
> >  
> > -#ifdef CONFIG_KVM
> > -    if (kvm_enabled()) {
> > -	if (kvm_init(smp_cpus) < 0) {
> > -	    fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
> > -	    exit(1);
> > -	}
> > -    }
> > -#endif
> > -
> >      if (pid_file && qemu_create_pidfile(pid_file) != 0) {
> >          if (daemonize) {
> >              uint8_t status = 1;
> > @@ -5956,6 +5947,15 @@ int main(int argc, char **argv, char **envp)
> >      }
> >  #endif
> >  
> > +#ifdef CONFIG_KVM
> > +    if (kvm_enabled()) {
> > +	if (kvm_init(smp_cpus) < 0) {
> > +	    fprintf(stderr, "Could not initialize KVM, will disable KVM support\n");
> > +	    exit(1);
> > +	}
> > +    }
> > +#endif
> > +
> >      if (monitor_device) {
> >          monitor_hd = qemu_chr_open("monitor", monitor_device, NULL);
> >          if (!monitor_hd) {
> 
> 

Re: [PATCH 3/9] change order of kvm_init call.

[Anthony Liguori]

Glauber Costa wrote:
> On Sun, Jul 26, 2009 at 08:59:44PM +0200, Jan Kiszka wrote:
>   
>> Glauber Costa wrote:
>>     
>>> The goal is to get rid of the call to kvm_init. But those things
>>> are subtle, and often break. So do it in a separate patch, to help
>>> finding potential issues in future bisections.
>>>       
>> Found such an issued: This patch triggers a segfault if no kvm modules
>> are loaded and you start qemu without -no-kvm. Please have a look.
>>
>> Jan
>>     
>
> ok, the culprit seems to be a 
>
>     if (kvm_enabled())
>         return;
>
> in the beginning of code_gen_alloc.
>
> It is 7f3d0cbe, by Avi, and according to changelog, suggested by anthony.
> I however, fail to realise the purpose of this optimization. For one thing,
> it totally dictates that kvm has absolutely to be enabled or disabled prior
> to this point. No mind changing later. Also, the real deal is to be able
> to compile out tcg entirely. The strategy of just disabling the code gen
> alloc is a minor nitpick that just papers over this.
>   

I agree with you in principle but I think reverting this papers over an 
issue.

Why are we touching code_gen_ptr when using KVM?  Can someone post the 
full back trace?

-- 
Regards,

Anthony Liguori

Re: [PATCH 3/9] change order of kvm_init call.

[Glauber Costa]

On Mon, Jul 27, 2009 at 12:49:06PM -0500, Anthony Liguori wrote:
> Glauber Costa wrote:
>> On Sun, Jul 26, 2009 at 08:59:44PM +0200, Jan Kiszka wrote:
>>   
>>> Glauber Costa wrote:
>>>     
>>>> The goal is to get rid of the call to kvm_init. But those things
>>>> are subtle, and often break. So do it in a separate patch, to help
>>>> finding potential issues in future bisections.
>>>>       
>>> Found such an issued: This patch triggers a segfault if no kvm modules
>>> are loaded and you start qemu without -no-kvm. Please have a look.
>>>
>>> Jan
>>>     
>>
>> ok, the culprit seems to be a 
>>
>>     if (kvm_enabled())
>>         return;
>>
>> in the beginning of code_gen_alloc.
>>
>> It is 7f3d0cbe, by Avi, and according to changelog, suggested by anthony.
>> I however, fail to realise the purpose of this optimization. For one thing,
>> it totally dictates that kvm has absolutely to be enabled or disabled prior
>> to this point. No mind changing later. Also, the real deal is to be able
>> to compile out tcg entirely. The strategy of just disabling the code gen
>> alloc is a minor nitpick that just papers over this.
>>   
>
> I agree with you in principle but I think reverting this papers over an  
> issue.
>
> Why are we touching code_gen_ptr when using KVM?  Can someone post the  
> full back trace?
we're not.

The issue happens exactly when the kvm modules are not loaded, then we're failing
to initialize kvm. However, in the patch that raised this issue, I'm moving
KVM initialization to after this code path. And in qemu-kvm.git, kvm is
enabled-by-default. So tcg code would think kvm is enabled and skip initialization,
while kvm code will fail to really initialize itself later.

Result? Mayhem.

Re: [PATCH 3/9] change order of kvm_init call.

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2232 bytes --]

Glauber Costa wrote:
> On Mon, Jul 27, 2009 at 12:49:06PM -0500, Anthony Liguori wrote:
>> Glauber Costa wrote:
>>> On Sun, Jul 26, 2009 at 08:59:44PM +0200, Jan Kiszka wrote:
>>>   
>>>> Glauber Costa wrote:
>>>>     
>>>>> The goal is to get rid of the call to kvm_init. But those things
>>>>> are subtle, and often break. So do it in a separate patch, to help
>>>>> finding potential issues in future bisections.
>>>>>       
>>>> Found such an issued: This patch triggers a segfault if no kvm modules
>>>> are loaded and you start qemu without -no-kvm. Please have a look.
>>>>
>>>> Jan
>>>>     
>>> ok, the culprit seems to be a 
>>>
>>>     if (kvm_enabled())
>>>         return;
>>>
>>> in the beginning of code_gen_alloc.
>>>
>>> It is 7f3d0cbe, by Avi, and according to changelog, suggested by anthony.
>>> I however, fail to realise the purpose of this optimization. For one thing,
>>> it totally dictates that kvm has absolutely to be enabled or disabled prior
>>> to this point. No mind changing later. Also, the real deal is to be able
>>> to compile out tcg entirely. The strategy of just disabling the code gen
>>> alloc is a minor nitpick that just papers over this.
>>>   
>> I agree with you in principle but I think reverting this papers over an  
>> issue.
>>
>> Why are we touching code_gen_ptr when using KVM?  Can someone post the  
>> full back trace?
> we're not.
> 
> The issue happens exactly when the kvm modules are not loaded, then we're failing
> to initialize kvm. However, in the patch that raised this issue, I'm moving
> KVM initialization to after this code path. And in qemu-kvm.git, kvm is
> enabled-by-default. So tcg code would think kvm is enabled and skip initialization,
> while kvm code will fail to really initialize itself later.
> 
> Result? Mayhem.
> 

I think we should simply resolves this the way upstream does: Do not
start if modules are missing and -no-kvm is omitted - or even switch
over to -enable-kvm as I think you already suggested in some other
thread. Then we can either fail or succeed, but not fall back more or
less silently. This falling back of qemu-kvm to tcg is a constant source
of confusion anyway.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [PATCH 3/9] change order of kvm_init call.

[Glauber Costa]

On Mon, Jul 27, 2009 at 08:10:24PM +0200, Jan Kiszka wrote:
> Glauber Costa wrote:
> > On Mon, Jul 27, 2009 at 12:49:06PM -0500, Anthony Liguori wrote:
> >> Glauber Costa wrote:
> >>> On Sun, Jul 26, 2009 at 08:59:44PM +0200, Jan Kiszka wrote:
> >>>   
> >>>> Glauber Costa wrote:
> >>>>     
> >>>>> The goal is to get rid of the call to kvm_init. But those things
> >>>>> are subtle, and often break. So do it in a separate patch, to help
> >>>>> finding potential issues in future bisections.
> >>>>>       
> >>>> Found such an issued: This patch triggers a segfault if no kvm modules
> >>>> are loaded and you start qemu without -no-kvm. Please have a look.
> >>>>
> >>>> Jan
> >>>>     
> >>> ok, the culprit seems to be a 
> >>>
> >>>     if (kvm_enabled())
> >>>         return;
> >>>
> >>> in the beginning of code_gen_alloc.
> >>>
> >>> It is 7f3d0cbe, by Avi, and according to changelog, suggested by anthony.
> >>> I however, fail to realise the purpose of this optimization. For one thing,
> >>> it totally dictates that kvm has absolutely to be enabled or disabled prior
> >>> to this point. No mind changing later. Also, the real deal is to be able
> >>> to compile out tcg entirely. The strategy of just disabling the code gen
> >>> alloc is a minor nitpick that just papers over this.
> >>>   
> >> I agree with you in principle but I think reverting this papers over an  
> >> issue.
> >>
> >> Why are we touching code_gen_ptr when using KVM?  Can someone post the  
> >> full back trace?
> > we're not.
> > 
> > The issue happens exactly when the kvm modules are not loaded, then we're failing
> > to initialize kvm. However, in the patch that raised this issue, I'm moving
> > KVM initialization to after this code path. And in qemu-kvm.git, kvm is
> > enabled-by-default. So tcg code would think kvm is enabled and skip initialization,
> > while kvm code will fail to really initialize itself later.
> > 
> > Result? Mayhem.
> > 
> 
> I think we should simply resolves this the way upstream does: Do not
> start if modules are missing and -no-kvm is omitted - or even switch
> over to -enable-kvm as I think you already suggested in some other
> thread. Then we can either fail or succeed, but not fall back more or
> less silently. This falling back of qemu-kvm to tcg is a constant source
> of confusion anyway.

switching to --enable-kvm would be my preferred solution, but guys from
mgmt tools may not like it.

Re: [PATCH 3/9] change order of kvm_init call.

[Daniel P. Berrange]

On Mon, Jul 27, 2009 at 03:20:08PM -0300, Glauber Costa wrote:
> On Mon, Jul 27, 2009 at 08:10:24PM +0200, Jan Kiszka wrote:
> > 
> > I think we should simply resolves this the way upstream does: Do not
> > start if modules are missing and -no-kvm is omitted - or even switch
> > over to -enable-kvm as I think you already suggested in some other
> > thread. Then we can either fail or succeed, but not fall back more or
> > less silently. This falling back of qemu-kvm to tcg is a constant source
> > of confusion anyway.
> 
> switching to --enable-kvm would be my preferred solution, but guys from
> mgmt tools may not like it.

Totally agree that it should never ever fallback to a  different mode than
the one requested, since falling  back from KVM to QEMU simply means the 
user doesn't discover the problem till their  VM install has wasted an 
hour of their time. Personally I would vote for --accelmode qemu|kvm|kqemu 
since it is more future proof, but I'm not too bothered if people prefer 
to have --enable-kvm on the grounds that kqemu is being killed off. 
libvirt just needs a reliable way to request one of qemu, kvm, or kqemu, 
and either get an error message, or have the requested mode work.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [PATCH 3/9] change order of kvm_init call.

[Anthony Liguori]

Glauber Costa wrote:
> we're not.
>
> The issue happens exactly when the kvm modules are not loaded, then we're failing
> to initialize kvm. However, in the patch that raised this issue, I'm moving
> KVM initialization to after this code path. And in qemu-kvm.git, kvm is
> enabled-by-default. So tcg code would think kvm is enabled and skip initialization,
> while kvm code will fail to really initialize itself later.
>   

That's pure evilness.

Good argument to never fall back to TCG.

-- 
Regards,

Anthony Liguori

Re: [PATCH 3/9] change order of kvm_init call.

[Glauber Costa]

On Mon, Jul 27, 2009 at 07:28:17PM +0100, Daniel P. Berrange wrote:
> On Mon, Jul 27, 2009 at 03:20:08PM -0300, Glauber Costa wrote:
> > On Mon, Jul 27, 2009 at 08:10:24PM +0200, Jan Kiszka wrote:
> > > 
> > > I think we should simply resolves this the way upstream does: Do not
> > > start if modules are missing and -no-kvm is omitted - or even switch
> > > over to -enable-kvm as I think you already suggested in some other
> > > thread. Then we can either fail or succeed, but not fall back more or
> > > less silently. This falling back of qemu-kvm to tcg is a constant source
> > > of confusion anyway.
> > 
> > switching to --enable-kvm would be my preferred solution, but guys from
> > mgmt tools may not like it.
> 
> Totally agree that it should never ever fallback to a  different mode than
> the one requested, since falling  back from KVM to QEMU simply means the 
> user doesn't discover the problem till their  VM install has wasted an 
> hour of their time. Personally I would vote for --accelmode qemu|kvm|kqemu 
> since it is more future proof, but I'm not too bothered if people prefer 
> to have --enable-kvm on the grounds that kqemu is being killed off. 
> libvirt just needs a reliable way to request one of qemu, kvm, or kqemu, 
> and either get an error message, or have the requested mode work.
The big problem here is that in qemu-kvm.git, kvm happens without any user request.
That would be the advantage of --enable-kvm or --accelmode, or whatever.
Simply changing the default to kill the VM if we fail to initialize KVM is cumbersome,
because it would mean that users of pure tcg would have to add an option for a
basic VM to work.

Re: [PATCH 3/9] change order of kvm_init call.

[Daniel P. Berrange]

On Mon, Jul 27, 2009 at 03:38:57PM -0300, Glauber Costa wrote:
> On Mon, Jul 27, 2009 at 07:28:17PM +0100, Daniel P. Berrange wrote:
> > On Mon, Jul 27, 2009 at 03:20:08PM -0300, Glauber Costa wrote:
> > > On Mon, Jul 27, 2009 at 08:10:24PM +0200, Jan Kiszka wrote:
> > > > 
> > > > I think we should simply resolves this the way upstream does: Do not
> > > > start if modules are missing and -no-kvm is omitted - or even switch
> > > > over to -enable-kvm as I think you already suggested in some other
> > > > thread. Then we can either fail or succeed, but not fall back more or
> > > > less silently. This falling back of qemu-kvm to tcg is a constant source
> > > > of confusion anyway.
> > > 
> > > switching to --enable-kvm would be my preferred solution, but guys from
> > > mgmt tools may not like it.
> > 
> > Totally agree that it should never ever fallback to a  different mode than
> > the one requested, since falling  back from KVM to QEMU simply means the 
> > user doesn't discover the problem till their  VM install has wasted an 
> > hour of their time. Personally I would vote for --accelmode qemu|kvm|kqemu 
> > since it is more future proof, but I'm not too bothered if people prefer 
> > to have --enable-kvm on the grounds that kqemu is being killed off. 
> > libvirt just needs a reliable way to request one of qemu, kvm, or kqemu, 
> > and either get an error message, or have the requested mode work.
> The big problem here is that in qemu-kvm.git, kvm happens without any user request.
> That would be the advantage of --enable-kvm or --accelmode, or whatever.
> Simply changing the default to kill the VM if we fail to initialize KVM is cumbersome,
> because it would mean that users of pure tcg would have to add an option for a
> basic VM to work.

Well, we could go for logic like:

 * No arg given          => try kvm, try kqemu, try tcg
 * --accelmode arg given => try $arg, and fail if unavailable

then libvirt would simply always supply --accelmode for all VMs,
while people running qemu manually would get best available 

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

Re: [PATCH 3/9] change order of kvm_init call.

[Glauber Costa]

On Mon, Jul 27, 2009 at 07:44:27PM +0100, Daniel P. Berrange wrote:
> On Mon, Jul 27, 2009 at 03:38:57PM -0300, Glauber Costa wrote:
> > On Mon, Jul 27, 2009 at 07:28:17PM +0100, Daniel P. Berrange wrote:
> > > On Mon, Jul 27, 2009 at 03:20:08PM -0300, Glauber Costa wrote:
> > > > On Mon, Jul 27, 2009 at 08:10:24PM +0200, Jan Kiszka wrote:
> > > > > 
> > > > > I think we should simply resolves this the way upstream does: Do not
> > > > > start if modules are missing and -no-kvm is omitted - or even switch
> > > > > over to -enable-kvm as I think you already suggested in some other
> > > > > thread. Then we can either fail or succeed, but not fall back more or
> > > > > less silently. This falling back of qemu-kvm to tcg is a constant source
> > > > > of confusion anyway.
> > > > 
> > > > switching to --enable-kvm would be my preferred solution, but guys from
> > > > mgmt tools may not like it.
> > > 
> > > Totally agree that it should never ever fallback to a  different mode than
> > > the one requested, since falling  back from KVM to QEMU simply means the 
> > > user doesn't discover the problem till their  VM install has wasted an 
> > > hour of their time. Personally I would vote for --accelmode qemu|kvm|kqemu 
> > > since it is more future proof, but I'm not too bothered if people prefer 
> > > to have --enable-kvm on the grounds that kqemu is being killed off. 
> > > libvirt just needs a reliable way to request one of qemu, kvm, or kqemu, 
> > > and either get an error message, or have the requested mode work.
> > The big problem here is that in qemu-kvm.git, kvm happens without any user request.
> > That would be the advantage of --enable-kvm or --accelmode, or whatever.
> > Simply changing the default to kill the VM if we fail to initialize KVM is cumbersome,
> > because it would mean that users of pure tcg would have to add an option for a
> > basic VM to work.
> 
> Well, we could go for logic like:
> 
>  * No arg given          => try kvm, try kqemu, try tcg
>  * --accelmode arg given => try $arg, and fail if unavailable
> 
> then libvirt would simply always supply --accelmode for all VMs,
> while people running qemu manually would get best available 
were best available can mean a crash.
If we're getting this sort of things now, that we're in fall back mode,
and thus testing it once in a while, imagine when most of us don't even
bother. The only sane semantics to me is:

  * No arg given => tcg.
  * some arg given: try it, and if we fail, exit.