[Qemu-devel] [PATCH 1/2] Move KVM init to arch_init.c, compile vl.c once

[Qemu-devel] [PATCH 1/2] Move KVM init to arch_init.c, compile vl.c once

[Blue Swirl]

Remove dependency of vl.c to KVM, then we can partially revert
b33612d03540fda7fa67485f1c20395beb7a2bf0.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 Makefile.objs   |    2 +-
 Makefile.target |    2 +-
 arch_init.c     |    5 +++++
 arch_init.h     |    1 +
 kvm-all.c       |    3 +++
 kvm.h           |    8 ++++++++
 vl.c            |   16 +++++-----------
 7 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 11e44a0..cb2ec2c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -128,7 +128,7 @@ user-obj-y += cutils.o cache-utils.o
 # libhw

 hw-obj-y =
-hw-obj-y += loader.o
+hw-obj-y += vl.o loader.o
 hw-obj-y += virtio.o virtio-console.o
 hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.o
 hw-obj-y += watchdog.o
diff --git a/Makefile.target b/Makefile.target
index 167fc8d..2aa02f5 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -161,7 +161,7 @@ endif #CONFIG_BSD_USER
 # System emulator target
 ifdef CONFIG_SOFTMMU

-obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o vl.o
+obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
 obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o
virtio-serial-bus.o
diff --git a/arch_init.c b/arch_init.c
index cfc03ea..055e65d 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -508,3 +508,8 @@ int xen_available(void)
     return 0;
 #endif
 }
+
+int kvm_maybe_init(int smp_cpus)
+{
+    return kvm_init(smp_cpus);
+}
diff --git a/arch_init.h b/arch_init.h
index 682890c..52dd327 100644
--- a/arch_init.h
+++ b/arch_init.h
@@ -29,5 +29,6 @@ void cpudef_init(void);
 int audio_available(void);
 int kvm_available(void);
 int xen_available(void);
+int kvm_maybe_init(int smp_cpus);

 #endif
diff --git a/kvm-all.c b/kvm-all.c
index 7aa5e57..dc99aae 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -51,6 +51,8 @@ typedef struct KVMSlot

 typedef struct kvm_dirty_log KVMDirtyLog;

+int kvm_allowed = 0;
+
 struct KVMState
 {
     KVMSlot slots[32];
@@ -670,6 +672,7 @@ int kvm_init(int smp_cpus)

     kvm_state = s;
     cpu_register_phys_memory_client(&kvm_cpu_phys_memory_client);
+    kvm_allowed = 1;

     return 0;

diff --git a/kvm.h b/kvm.h
index 1e5be27..979f640 100644
--- a/kvm.h
+++ b/kvm.h
@@ -34,7 +34,15 @@ struct kvm_run;

 /* external API */

+#ifdef CONFIG_KVM
 int kvm_init(int smp_cpus);
+#else
+static inline
+int kvm_init(int smp_cpus)
+{
+    return -ENOSYS;
+}
+#endif

 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUState *env);
diff --git a/vl.c b/vl.c
index 6768cf1..6958b2c 100644
--- a/vl.c
+++ b/vl.c
@@ -145,7 +145,6 @@ int main(int argc, char **argv)
 #include "dma.h"
 #include "audio/audio.h"
 #include "migration.h"
-#include "kvm.h"
 #include "balloon.h"
 #include "qemu-option.h"
 #include "qemu-config.h"
@@ -241,7 +240,6 @@ uint8_t qemu_uuid[16];
 static QEMUBootSetHandler *boot_set_handler;
 static void *boot_set_opaque;

-int kvm_allowed = 0;
 uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;

@@ -2649,6 +2647,7 @@ int main(int argc, char **argv, char **envp)
 #endif
     int show_vnc_port = 0;
     int defconfig = 1;
+    int enable_kvm = 0;

     error_set_progname(argv[0]);

@@ -3239,7 +3238,7 @@ int main(int argc, char **argv, char **envp)
                     printf("Option %s not supported for this
target\n", popt->name);
                     exit(1);
                 }
-                kvm_allowed = 1;
+                enable_kvm = 1;
                 break;
             case QEMU_OPTION_usb:
                 usb_enabled = 1;
@@ -3585,14 +3584,9 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }

-    if (kvm_enabled()) {
-        int ret;
-
-        ret = kvm_init(smp_cpus);
-        if (ret < 0) {
-            fprintf(stderr, "failed to initialize KVM\n");
-            exit(1);
-        }
+    if (enable_kvm && kvm_maybe_init(smp_cpus) < 0) {
+        fprintf(stderr, "failed to initialize KVM\n");
+        exit(1);
     }

     if (qemu_init_main_loop()) {
-- 
1.6.2.4

[Qemu-devel] [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Paolo Bonzini]

This allows limited use of kvm functions (which will return ENOSYS)
even in once-compiled modules.  The patch also improves a bit the error
messages for KVM initialization.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
        What about this instead?  I don't like that kvm-stub.c is
        compiled per-target too, but it's basically impossible to
        change this without major changes to the build system.

 Makefile.target |    2 +
 kvm-all.c       |    6 ++-
 kvm-stub.c      |  135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kvm.h           |   13 +----
 vl.c            |   16 +++----
 5 files changed, 151 insertions(+), 21 deletions(-)
 create mode 100644 kvm-stub.c

diff --git a/Makefile.target b/Makefile.target
index 167fc8d..c504617 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -1,6 +1,7 @@
 # -*- Mode: makefile -*-
 
 GENERATED_HEADERS = config-target.h
+CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
 
 include ../config-host.mak
 include config-devices.mak
@@ -170,6 +171,7 @@ obj-y += vhost_net.o
 obj-$(CONFIG_VHOST_NET) += vhost.o
 obj-y += rwhandler.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
+obj-$(CONFIG_NO_KVM) += kvm-stub.o
 LIBS+=-lz
 
 QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
diff --git a/kvm-all.c b/kvm-all.c
index 7aa5e57..373fd34 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1157,9 +1157,9 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset)
     return r;
 }
 
-#ifdef KVM_IOEVENTFD
 int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
 {
+#ifdef KVM_IOEVENTFD
     struct kvm_ioeventfd kick = {
         .datamatch = val,
         .addr = addr,
@@ -1176,5 +1176,7 @@ int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
     if (r < 0)
         return r;
     return 0;
-}
+#else
+    return -ENOSYS;
 #endif
+}
diff --git a/kvm-stub.c b/kvm-stub.c
new file mode 100644
index 0000000..80bb908
--- /dev/null
+++ b/kvm-stub.c
@@ -0,0 +1,135 @@
+/*
+ * QEMU KVM stub
+ *
+ * Copyright Red Hat, Inc. 2010
+ *
+ * Author: Paolo Bonzini     <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "sysemu.h"
+#include "hw/hw.h"
+#include "gdbstub.h"
+#include "kvm.h"
+
+int kvm_irqchip_in_kernel(void)
+{
+    return 0;
+}
+
+int kvm_pit_in_kernel(void)
+{
+    return 0;
+}
+
+
+int kvm_init_vcpu(CPUState *env)
+{
+    return -ENOSYS;
+}
+
+int kvm_log_start(target_phys_addr_t phys_addr, ram_addr_t size)
+{
+    return -ENOSYS;
+}
+
+int kvm_log_stop(target_phys_addr_t phys_addr, ram_addr_t size)
+{
+    return -ENOSYS;
+}
+
+int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
+{
+    return -ENOSYS;
+}
+
+int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
+{
+    return -ENOSYS;
+}
+
+int kvm_check_extension(KVMState *s, unsigned int extension)
+{
+    return 0;
+}
+
+int kvm_init(int smp_cpus)
+{
+    return -ENOSYS;
+}
+
+void kvm_flush_coalesced_mmio_buffer(void)
+{
+}
+
+void kvm_cpu_synchronize_state(CPUState *env)
+{
+}
+
+void kvm_cpu_synchronize_post_reset(CPUState *env)
+{
+}
+
+void kvm_cpu_synchronize_post_init(CPUState *env)
+{
+}
+
+int kvm_cpu_exec(CPUState *env)
+{
+    abort ();
+}
+
+int kvm_has_sync_mmu(void)
+{
+    return 0;
+}
+
+int kvm_has_vcpu_events(void)
+{
+    return 0;
+}
+
+int kvm_has_robust_singlestep(void)
+{
+    return 0;
+}
+
+void kvm_setup_guest_memory(void *start, size_t size)
+{
+}
+
+int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap)
+{
+    tb_flush(env);
+    return 0;
+}
+
+int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr,
+                          target_ulong len, int type)
+{
+    return -EINVAL;
+}
+
+int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr,
+                          target_ulong len, int type)
+{
+    return -EINVAL;
+}
+
+void kvm_remove_all_breakpoints(CPUState *current_env)
+{
+}
+
+int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset)
+{
+    abort();
+}
+
+int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
+{
+    return -ENOSYS;
+}
diff --git a/kvm.h b/kvm.h
index 1e5be27..ea3c97d 100644
--- a/kvm.h
+++ b/kvm.h
@@ -23,8 +23,9 @@
 #include <linux/kvm.h>
 #endif
 
-#ifdef CONFIG_KVM
 extern int kvm_allowed;
+
+#if defined CONFIG_KVM || !defined NEED_CPU_H
 #define kvm_enabled() (kvm_allowed)
 #else
 #define kvm_enabled() (0)
@@ -167,15 +168,7 @@ static inline void cpu_synchronize_post_init(CPUState *env)
     }
 }
 
-#if defined(KVM_IOEVENTFD) && defined(CONFIG_KVM)
-int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign);
-#else
-static inline
-int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign)
-{
-    return -ENOSYS;
-}
 #endif
 
-#endif
+int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign);
 #endif
diff --git a/vl.c b/vl.c
index 6768cf1..9fe4682 100644
--- a/vl.c
+++ b/vl.c
@@ -3235,10 +3235,6 @@ int main(int argc, char **argv, char **envp)
                 do_smbios_option(optarg);
                 break;
             case QEMU_OPTION_enable_kvm:
-                if (!(kvm_available())) {
-                    printf("Option %s not supported for this target\n", popt->name);
-                    exit(1);
-                }
                 kvm_allowed = 1;
                 break;
             case QEMU_OPTION_usb:
@@ -3585,12 +3581,14 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (kvm_enabled()) {
-        int ret;
-
-        ret = kvm_init(smp_cpus);
+    if (kvm_allowed) {
+        int ret = kvm_init(smp_cpus);
         if (ret < 0) {
-            fprintf(stderr, "failed to initialize KVM\n");
+            if (!kvm_available()) {
+                printf("KVM not supported for this target\n");
+            } else {
+                fprintf(stderr, "failed to initialize KVM: %s\n", strerror(-ret));
+            }
             exit(1);
         }
     }
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Blue Swirl]

On 4/2/10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> This allows limited use of kvm functions (which will return ENOSYS)
>  even in once-compiled modules.  The patch also improves a bit the error
>  messages for KVM initialization.
>
>  Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>  ---
>         What about this instead?  I don't like that kvm-stub.c is
>         compiled per-target too, but it's basically impossible to
>         change this without major changes to the build system.

>
>   Makefile.target |    2 +
>   kvm-all.c       |    6 ++-
>   kvm-stub.c      |  135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   kvm.h           |   13 +----
>   vl.c            |   16 +++----
>   5 files changed, 151 insertions(+), 21 deletions(-)
>   create mode 100644 kvm-stub.c
>
>  diff --git a/Makefile.target b/Makefile.target
>  index 167fc8d..c504617 100644
>  --- a/Makefile.target
>  +++ b/Makefile.target
>  @@ -1,6 +1,7 @@
>   # -*- Mode: makefile -*-
>
>   GENERATED_HEADERS = config-target.h
>  +CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
>
>   include ../config-host.mak
>   include config-devices.mak
>  @@ -170,6 +171,7 @@ obj-y += vhost_net.o
>   obj-$(CONFIG_VHOST_NET) += vhost.o
>   obj-y += rwhandler.o
>   obj-$(CONFIG_KVM) += kvm.o kvm-all.o
>  +obj-$(CONFIG_NO_KVM) += kvm-stub.o
>   LIBS+=-lz
>
>   QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
>  diff --git a/kvm-all.c b/kvm-all.c
>  index 7aa5e57..373fd34 100644
>  --- a/kvm-all.c
>  +++ b/kvm-all.c
>  @@ -1157,9 +1157,9 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset)
>      return r;
>   }
>
>  -#ifdef KVM_IOEVENTFD
>   int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
>   {
>  +#ifdef KVM_IOEVENTFD
>      struct kvm_ioeventfd kick = {
>          .datamatch = val,
>          .addr = addr,
>  @@ -1176,5 +1176,7 @@ int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
>      if (r < 0)
>          return r;
>      return 0;
>  -}
>  +#else
>  +    return -ENOSYS;
>   #endif
>  +}
>  diff --git a/kvm-stub.c b/kvm-stub.c
>  new file mode 100644
>  index 0000000..80bb908
>  --- /dev/null
>  +++ b/kvm-stub.c
>  @@ -0,0 +1,135 @@
>  +/*
>  + * QEMU KVM stub
>  + *
>  + * Copyright Red Hat, Inc. 2010
>  + *
>  + * Author: Paolo Bonzini     <pbonzini@redhat.com>
>  + *
>  + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>  + * See the COPYING file in the top-level directory.
>  + *
>  + */
>  +
>  +#include "qemu-common.h"
>  +#include "sysemu.h"
>  +#include "hw/hw.h"
>  +#include "gdbstub.h"
>  +#include "kvm.h"
>  +
>  +int kvm_irqchip_in_kernel(void)
>  +{
>  +    return 0;
>  +}
>  +
>  +int kvm_pit_in_kernel(void)
>  +{
>  +    return 0;
>  +}
>  +
>  +
>  +int kvm_init_vcpu(CPUState *env)
>  +{
>  +    return -ENOSYS;
>  +}
>  +
>  +int kvm_log_start(target_phys_addr_t phys_addr, ram_addr_t size)
>  +{
>  +    return -ENOSYS;
>  +}
>  +
>  +int kvm_log_stop(target_phys_addr_t phys_addr, ram_addr_t size)
>  +{
>  +    return -ENOSYS;
>  +}
>  +
>  +int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
>  +{
>  +    return -ENOSYS;
>  +}
>  +
>  +int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
>  +{
>  +    return -ENOSYS;
>  +}
>  +
>  +int kvm_check_extension(KVMState *s, unsigned int extension)
>  +{
>  +    return 0;
>  +}
>  +
>  +int kvm_init(int smp_cpus)
>  +{
>  +    return -ENOSYS;
>  +}
>  +
>  +void kvm_flush_coalesced_mmio_buffer(void)
>  +{
>  +}
>  +
>  +void kvm_cpu_synchronize_state(CPUState *env)
>  +{
>  +}
>  +
>  +void kvm_cpu_synchronize_post_reset(CPUState *env)
>  +{
>  +}
>  +
>  +void kvm_cpu_synchronize_post_init(CPUState *env)
>  +{
>  +}
>  +
>  +int kvm_cpu_exec(CPUState *env)
>  +{
>  +    abort ();
>  +}
>  +
>  +int kvm_has_sync_mmu(void)
>  +{
>  +    return 0;
>  +}
>  +
>  +int kvm_has_vcpu_events(void)
>  +{
>  +    return 0;
>  +}
>  +
>  +int kvm_has_robust_singlestep(void)
>  +{
>  +    return 0;
>  +}
>  +
>  +void kvm_setup_guest_memory(void *start, size_t size)
>  +{
>  +}
>  +
>  +int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap)
>  +{
>  +    tb_flush(env);
>  +    return 0;
>  +}
>  +
>  +int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr,
>  +                          target_ulong len, int type)
>  +{
>  +    return -EINVAL;
>  +}
>  +
>  +int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr,
>  +                          target_ulong len, int type)
>  +{
>  +    return -EINVAL;
>  +}
>  +
>  +void kvm_remove_all_breakpoints(CPUState *current_env)
>  +{
>  +}
>  +
>  +int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset)
>  +{
>  +    abort();
>  +}
>  +
>  +int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
>  +{
>  +    return -ENOSYS;
>  +}
>  diff --git a/kvm.h b/kvm.h
>  index 1e5be27..ea3c97d 100644
>  --- a/kvm.h
>  +++ b/kvm.h
>  @@ -23,8 +23,9 @@
>   #include <linux/kvm.h>
>   #endif
>
>  -#ifdef CONFIG_KVM
>   extern int kvm_allowed;
>  +
>  +#if defined CONFIG_KVM || !defined NEED_CPU_H
>   #define kvm_enabled() (kvm_allowed)
>   #else
>   #define kvm_enabled() (0)
>  @@ -167,15 +168,7 @@ static inline void cpu_synchronize_post_init(CPUState *env)
>      }
>   }
>
>  -#if defined(KVM_IOEVENTFD) && defined(CONFIG_KVM)
>  -int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign);
>  -#else
>  -static inline
>  -int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign)
>  -{
>  -    return -ENOSYS;
>  -}
>   #endif
>
>  -#endif
>  +int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign);
>   #endif
>  diff --git a/vl.c b/vl.c
>  index 6768cf1..9fe4682 100644
>  --- a/vl.c
>  +++ b/vl.c
>  @@ -3235,10 +3235,6 @@ int main(int argc, char **argv, char **envp)
>                  do_smbios_option(optarg);
>                  break;
>              case QEMU_OPTION_enable_kvm:
>  -                if (!(kvm_available())) {
>  -                    printf("Option %s not supported for this target\n", popt->name);
>  -                    exit(1);
>  -                }
>                  kvm_allowed = 1;
>                  break;
>              case QEMU_OPTION_usb:
>  @@ -3585,12 +3581,14 @@ int main(int argc, char **argv, char **envp)
>          exit(1);
>      }
>
>  -    if (kvm_enabled()) {
>  -        int ret;
>  -
>  -        ret = kvm_init(smp_cpus);
>  +    if (kvm_allowed) {
>  +        int ret = kvm_init(smp_cpus);
>          if (ret < 0) {
>  -            fprintf(stderr, "failed to initialize KVM\n");
>  +            if (!kvm_available()) {
>  +                printf("KVM not supported for this target\n");
>  +            } else {
>  +                fprintf(stderr, "failed to initialize KVM: %s\n", strerror(-ret));
>  +            }

Could you stub also kvm_init? Then this part would be simpler because
you can call unconditionally kvm_init. I guess there would be no need
to export kvm_allowed for !CONFIG_KVM case.

[Qemu-devel] Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Paolo Bonzini]

On 04/02/2010 07:29 PM, Blue Swirl wrote:
> Could you stub also kvm_init?

It is stubbed, but it returns ENOSYS:

  +int kvm_init(int smp_cpus)
  +{
  +    return -ENOSYS;
  +}
  +

and in fact I'm relying this to remove this:

  if (!(kvm_available())) {
      printf("Option %s not supported for this target\n", popt->name);
      exit(1);
  }

and instead looking for an error when I call kvm_init.  I don't see why 
kvm_init should be called if not passing -enable-kvm (which is kvm_allowed).

> Then this part would be simpler because
> you can call unconditionally kvm_init. I guess there would be no need
> to export kvm_allowed for !CONFIG_KVM case.

I don't understand what you mean.  If this doesn't clarify enough, feel 
free to pick up the patch and tweak it; I'm not going to spend much time 
on QEMU for a while.

Paolo

[Qemu-devel] Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Blue Swirl]

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

On 4/2/10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 04/02/2010 07:29 PM, Blue Swirl wrote:
>
> > Could you stub also kvm_init?
> >
>
>  It is stubbed, but it returns ENOSYS:
>
>   +int kvm_init(int smp_cpus)
>   +{
>   +    return -ENOSYS;
>   +}
>   +

Sorry, I expected this to be near the end.

>  and in fact I'm relying this to remove this:
>
>   if (!(kvm_available())) {
>      printf("Option %s not supported for this target\n", popt->name);
>      exit(1);
>   }
>
>  and instead looking for an error when I call kvm_init.  I don't see why
> kvm_init should be called if not passing -enable-kvm (which is kvm_allowed).
>
>
> > Then this part would be simpler because
> > you can call unconditionally kvm_init. I guess there would be no need
> > to export kvm_allowed for !CONFIG_KVM case.
> >
>
>  I don't understand what you mean.  If this doesn't clarify enough, feel
> free to pick up the patch and tweak it; I'm not going to spend much time on
> QEMU for a while.

I merged your patch and mine. Does it still look reasonable?

[-- Attachment #2: 0001-provide-a-stub-version-of-kvm-all.c-if-CONFIG_KVM.patch --]
[-- Type: text/x-diff, Size: 8221 bytes --]

From ecda8242df18748b73a68d3f39a89a2ac483af4d Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Fri, 2 Apr 2010 18:07:55 +0000
Subject: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

This allows limited use of kvm functions (which will return ENOSYS)
even in once-compiled modules.  The patch also improves a bit the error
messages for KVM initialization.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 Makefile.objs   |    2 +-
 Makefile.target |    4 +-
 kvm-all.c       |    9 +++-
 kvm-stub.c      |  135 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kvm.h           |   15 +-----
 sysemu.h        |    1 +
 vl.c            |   13 ++----
 7 files changed, 154 insertions(+), 25 deletions(-)
 create mode 100644 kvm-stub.c

diff --git a/Makefile.objs b/Makefile.objs
index 11e44a0..cb2ec2c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -128,7 +128,7 @@ user-obj-y += cutils.o cache-utils.o
 # libhw
 
 hw-obj-y =
-hw-obj-y += loader.o
+hw-obj-y += vl.o loader.o
 hw-obj-y += virtio.o virtio-console.o
 hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.o
 hw-obj-y += watchdog.o
diff --git a/Makefile.target b/Makefile.target
index 167fc8d..c1bfc41 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -1,6 +1,7 @@
 # -*- Mode: makefile -*-
 
 GENERATED_HEADERS = config-target.h
+CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)
 
 include ../config-host.mak
 include config-devices.mak
@@ -161,7 +162,7 @@ endif #CONFIG_BSD_USER
 # System emulator target
 ifdef CONFIG_SOFTMMU
 
-obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o vl.o
+obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
 obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o
@@ -170,6 +171,7 @@ obj-y += vhost_net.o
 obj-$(CONFIG_VHOST_NET) += vhost.o
 obj-y += rwhandler.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
+obj-$(CONFIG_NO_KVM) += kvm-stub.o
 LIBS+=-lz
 
 QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
diff --git a/kvm-all.c b/kvm-all.c
index 7aa5e57..f6317ee 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -51,6 +51,8 @@ typedef struct KVMSlot
 
 typedef struct kvm_dirty_log KVMDirtyLog;
 
+int kvm_allowed = 0;
+
 struct KVMState
 {
     KVMSlot slots[32];
@@ -670,6 +672,7 @@ int kvm_init(int smp_cpus)
 
     kvm_state = s;
     cpu_register_phys_memory_client(&kvm_cpu_phys_memory_client);
+    kvm_allowed = 1;
 
     return 0;
 
@@ -1157,9 +1160,9 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset)
     return r;
 }
 
-#ifdef KVM_IOEVENTFD
 int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
 {
+#ifdef KVM_IOEVENTFD
     struct kvm_ioeventfd kick = {
         .datamatch = val,
         .addr = addr,
@@ -1176,5 +1179,7 @@ int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
     if (r < 0)
         return r;
     return 0;
-}
+#else
+    return -ENOSYS;
 #endif
+}
diff --git a/kvm-stub.c b/kvm-stub.c
new file mode 100644
index 0000000..80bb908
--- /dev/null
+++ b/kvm-stub.c
@@ -0,0 +1,135 @@
+/*
+ * QEMU KVM stub
+ *
+ * Copyright Red Hat, Inc. 2010
+ *
+ * Author: Paolo Bonzini     <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu-common.h"
+#include "sysemu.h"
+#include "hw/hw.h"
+#include "gdbstub.h"
+#include "kvm.h"
+
+int kvm_irqchip_in_kernel(void)
+{
+    return 0;
+}
+
+int kvm_pit_in_kernel(void)
+{
+    return 0;
+}
+
+
+int kvm_init_vcpu(CPUState *env)
+{
+    return -ENOSYS;
+}
+
+int kvm_log_start(target_phys_addr_t phys_addr, ram_addr_t size)
+{
+    return -ENOSYS;
+}
+
+int kvm_log_stop(target_phys_addr_t phys_addr, ram_addr_t size)
+{
+    return -ENOSYS;
+}
+
+int kvm_coalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
+{
+    return -ENOSYS;
+}
+
+int kvm_uncoalesce_mmio_region(target_phys_addr_t start, ram_addr_t size)
+{
+    return -ENOSYS;
+}
+
+int kvm_check_extension(KVMState *s, unsigned int extension)
+{
+    return 0;
+}
+
+int kvm_init(int smp_cpus)
+{
+    return -ENOSYS;
+}
+
+void kvm_flush_coalesced_mmio_buffer(void)
+{
+}
+
+void kvm_cpu_synchronize_state(CPUState *env)
+{
+}
+
+void kvm_cpu_synchronize_post_reset(CPUState *env)
+{
+}
+
+void kvm_cpu_synchronize_post_init(CPUState *env)
+{
+}
+
+int kvm_cpu_exec(CPUState *env)
+{
+    abort ();
+}
+
+int kvm_has_sync_mmu(void)
+{
+    return 0;
+}
+
+int kvm_has_vcpu_events(void)
+{
+    return 0;
+}
+
+int kvm_has_robust_singlestep(void)
+{
+    return 0;
+}
+
+void kvm_setup_guest_memory(void *start, size_t size)
+{
+}
+
+int kvm_update_guest_debug(CPUState *env, unsigned long reinject_trap)
+{
+    tb_flush(env);
+    return 0;
+}
+
+int kvm_insert_breakpoint(CPUState *current_env, target_ulong addr,
+                          target_ulong len, int type)
+{
+    return -EINVAL;
+}
+
+int kvm_remove_breakpoint(CPUState *current_env, target_ulong addr,
+                          target_ulong len, int type)
+{
+    return -EINVAL;
+}
+
+void kvm_remove_all_breakpoints(CPUState *current_env)
+{
+}
+
+int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset)
+{
+    abort();
+}
+
+int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
+{
+    return -ENOSYS;
+}
diff --git a/kvm.h b/kvm.h
index 1e5be27..ec66b2f 100644
--- a/kvm.h
+++ b/kvm.h
@@ -23,8 +23,9 @@
 #include <linux/kvm.h>
 #endif
 
-#ifdef CONFIG_KVM
+#if defined CONFIG_KVM || !defined NEED_CPU_H
 extern int kvm_allowed;
+
 #define kvm_enabled() (kvm_allowed)
 #else
 #define kvm_enabled() (0)
@@ -34,8 +35,6 @@ struct kvm_run;
 
 /* external API */
 
-int kvm_init(int smp_cpus);
-
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUState *env);
 
@@ -167,15 +166,7 @@ static inline void cpu_synchronize_post_init(CPUState *env)
     }
 }
 
-#if defined(KVM_IOEVENTFD) && defined(CONFIG_KVM)
-int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign);
-#else
-static inline
-int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign)
-{
-    return -ENOSYS;
-}
 #endif
 
-#endif
+int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign);
 #endif
diff --git a/sysemu.h b/sysemu.h
index d0effa0..0b423db 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -243,4 +243,5 @@ void rtc_change_mon_event(struct tm *tm);
 
 void register_devices(void);
 
+int kvm_init(int smp_cpus);
 #endif
diff --git a/vl.c b/vl.c
index 6768cf1..600dc7f 100644
--- a/vl.c
+++ b/vl.c
@@ -145,7 +145,6 @@ int main(int argc, char **argv)
 #include "dma.h"
 #include "audio/audio.h"
 #include "migration.h"
-#include "kvm.h"
 #include "balloon.h"
 #include "qemu-option.h"
 #include "qemu-config.h"
@@ -241,7 +240,6 @@ uint8_t qemu_uuid[16];
 static QEMUBootSetHandler *boot_set_handler;
 static void *boot_set_opaque;
 
-int kvm_allowed = 0;
 uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;
 
@@ -2649,6 +2647,7 @@ int main(int argc, char **argv, char **envp)
 #endif
     int show_vnc_port = 0;
     int defconfig = 1;
+    int enable_kvm = 0;
 
     error_set_progname(argv[0]);
 
@@ -3235,11 +3234,7 @@ int main(int argc, char **argv, char **envp)
                 do_smbios_option(optarg);
                 break;
             case QEMU_OPTION_enable_kvm:
-                if (!(kvm_available())) {
-                    printf("Option %s not supported for this target\n", popt->name);
-                    exit(1);
-                }
-                kvm_allowed = 1;
+                enable_kvm = 1;
                 break;
             case QEMU_OPTION_usb:
                 usb_enabled = 1;
@@ -3585,12 +3580,12 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (kvm_enabled()) {
+    if (enable_kvm) {
         int ret;
 
         ret = kvm_init(smp_cpus);
         if (ret < 0) {
-            fprintf(stderr, "failed to initialize KVM\n");
+            fprintf(stderr, "failed to initialize KVM: %s\n", strerror(-ret));
             exit(1);
         }
     }
-- 
1.5.6.5

[Qemu-devel] Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Paolo Bonzini]

On 04/02/2010 08:17 PM, Blue Swirl wrote:
> I merged your patch and mine. Does it still look reasonable?

Yes, of course.  I'd rather see them committed separately though.

Paolo

[Qemu-devel] Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Blue Swirl]

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

On 4/2/10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 04/02/2010 08:17 PM, Blue Swirl wrote:
>
> > I merged your patch and mine. Does it still look reasonable?
> >
>
>  Yes, of course.  I'd rather see them committed separately though.

This version is designed to be applied after your patch. I made my
patch a bit simpler.

[-- Attachment #2: 0001-Compile-vl.c-once.patch --]
[-- Type: text/x-diff, Size: 3848 bytes --]

From 1ce35cc1adc0eb54420d6647c3dfc46445788906 Mon Sep 17 00:00:00 2001
From: Blue Swirl <blauwirbel@gmail.com>
Date: Fri, 2 Apr 2010 19:00:35 +0000
Subject: [PATCH] Compile vl.c once

Remove dependency of vl.c to KVM, then we can partially revert
b33612d03540fda7fa67485f1c20395beb7a2bf0.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 Makefile.objs   |    2 +-
 Makefile.target |    2 +-
 kvm-all.c       |    3 +++
 kvm.h           |    4 +---
 sysemu.h        |    1 +
 vl.c            |    7 +++----
 6 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index 11e44a0..cb2ec2c 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -128,7 +128,7 @@ user-obj-y += cutils.o cache-utils.o
 # libhw
 
 hw-obj-y =
-hw-obj-y += loader.o
+hw-obj-y += vl.o loader.o
 hw-obj-y += virtio.o virtio-console.o
 hw-obj-y += fw_cfg.o pci.o pci_host.o pcie_host.o
 hw-obj-y += watchdog.o
diff --git a/Makefile.target b/Makefile.target
index c504617..c1bfc41 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -162,7 +162,7 @@ endif #CONFIG_BSD_USER
 # System emulator target
 ifdef CONFIG_SOFTMMU
 
-obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o vl.o
+obj-y = arch_init.o cpus.o monitor.o machine.o gdbstub.o
 # virtio has to be here due to weird dependency between PCI and virtio-net.
 # need to fix this properly
 obj-y += virtio-blk.o virtio-balloon.o virtio-net.o virtio-pci.o virtio-serial-bus.o
diff --git a/kvm-all.c b/kvm-all.c
index 373fd34..f6317ee 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -51,6 +51,8 @@ typedef struct KVMSlot
 
 typedef struct kvm_dirty_log KVMDirtyLog;
 
+int kvm_allowed = 0;
+
 struct KVMState
 {
     KVMSlot slots[32];
@@ -670,6 +672,7 @@ int kvm_init(int smp_cpus)
 
     kvm_state = s;
     cpu_register_phys_memory_client(&kvm_cpu_phys_memory_client);
+    kvm_allowed = 1;
 
     return 0;
 
diff --git a/kvm.h b/kvm.h
index ea3c97d..ec66b2f 100644
--- a/kvm.h
+++ b/kvm.h
@@ -23,9 +23,9 @@
 #include <linux/kvm.h>
 #endif
 
+#if defined CONFIG_KVM || !defined NEED_CPU_H
 extern int kvm_allowed;
 
-#if defined CONFIG_KVM || !defined NEED_CPU_H
 #define kvm_enabled() (kvm_allowed)
 #else
 #define kvm_enabled() (0)
@@ -35,8 +35,6 @@ struct kvm_run;
 
 /* external API */
 
-int kvm_init(int smp_cpus);
-
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUState *env);
 
diff --git a/sysemu.h b/sysemu.h
index d0effa0..0b423db 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -243,4 +243,5 @@ void rtc_change_mon_event(struct tm *tm);
 
 void register_devices(void);
 
+int kvm_init(int smp_cpus);
 #endif
diff --git a/vl.c b/vl.c
index 9fe4682..1005163 100644
--- a/vl.c
+++ b/vl.c
@@ -145,7 +145,6 @@ int main(int argc, char **argv)
 #include "dma.h"
 #include "audio/audio.h"
 #include "migration.h"
-#include "kvm.h"
 #include "balloon.h"
 #include "qemu-option.h"
 #include "qemu-config.h"
@@ -241,7 +240,6 @@ uint8_t qemu_uuid[16];
 static QEMUBootSetHandler *boot_set_handler;
 static void *boot_set_opaque;
 
-int kvm_allowed = 0;
 uint32_t xen_domid;
 enum xen_mode xen_mode = XEN_EMULATE;
 
@@ -2649,6 +2647,7 @@ int main(int argc, char **argv, char **envp)
 #endif
     int show_vnc_port = 0;
     int defconfig = 1;
+    int enable_kvm = 0;
 
     error_set_progname(argv[0]);
 
@@ -3235,7 +3234,7 @@ int main(int argc, char **argv, char **envp)
                 do_smbios_option(optarg);
                 break;
             case QEMU_OPTION_enable_kvm:
-                kvm_allowed = 1;
+                enable_kvm = 1;
                 break;
             case QEMU_OPTION_usb:
                 usb_enabled = 1;
@@ -3581,7 +3580,7 @@ int main(int argc, char **argv, char **envp)
         exit(1);
     }
 
-    if (kvm_allowed) {
+    if (enable_kvm) {
         int ret = kvm_init(smp_cpus);
         if (ret < 0) {
             if (!kvm_available()) {
-- 
1.5.6.5

[Qemu-devel] Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Paolo Bonzini]

On 04/02/2010 09:04 PM, Blue Swirl wrote:
> -int kvm_init(int smp_cpus);

I had missed this; I don't see a particular reason to move this out of 
kvm.h.  Anyway I don't feel strongly about this.

Paolo

[Qemu-devel] Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Blue Swirl]

On 4/3/10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 04/02/2010 09:04 PM, Blue Swirl wrote:
>
> > -int kvm_init(int smp_cpus);
> >
>
>  I had missed this; I don't see a particular reason to move this out of
> kvm.h.  Anyway I don't feel strongly about this.

The reason is to avoid including kvm.h by vl.c.

[Qemu-devel] Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Paolo Bonzini]

On 04/03/2010 11:07 AM, Blue Swirl wrote:
> On 4/3/10, Paolo Bonzini<pbonzini@redhat.com>  wrote:
>> On 04/02/2010 09:04 PM, Blue Swirl wrote:
>>
>>> -int kvm_init(int smp_cpus);
>>>
>>
>>   I had missed this; I don't see a particular reason to move this out of
>> kvm.h.  Anyway I don't feel strongly about this.
>
> The reason is to avoid including kvm.h by vl.c.

But that's not a problem, kvm.h can be included by compiled-once files; 
that was the reason to introduce the stubs in the first place.  kvm_* 
should be declared in kvm.h.

Paolo

[Qemu-devel] Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Blue Swirl]

On 4/4/10, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 04/03/2010 11:07 AM, Blue Swirl wrote:
>
> > On 4/3/10, Paolo Bonzini<pbonzini@redhat.com>  wrote:
> >
> > > On 04/02/2010 09:04 PM, Blue Swirl wrote:
> > >
> > >
> > > > -int kvm_init(int smp_cpus);
> > > >
> > > >
> > >
> > >  I had missed this; I don't see a particular reason to move this out of
> > > kvm.h.  Anyway I don't feel strongly about this.
> > >
> >
> > The reason is to avoid including kvm.h by vl.c.
> >
>
>  But that's not a problem, kvm.h can be included by compiled-once files;
> that was the reason to introduce the stubs in the first place.  kvm_* should
> be declared in kvm.h.

That can't be safe because CONFIG_KVM will not be defined for files
compiled once. So even with the stubs, those files would use inlined
stubs where they shouldn't.

[Qemu-devel] Re: [PATCH] provide a stub version of kvm-all.c if !CONFIG_KVM

[Paolo Bonzini]

>>> The reason is to avoid including kvm.h by vl.c.
>>
>>   But that's not a problem, kvm.h can be included by compiled-once files;
>> that was the reason to introduce the stubs in the first place.  kvm_* should
>> be declared in kvm.h.
>
> That can't be safe because CONFIG_KVM will not be defined for files
> compiled once. So even with the stubs, those files would use inlined
> stubs where they shouldn't.

1) Stubs are linked, not inlined.

2) vl.c does not use kvm_enabled

3) even if it did, it is defined like this after my patch:

#if defined CONFIG_KVM || !defined NEED_CPU_H
#define kvm_enabled() (kvm_allowed)
#else
#define kvm_enabled() (0)
#endif

Paolo