[Qemu-devel] [PATCH 0/3] Enhance kvm cpuid

[Qemu-devel] [PATCH 0/3] Enhance kvm cpuid

[Glauber Costa]

Hey guys

This is a series that aims at enhancing cpuid capabilities in qemu.
The first patch was already sent to the list, and includes suggestions
by Paul.

The other two gives us the ability to query the host for features that
are possibly non-suported, but present in the exposed qemu machine type.

kvm.h             |    1 +
target-i386/kvm.c |  101 +++++++++++++++++++++++++++++++++++++++++++++++------
2 files changed, 91 insertions(+), 11 deletions(-)

[Qemu-devel] [PATCH 1/3] expose paravirt feature list to cpuid

[Glauber Costa]

Expose the paravirt features provided by the host to the guest.
The features exposed uses qemu just as a medium, since they only
depend on a negotiation between the host and guest directly.

As a direct result, we're now able to run the kvm pvclock in qemu
based kvm guests.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 kvm.h             |    1 +
 target-i386/kvm.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/kvm.h b/kvm.h
index efce145..db58baa 100644
--- a/kvm.h
+++ b/kvm.h
@@ -17,6 +17,7 @@
 #include "config.h"
 
 #ifdef CONFIG_KVM
+#include <linux/kvm_para.h>
 extern int kvm_allowed;
 
 #define kvm_enabled() (kvm_allowed)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 2412ae4..7f4e5aa 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -33,16 +33,61 @@
     do { } while (0)
 #endif
 
+struct kvm_para_features {
+    int cap;
+    int feature;
+} para_features[] = {
+#ifdef KVM_CAP_CLOCKSOURCE
+	{ KVM_CAP_CLOCKSOURCE, KVM_FEATURE_CLOCKSOURCE },
+#endif
+#ifdef KVM_CAP_NOP_IO_DELAY
+	{ KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
+#endif
+#ifdef KVM_CAP_PV_MMU
+	{ KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
+#endif
+#ifdef KVM_CAP_CR3_CACHE
+	{ KVM_CAP_CR3_CACHE, KVM_FEATURE_CR3_CACHE },
+#endif
+};
+
+static uint32_t get_para_features(CPUState *env)
+{
+    uint32_t i, features = 0;
+
+    for (i = 0; i < ARRAY_SIZE(para_features); i++) {
+        if (kvm_ioctl(env->kvm_state, KVM_CHECK_EXTENSION, para_features[i].cap))
+            features |= (1 << para_features[i].feature);
+    }
+
+    return features;
+}
 int kvm_arch_init_vcpu(CPUState *env)
 {
     struct {
         struct kvm_cpuid cpuid;
         struct kvm_cpuid_entry entries[100];
     } __attribute__((packed)) cpuid_data;
-    uint32_t limit, i, cpuid_i;
+    uint32_t limit, i, cpuid_i = 0;
     uint32_t eax, ebx, ecx, edx;
+#ifdef KVM_CPUID_SIGNATURE
+    struct kvm_cpuid_entry *pv_ent;
+
+    /* Paravirtualization CPUIDs */
+    pv_ent = &cpuid_data.entries[cpuid_i++];
+    memset(pv_ent, 0, sizeof(*pv_ent));
+    pv_ent->function = KVM_CPUID_SIGNATURE;
+    pv_ent->eax = 0;
+    pv_ent->ebx = 0x4b4d564b; /* KVMKVMK */
+    pv_ent->ecx = 0x564b4d56; /* VMKV */
+    pv_ent->edx = 0x0000004d; /* M */
+
+    pv_ent = &cpuid_data.entries[cpuid_i++];
+    memset(pv_ent, 0, sizeof(*pv_ent));
+    pv_ent->function = KVM_CPUID_FEATURES;
+    pv_ent->eax = get_para_features(env);
+#endif
 
-    cpuid_i = 0;
 
     cpu_x86_cpuid(env, 0, &eax, &ebx, &ecx, &edx);
     limit = eax;
-- 
1.5.6.5

[Qemu-devel] [PATCH 2/3] convert cpuid registration to KVM_SET_CPUID2

[Glauber Costa]

Use KVM_SET_CPUID2 instead of KVM_SET_CPUID. This
will allow us to do registration of cpuid leaves without
multiple calls to the kernel in the future.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 target-i386/kvm.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 7f4e5aa..87ddff5 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -65,13 +65,13 @@ static uint32_t get_para_features(CPUState *env)
 int kvm_arch_init_vcpu(CPUState *env)
 {
     struct {
-        struct kvm_cpuid cpuid;
-        struct kvm_cpuid_entry entries[100];
+        struct kvm_cpuid2 cpuid;
+        struct kvm_cpuid_entry2 entries[100];
     } __attribute__((packed)) cpuid_data;
     uint32_t limit, i, cpuid_i = 0;
     uint32_t eax, ebx, ecx, edx;
 #ifdef KVM_CPUID_SIGNATURE
-    struct kvm_cpuid_entry *pv_ent;
+    struct kvm_cpuid_entry2 *pv_ent;
 
     /* Paravirtualization CPUIDs */
     pv_ent = &cpuid_data.entries[cpuid_i++];
@@ -93,7 +93,7 @@ int kvm_arch_init_vcpu(CPUState *env)
     limit = eax;
 
     for (i = 0; i <= limit; i++) {
-        struct kvm_cpuid_entry *c = &cpuid_data.entries[cpuid_i++];
+        struct kvm_cpuid_entry2 *c = &cpuid_data.entries[cpuid_i++];
 
         cpu_x86_cpuid(env, i, &eax, &ebx, &ecx, &edx);
         c->function = i;
@@ -107,7 +107,7 @@ int kvm_arch_init_vcpu(CPUState *env)
     limit = eax;
 
     for (i = 0x80000000; i <= limit; i++) {
-        struct kvm_cpuid_entry *c = &cpuid_data.entries[cpuid_i++];
+        struct kvm_cpuid_entry2 *c = &cpuid_data.entries[cpuid_i++];
 
         cpu_x86_cpuid(env, i, &eax, &ebx, &ecx, &edx);
         c->function = i;
@@ -119,7 +119,7 @@ int kvm_arch_init_vcpu(CPUState *env)
 
     cpuid_data.cpuid.nent = cpuid_i;
 
-    return kvm_vcpu_ioctl(env, KVM_SET_CPUID, &cpuid_data);
+    return kvm_vcpu_ioctl(env, KVM_SET_CPUID2, &cpuid_data);
 }
 
 static int kvm_has_msr_star(CPUState *env)
-- 
1.5.6.5

[Qemu-devel] [PATCH 3/3] mask out forbidden cpuid features

[Glauber Costa]

KVM has a (so far unused) ioctl to inform userspace
about supported cpuid bits in the current host.

The lack of this kind of checking can lead to bugs in which
a cpuid bit is exposed but not supported by the host kernel
(an example is fedora bug https://bugzilla.redhat.com/show_bug.cgi?id=481274)

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 target-i386/kvm.c |   44 +++++++++++++++++++++++++++++++++++++++-----
 1 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 87ddff5..08584dd 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -68,8 +68,18 @@ int kvm_arch_init_vcpu(CPUState *env)
         struct kvm_cpuid2 cpuid;
         struct kvm_cpuid_entry2 entries[100];
     } __attribute__((packed)) cpuid_data;
-    uint32_t limit, i, cpuid_i = 0;
+
+    struct {
+        struct kvm_cpuid2 cpuid;
+        struct kvm_cpuid_entry2 entries[100];
+    } __attribute__((packed)) cpuid_base_data;
+
+    uint32_t limit, i, r, cpuid_i = 0;
     uint32_t eax, ebx, ecx, edx;
+    struct kvm_cpuid_entry2 *func1 = NULL;
+    struct kvm_cpuid_entry2 *ext_func1 = NULL;
+    uint32_t ecx_mask = ~0U;
+    uint32_t edx_mask = ~0U;
 #ifdef KVM_CPUID_SIGNATURE
     struct kvm_cpuid_entry2 *pv_ent;
 
@@ -89,32 +99,56 @@ int kvm_arch_init_vcpu(CPUState *env)
 #endif
 
 
+    cpuid_base_data.cpuid.nent = 100;
+    r = kvm_ioctl(env->kvm_state, KVM_GET_SUPPORTED_CPUID, &cpuid_base_data);
+    if (r == -1)
+        cpuid_base_data.cpuid.nent = 0;
+
+    for (i = 0; i < cpuid_base_data.cpuid.nent; i++) {
+        if (cpuid_base_data.entries[i].function == 1)
+            func1 = &cpuid_base_data.entries[i];
+        if (cpuid_base_data.entries[i].function == 0x80000001)
+            ext_func1 = &cpuid_base_data.entries[i];
+    }
+
     cpu_x86_cpuid(env, 0, &eax, &ebx, &ecx, &edx);
     limit = eax;
 
     for (i = 0; i <= limit; i++) {
         struct kvm_cpuid_entry2 *c = &cpuid_data.entries[cpuid_i++];
 
+        if (func1 && (i == 1)) {
+            ecx_mask = func1->ecx;
+            edx_mask = func1->edx;
+        } else
+            ecx_mask = edx_mask = ~0U;
+
         cpu_x86_cpuid(env, i, &eax, &ebx, &ecx, &edx);
         c->function = i;
         c->eax = eax;
         c->ebx = ebx;
-        c->ecx = ecx;
-        c->edx = edx;
+        c->ecx = ecx & ecx_mask;
+        c->edx = edx & edx_mask;
     }
 
     cpu_x86_cpuid(env, 0x80000000, &eax, &ebx, &ecx, &edx);
     limit = eax;
 
     for (i = 0x80000000; i <= limit; i++) {
+
         struct kvm_cpuid_entry2 *c = &cpuid_data.entries[cpuid_i++];
+        if (ext_func1 && (i == 0x80000001)) {
+            ecx_mask = ext_func1->ecx;
+            edx_mask = ext_func1->edx;
+        } else
+            ecx_mask = edx_mask = ~0U;
 
         cpu_x86_cpuid(env, i, &eax, &ebx, &ecx, &edx);
         c->function = i;
         c->eax = eax;
         c->ebx = ebx;
-        c->ecx = ecx;
-        c->edx = edx;
+        c->ecx = ecx & ecx_mask;
+        c->edx = edx & edx_mask;
     }
 
     cpuid_data.cpuid.nent = cpuid_i;
-- 
1.5.6.5

[Qemu-devel] Re: [PATCH 1/3] expose paravirt feature list to cpuid

[Anthony Liguori]

Glauber Costa wrote:
> Expose the paravirt features provided by the host to the guest.
> The features exposed uses qemu just as a medium, since they only
> depend on a negotiation between the host and guest directly.
>
> As a direct result, we're now able to run the kvm pvclock in qemu
> based kvm guests.
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  kvm.h             |    1 +
>  target-i386/kvm.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 48 insertions(+), 2 deletions(-)
>
> diff --git a/kvm.h b/kvm.h
> index efce145..db58baa 100644
> --- a/kvm.h
> +++ b/kvm.h
> @@ -17,6 +17,7 @@
>  #include "config.h"
>
>  #ifdef CONFIG_KVM
> +#include <linux/kvm_para.h>
>  extern int kvm_allowed;
>
>  #define kvm_enabled() (kvm_allowed)
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 2412ae4..7f4e5aa 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -33,16 +33,61 @@
>      do { } while (0)
>  #endif
>
> +struct kvm_para_features {
> +    int cap;
> +    int feature;
> +} para_features[] = {
> +#ifdef KVM_CAP_CLOCKSOURCE
> +	{ KVM_CAP_CLOCKSOURCE, KVM_FEATURE_CLOCKSOURCE },
> +#endif
> +#ifdef KVM_CAP_NOP_IO_DELAY
> +	{ KVM_CAP_NOP_IO_DELAY, KVM_FEATURE_NOP_IO_DELAY },
> +#endif
> +#ifdef KVM_CAP_PV_MMU
> +	{ KVM_CAP_PV_MMU, KVM_FEATURE_MMU_OP },
> +#endif
> +#ifdef KVM_CAP_CR3_CACHE
> +	{ KVM_CAP_CR3_CACHE, KVM_FEATURE_CR3_CACHE },
> +#endif
> +};
> +
> +static uint32_t get_para_features(CPUState *env)
> +{
> +    uint32_t i, features = 0;
> +
> +    for (i = 0; i < ARRAY_SIZE(para_features); i++) {
> +        if (kvm_ioctl(env->kvm_state, KVM_CHECK_EXTENSION, para_features[i].cap))
> +            features |= (1 << para_features[i].feature);
> +    }
> +
> +    return features;
> +}
>  int kvm_arch_init_vcpu(CPUState *env)
>  {
>      struct {
>          struct kvm_cpuid cpuid;
>          struct kvm_cpuid_entry entries[100];
>      } __attribute__((packed)) cpuid_data;
> -    uint32_t limit, i, cpuid_i;
> +    uint32_t limit, i, cpuid_i = 0;
>      uint32_t eax, ebx, ecx, edx;
> +#ifdef KVM_CPUID_SIGNATURE
> +    struct kvm_cpuid_entry *pv_ent;
> +
> +    /* Paravirtualization CPUIDs */
> +    pv_ent = &cpuid_data.entries[cpuid_i++];
> +    memset(pv_ent, 0, sizeof(*pv_ent));
> +    pv_ent->function = KVM_CPUID_SIGNATURE;
> +    pv_ent->eax = 0;
> +    pv_ent->ebx = 0x4b4d564b; /* KVMKVMK */
> +    pv_ent->ecx = 0x564b4d56; /* VMKV */
> +    pv_ent->edx = 0x0000004d; /* M */
> +
> +    pv_ent = &cpuid_data.entries[cpuid_i++];
> +    memset(pv_ent, 0, sizeof(*pv_ent));
> +    pv_ent->function = KVM_CPUID_FEATURES;
> +    pv_ent->eax = get_para_features(env);
>   

I think this would be better suited for target-i386/helper.c in 
cpu_x86_cpuid().  It can be in an if (kvm_enabled()) for now but there 
should be a comment mentioning that we could enable some of these 
features in normal QEMU.

It can call a kvm helper function from target-i386/kvm.c to determine 
which features are supported (that could be later hooked by TCG).

Regards,

Anthony Liguori

> +#endif
>
> -    cpuid_i = 0;
>
>      cpu_x86_cpuid(env, 0, &eax, &ebx, &ecx, &edx);
>      limit = eax;
>