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

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.

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