[Qemu-devel] kvm: let's add HYPERV capabilities (was Re: [PATCH 01/20] hyper-v: introduce Hyper-V support infrastructure.)

[Michael S. Tsirkin]

On Fri, Jan 20, 2012 at 03:26:27PM -0200, Marcelo Tosatti wrote:
> From: Vadim Rozenfeld <address@hidden>
> 
> [Jan: fix build with CONFIG_USER_ONLY]
> 
> Signed-off-by: Marcelo Tosatti <address@hidden>
> ---
>  Makefile.target      |    2 +
>  target-i386/cpuid.c  |   14 +++++++++++
>  target-i386/hyperv.c |   64 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  target-i386/hyperv.h |   43 +++++++++++++++++++++++++++++++++
>  4 files changed, 123 insertions(+), 0 deletions(-)
>  create mode 100644 target-i386/hyperv.c
>  create mode 100644 target-i386/hyperv.h
> 
> diff --git a/Makefile.target b/Makefile.target
> index 06d79b8..798dd30 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -199,6 +199,8 @@ obj-$(CONFIG_NO_KVM) += kvm-stub.o
>  obj-y += memory.o savevm.o
>  LIBS+=-lz
>  
> +obj-i386-y +=hyperv.o
> +
>  QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
>  QEMU_CFLAGS += $(VNC_SASL_CFLAGS)
>  QEMU_CFLAGS += $(VNC_JPEG_CFLAGS)
> diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
> index 91a104b..b9bfeaf 100644
> --- a/target-i386/cpuid.c
> +++ b/target-i386/cpuid.c
> @@ -27,6 +27,8 @@
>  #include "qemu-option.h"
>  #include "qemu-config.h"
>  
> +#include "hyperv.h"
> +
>  /* feature flags taken from "Intel Processor Identification and the CPUID
>   * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
>   * between feature naming conventions, aliases may be added.
> @@ -716,6 +718,14 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
> const char *cpu_model)
>                      goto error;
>                  }
>                  x86_cpu_def->tsc_khz = tsc_freq / 1000;
> +            } else if (!strcmp(featurestr, "hv_spinlocks")) {
> +                char *err;
> +                numvalue = strtoul(val, &err, 0);
> +                if (!*val || *err) {
> +                    fprintf(stderr, "bad numerical value %s\n", val);
> +                    goto error;
> +                }
> +                hyperv_set_spinlock_retries(numvalue);
>              } else {
>                  fprintf(stderr, "unrecognized feature %s\n", featurestr);
>                  goto error;
> @@ -724,6 +734,10 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
> const char *cpu_model)
>              check_cpuid = 1;
>          } else if (!strcmp(featurestr, "enforce")) {
>              check_cpuid = enforce_cpuid = 1;
> +        } else if (!strcmp(featurestr, "hv_relaxed")) {
> +            hyperv_enable_relaxed_timing(true);
> +        } else if (!strcmp(featurestr, "hv_vapic")) {
> +            hyperv_enable_vapic_recommended(true);
>          } else {
>              fprintf(stderr, "feature string `%s' not in format 
> (+feature|-feature|feature=xyz)\n", featurestr);
>              goto error;
> diff --git a/target-i386/hyperv.c b/target-i386/hyperv.c
> new file mode 100644
> index 0000000..f284e99
> --- /dev/null
> +++ b/target-i386/hyperv.c
> @@ -0,0 +1,64 @@
> +/*
> + * QEMU Hyper-V support
> + *
> + * Copyright Red Hat, Inc. 2011
> + *
> + * Author: Vadim Rozenfeld     <address@hidden>
> + *
> + * 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 "hyperv.h"
> +
> +static bool hyperv_vapic;
> +static bool hyperv_relaxed_timing;
> +static int hyperv_spinlock_attempts = HYPERV_SPINLOCK_NEVER_RETRY;
> +
> +void hyperv_enable_vapic_recommended(bool val)
> +{
> +    hyperv_vapic = val;
> +}
> +
> +void hyperv_enable_relaxed_timing(bool val)
> +{
> +    hyperv_relaxed_timing = val;
> +}
> +
> +void hyperv_set_spinlock_retries(int val)
> +{
> +    hyperv_spinlock_attempts = val;
> +    if (hyperv_spinlock_attempts < 0xFFF) {
> +        hyperv_spinlock_attempts = 0xFFF;
> +    }
> +}
> +
> +bool hyperv_enabled(void)
> +{
> +    return hyperv_hypercall_available() || hyperv_relaxed_timing_enabled();
> +}
> +
> +bool hyperv_hypercall_available(void)
> +{
> +    if (hyperv_vapic ||
> +        (hyperv_spinlock_attempts != HYPERV_SPINLOCK_NEVER_RETRY)) {
> +      return true;
> +    }
> +    return false;
> +}
> +
> +bool hyperv_vapic_recommended(void)
> +{
> +    return hyperv_vapic;
> +}
> +
> +bool hyperv_relaxed_timing_enabled(void)
> +{
> +    return hyperv_relaxed_timing;
> +}
> +
> +int hyperv_get_spinlock_retries(void)
> +{
> +    return hyperv_spinlock_attempts;
> +}
> diff --git a/target-i386/hyperv.h b/target-i386/hyperv.h
> new file mode 100644
> index 0000000..15467bf
> --- /dev/null
> +++ b/target-i386/hyperv.h
> @@ -0,0 +1,43 @@
> +/*
> + * QEMU Hyper-V support
> + *
> + * Copyright Red Hat, Inc. 2011
> + *
> + * Author: Vadim Rozenfeld     <address@hidden>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#ifndef QEMU_HW_HYPERV_H
> +#define QEMU_HW_HYPERV_H 1
> +
> +#include "qemu-common.h"
> +#include <asm/hyperv.h>
> +
> +#ifndef HYPERV_SPINLOCK_NEVER_RETRY
> +#define HYPERV_SPINLOCK_NEVER_RETRY             0xFFFFFFFF
> +#endif
> +
> +#ifndef KVM_CPUID_SIGNATURE_NEXT
> +#define KVM_CPUID_SIGNATURE_NEXT                0x40000100
> +#endif
> +
> +#ifndef CONFIG_USER_ONLY
> +void hyperv_enable_vapic_recommended(bool val);
> +void hyperv_enable_relaxed_timing(bool val);
> +void hyperv_set_spinlock_retries(int val);
> +#else
> +static inline void hyperv_enable_vapic_recommended(bool val) { }
> +static inline void hyperv_enable_relaxed_timing(bool val) { }
> +static inline void hyperv_set_spinlock_retries(int val) { }
> +#endif
> +
> +bool hyperv_enabled(void);
> +bool hyperv_hypercall_available(void);
> +bool hyperv_vapic_recommended(void);
> +bool hyperv_relaxed_timing_enabled(void);
> +int hyperv_get_spinlock_retries(void);
> +
> +#endif /* QEMU_HW_HYPERV_H */
> -- 
> 1.7.6.4


So basically, this means that you enable hyperv
features unconditionally when user requests them.
If you run qemu on an old kernel without hyperv support and request
hyperv features through flags, guest will crash.

This is not a friendly way to do this.

Linux currently exposes:

linux-headers/linux/kvm.h:#define KVM_CAP_HYPERV 44
linux-headers/linux/kvm.h:#define KVM_CAP_HYPERV_VAPIC 45
linux-headers/linux/kvm.h:#define KVM_CAP_HYPERV_SPIN 46

I think qemu should be fixed to check the appropriate
capability before enabling features:
KVM_CAP_HYPERV for relaxed
KVM_CAP_HYPERV_VAPIC for vapic
KVM_CAP_HYPERV_SPIN for spinlocks

As we will add more features we will have to add more
capabilities.
Are there any features implemented now that lack a
capability? It doesn't look like there are any ...


> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to address@hidden
> More majordomo info at  http://vger.kernel.org/majordomo-info.html