Add KVM paravirt cpuid leaf

Initialize KVM paravirt cpuid leaf and allow user to control guest visible PV features through -cpu flag.

Signed-off-by Gleb Natapov<[email protected]>

target-i386/cpu.h +2 -1

index f3834b3..216b00e 100644
@@ -701,7 +701,8 @@ typedef struct CPUX86State {
      uint8_t nmi_pending;
      uint8_t has_error_code;
      uint32_t sipi_vector;
-
+    uint32_t cpuid_kvm_features;
+
      /* in order to simplify APIC support, we leave this pointer to the
         user */
      struct APICState *apic_state;

target-i386/helper.c +27 -8

index c39a993..20ec4b6 100644
@@ -58,10 +58,18 @@ static const char *ext3_feature_name[] = {
      NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
  };
 
+static const char *kvm_feature_name[] = {
+    "kvmclock", "kvm_nopiodelay", "kvm_mmu", NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+};
+
  static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
                                      uint32_t *ext_features,
                                      uint32_t *ext2_features,
-                                    uint32_t *ext3_features)
+                                    uint32_t *ext3_features,
+                                    uint32_t *kvm_features)
  {
      int i;
      int found = 0;

@@ -86,6 +94,12 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
              *ext3_features |= 1<<  i;
              found = 1;
          }
+    for ( i = 0 ; i<  32 ; i++ )
+        if (kvm_feature_name[i]&&  !strcmp (flagname, kvm_feature_name[i])) {
+            *kvm_features |= 1<<  i;
+            found = 1;
+        }
+
      if (!found) {
          fprintf(stderr, "CPU feature %s not found\n", flagname);
      }

@@ -98,7 +112,7 @@ typedef struct x86_def_t {
      int family;
      int model;
      int stepping;
-    uint32_t features, ext_features, ext2_features, ext3_features;
+    uint32_t features, ext_features, ext2_features, ext3_features, kvm_features;
      uint32_t xlevel;
      char model_id[48];
      int vendor_override;

@@ -375,8 +389,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
 
      char *s = strdup(cpu_model);
      char *featurestr, *name = strtok(s, ",");
-    uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0;
-    uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0;
+    uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0, plus_kvm_features = 0;
+    uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0, minus_kvm_features = 0;
      uint32_t numvalue;
 
      def = NULL;

@@ -394,17 +408,20 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
          memcpy(x86_cpu_def, def, sizeof(*def));
      }
 
+    plus_kvm_features = ~0; /* not supported bits will be filtered out later */
+
      add_flagname_to_bitmaps("hypervisor",&plus_features,
-&plus_ext_features,&plus_ext2_features,&plus_ext3_features);
+&plus_ext_features,&plus_ext2_features,&plus_ext3_features,
+&plus_kvm_features);
 
      featurestr = strtok(NULL, ",");
 
      while (featurestr) {
          char *val;
          if (featurestr[0] == '+') {
-            add_flagname_to_bitmaps(featurestr + 1,&plus_features,&plus_ext_features,&plus_ext2_features,&plus_ext3_features);
+            add_flagname_to_bitmaps(featurestr + 1,&plus_features,&plus_ext_features,&plus_ext2_features,&plus_ext3_features,&plus_kvm_features);
          } else if (featurestr[0] == '-') {
-            add_flagname_to_bitmaps(featurestr + 1,&minus_features,&minus_ext_features,&minus_ext2_features,&minus_ext3_features);
+            add_flagname_to_bitmaps(featurestr + 1,&minus_features,&minus_ext_features,&minus_ext2_features,&minus_ext3_features,&minus_kvm_features);
          } else if ((val = strchr(featurestr, '='))) {
              *val = 0; val++;
              if (!strcmp(featurestr, "family")) {

@@ -481,10 +498,12 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
      x86_cpu_def->ext_features |= plus_ext_features;
      x86_cpu_def->ext2_features |= plus_ext2_features;
      x86_cpu_def->ext3_features |= plus_ext3_features;
+    x86_cpu_def->kvm_features |= plus_kvm_features;
      x86_cpu_def->features&= ~minus_features;
      x86_cpu_def->ext_features&= ~minus_ext_features;
      x86_cpu_def->ext2_features&= ~minus_ext2_features;
      x86_cpu_def->ext3_features&= ~minus_ext3_features;
+    x86_cpu_def->kvm_features&= ~minus_kvm_features;
      free(s);
      return 0;
 

@@ -529,7 +548,7 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
      env->cpuid_ext_features = def->ext_features;
      env->cpuid_ext2_features = def->ext2_features;
      env->cpuid_xlevel = def->xlevel;
-    env->cpuid_ext3_features = def->ext3_features;
+    env->cpuid_kvm_features = def->kvm_features;
      {
          const char *model_id = def->model_id;
          int c, len, i;

target-i386/kvm.c +32 -0

index de79eb7..1d93e63 100644
@@ -17,6 +17,7 @@
  #include<sys/mman.h>
 
  #include<linux/kvm.h>
+#include<linux/kvm_para.h>
 
  #include "qemu-common.h"
  #include "sysemu.h"

@@ -134,6 +135,37 @@ static void kvm_trim_features(uint32_t *features, uint32_t supported)
      }
  }
 
+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
+        { -1, -1 }
+};
+
+static int get_para_features(CPUState *env)
+{
+        int i, features = 0;
+
+        for (i = 0; i<  ARRAY_SIZE(para_features) - 1; i++) {
+                if (kvm_check_extension(env->kvm_state, para_features[i].cap))
+                        features |= (1<<  para_features[i].feature);
+        }
+
+        return features;
+}
+
  int kvm_arch_init_vcpu(CPUState *env)
  {
      struct {

Quoted 174 lines · Gleb Natapov

Initialize KVM paravirt cpuid leaf and allow user to control guest visible PV features through -cpu flag.

Signed-off-by Gleb Natapov<[email protected]>

target-i386/cpu.h +2 -1

index f3834b3..216b00e 100644
@@ -701,7 +701,8 @@ typedef struct CPUX86State {
     uint8_t nmi_pending;
     uint8_t has_error_code;
     uint32_t sipi_vector;
-
+    uint32_t cpuid_kvm_features;
+
     /* in order to simplify APIC support, we leave this pointer to the
        user */
     struct APICState *apic_state;

target-i386/helper.c +27 -8

index c39a993..20ec4b6 100644
@@ -58,10 +58,18 @@ static const char *ext3_feature_name[] = {
     NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
 };
 
+static const char *kvm_feature_name[] = {
+    "kvmclock", "kvm_nopiodelay", "kvm_mmu", NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+};
+
 static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
                                     uint32_t *ext_features,
                                     uint32_t *ext2_features,
-                                    uint32_t *ext3_features)
+                                    uint32_t *ext3_features,
+                                    uint32_t *kvm_features)
 {
     int i;
     int found = 0;

@@ -86,6 +94,12 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
             *ext3_features |= 1<<  i;
             found = 1;
         }
+    for ( i = 0 ; i<  32 ; i++ )
+        if (kvm_feature_name[i]&&  !strcmp (flagname, kvm_feature_name[i])) {
+            *kvm_features |= 1<<  i;
+            found = 1;
+        }
+
     if (!found) {
         fprintf(stderr, "CPU feature %s not found\n", flagname);
     }

@@ -98,7 +112,7 @@ typedef struct x86_def_t {
     int family;
     int model;
     int stepping;
-    uint32_t features, ext_features, ext2_features, ext3_features;
+    uint32_t features, ext_features, ext2_features, ext3_features, kvm_features;
     uint32_t xlevel;
     char model_id[48];
     int vendor_override;

@@ -375,8 +389,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
 
     char *s = strdup(cpu_model);
     char *featurestr, *name = strtok(s, ",");
-    uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0;
-    uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0;
+    uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0, plus_kvm_features = 0;
+    uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0, minus_kvm_features = 0;
     uint32_t numvalue;
 
     def = NULL;

@@ -394,17 +408,20 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
         memcpy(x86_cpu_def, def, sizeof(*def));
     }
 
+    plus_kvm_features = ~0; /* not supported bits will be filtered out later */
+
     add_flagname_to_bitmaps("hypervisor",&plus_features,
-&plus_ext_features,&plus_ext2_features,&plus_ext3_features);
+&plus_ext_features,&plus_ext2_features,&plus_ext3_features,
+&plus_kvm_features);
 
     featurestr = strtok(NULL, ",");
 
     while (featurestr) {
         char *val;
         if (featurestr[0] == '+') {
-            add_flagname_to_bitmaps(featurestr + 1,&plus_features,&plus_ext_features,&plus_ext2_features,&plus_ext3_features);
+            add_flagname_to_bitmaps(featurestr + 1,&plus_features,&plus_ext_features,&plus_ext2_features,&plus_ext3_features,&plus_kvm_features);
         } else if (featurestr[0] == '-') {
-            add_flagname_to_bitmaps(featurestr + 1,&minus_features,&minus_ext_features,&minus_ext2_features,&minus_ext3_features);
+            add_flagname_to_bitmaps(featurestr + 1,&minus_features,&minus_ext_features,&minus_ext2_features,&minus_ext3_features,&minus_kvm_features);
         } else if ((val = strchr(featurestr, '='))) {
             *val = 0; val++;
             if (!strcmp(featurestr, "family")) {

@@ -481,10 +498,12 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
     x86_cpu_def->ext_features |= plus_ext_features;
     x86_cpu_def->ext2_features |= plus_ext2_features;
     x86_cpu_def->ext3_features |= plus_ext3_features;
+    x86_cpu_def->kvm_features |= plus_kvm_features;
     x86_cpu_def->features&= ~minus_features;
     x86_cpu_def->ext_features&= ~minus_ext_features;
     x86_cpu_def->ext2_features&= ~minus_ext2_features;
     x86_cpu_def->ext3_features&= ~minus_ext3_features;
+    x86_cpu_def->kvm_features&= ~minus_kvm_features;
     free(s);
     return 0;
 

@@ -529,7 +548,7 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
     env->cpuid_ext_features = def->ext_features;
     env->cpuid_ext2_features = def->ext2_features;
     env->cpuid_xlevel = def->xlevel;
-    env->cpuid_ext3_features = def->ext3_features;
+    env->cpuid_kvm_features = def->kvm_features;
     {
         const char *model_id = def->model_id;
         int c, len, i;

target-i386/kvm.c +32 -0

index de79eb7..1d93e63 100644
@@ -17,6 +17,7 @@
 #include<sys/mman.h>
 
 #include<linux/kvm.h>
+#include<linux/kvm_para.h>
 
 #include "qemu-common.h"
 #include "sysemu.h"

@@ -134,6 +135,37 @@ static void kvm_trim_features(uint32_t *features, uint32_t supported)
     }
 }
 
+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
+        { -1, -1 }
+};
+
+static int get_para_features(CPUState *env)
+{
+        int i, features = 0;
+
+        for (i = 0; i<  ARRAY_SIZE(para_features) - 1; i++) {
+                if (kvm_check_extension(env->kvm_state, para_features[i].cap))
+                        features |= (1<<  para_features[i].feature);
+        }
+
+        return features;
+}
+
 int kvm_arch_init_vcpu(CPUState *env)
 {
     struct {

@@ -142,6 +174,10 @@ int kvm_arch_init_vcpu(CPUState *env)
     } __attribute__((packed)) cpuid_data;
     uint32_t limit, i, j, cpuid_i;

target-i386/kvm.c int kvm_arch_init_vcpu(CPUState *env) · line 178

Quoted 3 lines

target-i386/kvm.c +2 -0

     uint32_t unused;
+    struct kvm_cpuid_entry2 *c;
+#ifdef KVM_CPUID_SIGNATURE

KVM_CPUID_SIGNATURE is unnecessary as a guard because it's always been present in kvm_para.h. Since we include kvm_para.h unconditionally, there's no way we wouldn't have KVM_CPUID_SIGNATURE.

Quoted 6 lines

KVM_CPUID_SIGNATURE is unnecessary as a guard because it's always been present in kvm_para.h. Since we include kvm_para.h unconditionally, there's no way we wouldn't have KVM_CPUID_SIGNATURE.

It is there in qemu-kvm and we suppose to bring knowledge from there :)

But I can drop it if you like.

--
Gleb.

I don't want to deviate unnecessarily from qemu-kvm so we can approach this in one of two ways. We can remove the guard in qemu-kvm and Avi can send me a pull request with this feature. I can also just pull in the patch as-is and we can fix this in both places later. I'm happy either way.

Quoted 6 lines · Anthony Liguori

I don't want to deviate unnecessarily from qemu-kvm so we can approach this in one of two ways. We can remove the guard in qemu-kvm and Avi can send me a pull request with this feature. I can also just pull in the patch as-is and we can fix this in both places later. I'm happy either way.

Let's go through qemu-kvm.git. I'll post information about the queue policy soon.

Quoted 8 lines · Avi Kivity

Quoted 6 lines · Anthony Liguori

I don't want to deviate unnecessarily from qemu-kvm so we can approach this in one of two ways. We can remove the guard in qemu-kvm and Avi can send me a pull request with this feature. I can also just pull in the patch as-is and we can fix this in both places later. I'm happy either way.

Quoted 4 lines

Let's go through qemu-kvm.git. I'll post information about the queue policy soon.

Till yesterday it was other way around.

May be switch rules after
applying the patch?

--
Gleb.

Quoted 10 lines · Gleb Natapov

Quoted 9 lines · Avi Kivity

Quoted 5 lines · Anthony Liguori

I don't want to deviate unnecessarily from qemu-kvm so we can approach this in one of two ways. We can remove the guard in qemu-kvm and Avi can send me a pull request with this feature. I can also just pull in the patch as-is and we can fix this in both places later. I'm happy either way.

Let's go through qemu-kvm.git. I'll post information about the queue policy soon.

Quoted 1 line

Till yesterday it was other way around.

Not qemu-kvm.git master, but a for_upstream branch that actually tracks qemu.git. The point is to get Avi involved in shaping the way kvm support in qemu.git evolves so that we can converge.

Quoted 2 lines

May be switch rules after
applying the patch?

Let's give Avi a chance to get his tree setup. We can revisit in a couple days if necessary.

Regards,

Anthony Liguori

Quoted 2 lines

--
Gleb.

Quoted 6 lines · Gleb Natapov

Quoted 5 lines

KVM_CPUID_SIGNATURE is unnecessary as a guard because it's always been present in kvm_para.h. Since we include kvm_para.h unconditionally, there's no way we wouldn't have KVM_CPUID_SIGNATURE.

It is there in qemu-kvm and we suppose to bring knowledge from there :)

Hopefully just the good bits.

Quoted 1 line

But I can drop it if you like.

I don't want to deviate unnecessarily from qemu-kvm so we can approach this in one of two ways. We can remove the guard in qemu-kvm and Avi can send me a pull request with this feature. I can also just pull in the patch as-is and we can fix this in both places later. I'm happy either way.

target-i386/kvm.c line 20

Quoted 9 lines · Gleb Natapov

target-i386/kvm.c +1 -0

index 4084503..6a841de 100644
@@ -17,6 +17,7 @@
  #include<sys/mman.h>
 
  #include<linux/kvm.h>
+#include<linux/kvm_para.h>

This breaks the build on a default F12 install because while kvm.h is present, kvm_para.h is not. This is a hard one to fix.

We can default the kvm search path to /lib/modules/$(uname -r)/build, we can fix the glibc headers and live with it, or we can pull in the kvm headers into qemu.

Avi/Marcelo/Jan, any thoughts from the qemu-kvm side?

Quoted 19 lines · Anthony Liguori

target-i386/kvm.c line 21

Quoted 10 lines · Gleb Natapov

target-i386/kvm.c +1 -0

index 4084503..6a841de 100644
@@ -17,6 +17,7 @@
   #include<sys/mman.h>
 
   #include<linux/kvm.h>
+#include<linux/kvm_para.h>
 

This breaks the build on a default F12 install because while kvm.h is present, kvm_para.h is not. This is a hard one to fix.

We can default the kvm search path to /lib/modules/$(uname -r)/build, we can fix the glibc headers and live with it, or we can pull in the kvm headers into qemu.

Quoted 2 lines

Avi/Marcelo/Jan, any thoughts from the qemu-kvm side?

kvm-kmod-wise, I can include arch and generic kvm_para.h in the next release (missed the need for it so far). I'm planning to write a qemu patch to ask pkg-config for kvm-kmod headers.

If we can live with considering the cpuid leaf a feature that depends on a recent kvm-kmod version and is disabled otherwise, we are done. If not, tricks like the above are required.

Jan

Quoted 21 lines · Jan Kiszka

Quoted 19 lines · Anthony Liguori

target-i386/kvm.c line 21

Quoted 10 lines · Gleb Natapov

target-i386/kvm.c +1 -0

index 4084503..6a841de 100644
@@ -17,6 +17,7 @@
   #include<sys/mman.h>
 
   #include<linux/kvm.h>
+#include<linux/kvm_para.h>
 

This breaks the build on a default F12 install because while kvm.h is present, kvm_para.h is not. This is a hard one to fix.

We can default the kvm search path to /lib/modules/$(uname -r)/build, we can fix the glibc headers and live with it, or we can pull in the kvm headers into qemu.

Quoted 6 lines

Quoted 2 lines

Avi/Marcelo/Jan, any thoughts from the qemu-kvm side?

kvm-kmod-wise, I can include arch and generic kvm_para.h in the next release (missed the need for it so far). I'm planning to write a qemu patch to ask pkg-config for kvm-kmod headers.

That would be nice.

I assume a change has to be made in the kernel too so that the libc headers are updated. IOW, I assume make headers_install doesn't currently install kvm_para.h

Quoted 7 lines · Gleb Natapov

target-i386/kvm.c +0 -0

index 4084503..6a841de 100644
@@ -17,6 +17,7 @@
 #include<sys/mman.h>
 

target-i386/kvm.c line 20

Quoted 2 lines

target-i386/kvm.c +1 -0

 #include<linux/kvm.h>
+#include<linux/kvm_para.h>

This breaks the build on a default F12 install because while kvm.h is present, kvm_para.h is not. This is a hard one to fix.

We can default the kvm search path to /lib/modules/$(uname -r)/build, we can fix the glibc headers and live with it, or we can pull in the kvm headers into qemu.

Avi/Marcelo/Jan, any thoughts from the qemu-kvm side?

Regards,

Anthony Liguori

Quoted 9 lines · Anthony Liguori

Quoted 7 lines · Gleb Natapov

target-i386/kvm.c +0 -0

index 4084503..6a841de 100644
@@ -17,6 +17,7 @@
  #include<sys/mman.h>
 

Quoted 7 lines

target-i386/kvm.c line 21

Quoted 3 lines

target-i386/kvm.c +1 -0

  #include<linux/kvm.h>
+#include<linux/kvm_para.h>
 

This breaks the build on a default F12 install because while kvm.h is present, kvm_para.h is not. This is a hard one to fix.

Avi how qemu-kvm compiles there? Or it doesn't?

Quoted 7 lines · Gleb Natapov

target-i386/kvm.c +0 -0

index 4084503..6a841de 100644
@@ -17,6 +17,7 @@
  #include<sys/mman.h>
 

target-i386/kvm.c line 20

Quoted 2 lines

target-i386/kvm.c +1 -0

  #include<linux/kvm.h>
+#include<linux/kvm_para.h>

This breaks the build on a default F12 install because while kvm.h is present, kvm_para.h is not. This is a hard one to fix.

We can default the kvm search path to /lib/modules/$(uname -r)/build, we can fix the glibc headers and live with it, or we can pull in the kvm headers into qemu.

Avi/Marcelo/Jan, any thoughts from the qemu-kvm side?

Initialize KVM paravirt cpuid leaf and allow user to control guest visible PV features through -cpu flag.

Signed-off-by Gleb Natapov<[email protected]>

---
v1->v2
fix indentation
remove unneeded ifdefs
v2->v3
added needed ifdefs (CONFIG_KVM_PARA)

target-i386/cpu.h +2 -1

index f3834b3..216b00e 100644
@@ -701,7 +701,8 @@ typedef struct CPUX86State {
      uint8_t nmi_pending;
      uint8_t has_error_code;
      uint32_t sipi_vector;
-
+    uint32_t cpuid_kvm_features;
+
      /* in order to simplify APIC support, we leave this pointer to the
         user */
      struct APICState *apic_state;

target-i386/helper.c +27 -8

index 049fccf..70762bb 100644
@@ -58,10 +58,18 @@ static const char *ext3_feature_name[] = {
      NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
  };
 
+static const char *kvm_feature_name[] = {
+    "kvmclock", "kvm_nopiodelay", "kvm_mmu", NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+};
+
  static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
                                      uint32_t *ext_features,
                                      uint32_t *ext2_features,
-                                    uint32_t *ext3_features)
+                                    uint32_t *ext3_features,
+                                    uint32_t *kvm_features)
  {
      int i;
      int found = 0;

@@ -86,6 +94,12 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
              *ext3_features |= 1<<  i;
              found = 1;
          }
+    for ( i = 0 ; i<  32 ; i++ )
+        if (kvm_feature_name[i]&&  !strcmp (flagname, kvm_feature_name[i])) {
+            *kvm_features |= 1<<  i;
+            found = 1;
+        }
+
      if (!found) {
          fprintf(stderr, "CPU feature %s not found\n", flagname);
      }

@@ -98,7 +112,7 @@ typedef struct x86_def_t {
      int family;
      int model;
      int stepping;
-    uint32_t features, ext_features, ext2_features, ext3_features;
+    uint32_t features, ext_features, ext2_features, ext3_features, kvm_features;
      uint32_t xlevel;
      char model_id[48];
      int vendor_override;

@@ -375,8 +389,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
 
      char *s = strdup(cpu_model);
      char *featurestr, *name = strtok(s, ",");
-    uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0;
-    uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0;
+    uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0, plus_kvm_features = 0;
+    uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0, minus_kvm_features = 0;
      uint32_t numvalue;
 
      def = NULL;

@@ -394,17 +408,20 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
          memcpy(x86_cpu_def, def, sizeof(*def));
      }
 
+    plus_kvm_features = ~0; /* not supported bits will be filtered out later */
+
      add_flagname_to_bitmaps("hypervisor",&plus_features,
-&plus_ext_features,&plus_ext2_features,&plus_ext3_features);
+&plus_ext_features,&plus_ext2_features,&plus_ext3_features,
+&plus_kvm_features);
 
      featurestr = strtok(NULL, ",");
 
      while (featurestr) {
          char *val;
          if (featurestr[0] == '+') {
-            add_flagname_to_bitmaps(featurestr + 1,&plus_features,&plus_ext_features,&plus_ext2_features,&plus_ext3_features);
+            add_flagname_to_bitmaps(featurestr + 1,&plus_features,&plus_ext_features,&plus_ext2_features,&plus_ext3_features,&plus_kvm_features);
          } else if (featurestr[0] == '-') {
-            add_flagname_to_bitmaps(featurestr + 1,&minus_features,&minus_ext_features,&minus_ext2_features,&minus_ext3_features);
+            add_flagname_to_bitmaps(featurestr + 1,&minus_features,&minus_ext_features,&minus_ext2_features,&minus_ext3_features,&minus_kvm_features);
          } else if ((val = strchr(featurestr, '='))) {
              *val = 0; val++;
              if (!strcmp(featurestr, "family")) {

@@ -481,10 +498,12 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
      x86_cpu_def->ext_features |= plus_ext_features;
      x86_cpu_def->ext2_features |= plus_ext2_features;
      x86_cpu_def->ext3_features |= plus_ext3_features;
+    x86_cpu_def->kvm_features |= plus_kvm_features;
      x86_cpu_def->features&= ~minus_features;
      x86_cpu_def->ext_features&= ~minus_ext_features;
      x86_cpu_def->ext2_features&= ~minus_ext2_features;
      x86_cpu_def->ext3_features&= ~minus_ext3_features;
+    x86_cpu_def->kvm_features&= ~minus_kvm_features;
      free(s);
      return 0;
 

@@ -529,7 +548,7 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
      env->cpuid_ext_features = def->ext_features;
      env->cpuid_ext2_features = def->ext2_features;
      env->cpuid_xlevel = def->xlevel;
-    env->cpuid_ext3_features = def->ext3_features;
+    env->cpuid_kvm_features = def->kvm_features;
      {
          const char *model_id = def->model_id;
          int c, len, i;

target-i386/kvm.c +49 -0

index 4084503..5b093ce 100644
@@ -25,6 +25,10 @@
  #include "gdbstub.h"
  #include "host-utils.h"
 
+#ifdef CONFIG_KVM_PARA
+#include<linux/kvm_para.h>
+#endif
+//
  //#define DEBUG_KVM
 
  #ifdef DEBUG_KVM

@@ -134,6 +138,39 @@ static void kvm_trim_features(uint32_t *features, uint32_t supported)
      }
  }
 
+#ifdef CONFIG_KVM_PARA
+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
+        { -1, -1 }
+};
+
+static int get_para_features(CPUState *env)
+{
+        int i, features = 0;
+
+        for (i = 0; i<  ARRAY_SIZE(para_features) - 1; i++) {
+                if (kvm_check_extension(env->kvm_state, para_features[i].cap))
+                        features |= (1<<  para_features[i].feature);
+        }
+
+        return features;
+}
+#endif
+
  int kvm_arch_init_vcpu(CPUState *env)
  {
      struct {

@@ -142,6 +179,10 @@ int kvm_arch_init_vcpu(CPUState *env)
      } __attribute__((packed)) cpuid_data;
      uint32_t limit, i, j, cpuid_i;
      uint32_t unused;
+    struct kvm_cpuid_entry2 *c;
+#ifdef KVM_CPUID_SIGNATURE
+    uint32_t signature[3];
+#endif
 
      env->mp_state = KVM_MP_STATE_RUNNABLE;
 

@@ -160,10 +201,27 @@ int kvm_arch_init_vcpu(CPUState *env)
 
      cpuid_i = 0;
 
+#ifdef CONFIG_KVM_PARA
+    /* Paravirtualization CPUIDs */
+    memcpy(signature, "KVMKVMKVM\0\0\0", 12);
+    c =&cpuid_data.entries[cpuid_i++];
+    memset(c, 0, sizeof(*c));
+    c->function = KVM_CPUID_SIGNATURE;
+    c->eax = 0;
+    c->ebx = signature[0];

Quoted 13 lines

target-i386/kvm.c +13 -0

+#ifdef CONFIG_KVM_PARA
+    /* Paravirtualization CPUIDs */
+    memcpy(signature, "KVMKVMKVM\0\0\0", 12);
+    c =&cpuid_data.entries[cpuid_i++];
+    memset(c, 0, sizeof(*c));
+    c->function = KVM_CPUID_SIGNATURE;
+    c->eax = 0;
+    c->ebx = signature[0];
+    c->ecx = signature[1];
+    c->edx = signature[2];
+
+    c =&cpuid_data.entries[cpuid_i++];
+    memset(c, 0, sizeof(*c));

target-i386/kvm.c int kvm_arch_init_vcpu(CPUState *env)

Quoted 5 lines

target-i386/kvm.c +4 -0

+    c->function = KVM_CPUID_FEATURES;
+    c->eax = env->cpuid_kvm_features&  get_para_features(env);
+#endif
+
     cpu_x86_cpuid(env, 0, 0,&limit,&unused,&unused,&unused);

Instead of hooking here, would it make more sense to tie into the generic cpuid function in helper.c?

Quoted 13 lines · Anthony Liguori

Quoted 13 lines

target-i386/kvm.c +13 -0

+#ifdef CONFIG_KVM_PARA
+    /* Paravirtualization CPUIDs */
+    memcpy(signature, "KVMKVMKVM\0\0\0", 12);
+    c =&cpuid_data.entries[cpuid_i++];
+    memset(c, 0, sizeof(*c));
+    c->function = KVM_CPUID_SIGNATURE;
+    c->eax = 0;
+    c->ebx = signature[0];
+    c->ecx = signature[1];
+    c->edx = signature[2];
+
+    c =&cpuid_data.entries[cpuid_i++];
+    memset(c, 0, sizeof(*c));

Quoted 9 lines

target-i386/kvm.c int kvm_arch_init_vcpu(CPUState *env)

Quoted 5 lines

target-i386/kvm.c +4 -0

+    c->function = KVM_CPUID_FEATURES;
+    c->eax = env->cpuid_kvm_features&  get_para_features(env);
+#endif
+
     cpu_x86_cpuid(env, 0, 0,&limit,&unused,&unused,&unused);

Instead of hooking here, would it make more sense to tie into the generic cpuid function in helper.c?

What do you mean? This patch ties into generic cpuid function in helper.c and pars kvm cpu flags there. Here we just configure kernel.

Or do you mean making so that PV leaf will be available via cpu_x86_cpuid()? This make sense, but lets do it after merging this code path with qemu-kvm and the proposed patch brings qemu and qemu-kvm close together.

Initialize KVM paravirt cpuid leaf and allow user to control guest visible PV features through -cpu flag.

Signed-off-by Gleb Natapov<[email protected]>

---
v1->v2
fix indentation
remove unneeded ifdefs
v2->v3
added needed ifdefs (CONFIG_KVM_PARA)

target-i386/cpu.h +2 -1

index f3834b3..216b00e 100644
@@ -701,7 +701,8 @@ typedef struct CPUX86State {
      uint8_t nmi_pending;
      uint8_t has_error_code;
      uint32_t sipi_vector;
-
+    uint32_t cpuid_kvm_features;
+
      /* in order to simplify APIC support, we leave this pointer to the
         user */
      struct APICState *apic_state;

target-i386/helper.c +27 -8

index 049fccf..70762bb 100644
@@ -58,10 +58,18 @@ static const char *ext3_feature_name[] = {
      NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
  };
 
+static const char *kvm_feature_name[] = {
+    "kvmclock", "kvm_nopiodelay", "kvm_mmu", NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+};
+
  static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
                                      uint32_t *ext_features,
                                      uint32_t *ext2_features,
-                                    uint32_t *ext3_features)
+                                    uint32_t *ext3_features,
+                                    uint32_t *kvm_features)
  {
      int i;
      int found = 0;

@@ -86,6 +94,12 @@ static void add_flagname_to_bitmaps(const char *flagname, uint32_t *features,
              *ext3_features |= 1<<  i;
              found = 1;
          }
+    for ( i = 0 ; i<  32 ; i++ )
+        if (kvm_feature_name[i]&&  !strcmp (flagname, kvm_feature_name[i])) {
+            *kvm_features |= 1<<  i;
+            found = 1;
+        }
+
      if (!found) {
          fprintf(stderr, "CPU feature %s not found\n", flagname);
      }

@@ -98,7 +112,7 @@ typedef struct x86_def_t {
      int family;
      int model;
      int stepping;
-    uint32_t features, ext_features, ext2_features, ext3_features;
+    uint32_t features, ext_features, ext2_features, ext3_features, kvm_features;
      uint32_t xlevel;
      char model_id[48];
      int vendor_override;

@@ -375,8 +389,8 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
 
      char *s = strdup(cpu_model);
      char *featurestr, *name = strtok(s, ",");
-    uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0;
-    uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0;
+    uint32_t plus_features = 0, plus_ext_features = 0, plus_ext2_features = 0, plus_ext3_features = 0, plus_kvm_features = 0;
+    uint32_t minus_features = 0, minus_ext_features = 0, minus_ext2_features = 0, minus_ext3_features = 0, minus_kvm_features = 0;
      uint32_t numvalue;
 
      def = NULL;

@@ -394,17 +408,20 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
          memcpy(x86_cpu_def, def, sizeof(*def));
      }
 
+    plus_kvm_features = ~0; /* not supported bits will be filtered out later */
+
      add_flagname_to_bitmaps("hypervisor",&plus_features,
-&plus_ext_features,&plus_ext2_features,&plus_ext3_features);
+&plus_ext_features,&plus_ext2_features,&plus_ext3_features,
+&plus_kvm_features);
 
      featurestr = strtok(NULL, ",");
 
      while (featurestr) {
          char *val;
          if (featurestr[0] == '+') {
-            add_flagname_to_bitmaps(featurestr + 1,&plus_features,&plus_ext_features,&plus_ext2_features,&plus_ext3_features);
+            add_flagname_to_bitmaps(featurestr + 1,&plus_features,&plus_ext_features,&plus_ext2_features,&plus_ext3_features,&plus_kvm_features);
          } else if (featurestr[0] == '-') {
-            add_flagname_to_bitmaps(featurestr + 1,&minus_features,&minus_ext_features,&minus_ext2_features,&minus_ext3_features);
+            add_flagname_to_bitmaps(featurestr + 1,&minus_features,&minus_ext_features,&minus_ext2_features,&minus_ext3_features,&minus_kvm_features);
          } else if ((val = strchr(featurestr, '='))) {
              *val = 0; val++;
              if (!strcmp(featurestr, "family")) {

@@ -481,10 +498,12 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, const char *cpu_model)
      x86_cpu_def->ext_features |= plus_ext_features;
      x86_cpu_def->ext2_features |= plus_ext2_features;
      x86_cpu_def->ext3_features |= plus_ext3_features;
+    x86_cpu_def->kvm_features |= plus_kvm_features;
      x86_cpu_def->features&= ~minus_features;
      x86_cpu_def->ext_features&= ~minus_ext_features;
      x86_cpu_def->ext2_features&= ~minus_ext2_features;
      x86_cpu_def->ext3_features&= ~minus_ext3_features;
+    x86_cpu_def->kvm_features&= ~minus_kvm_features;
      free(s);
      return 0;
 

@@ -529,7 +548,7 @@ static int cpu_x86_register (CPUX86State *env, const char *cpu_model)
      env->cpuid_ext_features = def->ext_features;
      env->cpuid_ext2_features = def->ext2_features;
      env->cpuid_xlevel = def->xlevel;
-    env->cpuid_ext3_features = def->ext3_features;
+    env->cpuid_kvm_features = def->kvm_features;
      {
          const char *model_id = def->model_id;
          int c, len, i;

target-i386/kvm.c +60 -3

index 4084503..5b093ce 100644
@@ -25,6 +25,10 @@
  #include "gdbstub.h"
  #include "host-utils.h"
 
+#ifdef CONFIG_KVM_PARA
+#include<linux/kvm_para.h>
+#endif
+//
  //#define DEBUG_KVM
 
  #ifdef DEBUG_KVM

@@ -134,6 +138,39 @@ static void kvm_trim_features(uint32_t *features, uint32_t supported)
      }
  }
 
+#ifdef CONFIG_KVM_PARA
+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
+        { -1, -1 }
+};
+
+static int get_para_features(CPUState *env)
+{
+        int i, features = 0;
+
+        for (i = 0; i<  ARRAY_SIZE(para_features) - 1; i++) {
+                if (kvm_check_extension(env->kvm_state, para_features[i].cap))
+                        features |= (1<<  para_features[i].feature);
+        }
+
+        return features;
+}
+#endif
+
  int kvm_arch_init_vcpu(CPUState *env)
  {
      struct {

@@ -142,6 +179,10 @@ int kvm_arch_init_vcpu(CPUState *env)
      } __attribute__((packed)) cpuid_data;
      uint32_t limit, i, j, cpuid_i;
      uint32_t unused;
+    struct kvm_cpuid_entry2 *c;
+#ifdef KVM_CPUID_SIGNATURE
+    uint32_t signature[3];
+#endif
 
      env->mp_state = KVM_MP_STATE_RUNNABLE;
 

@@ -160,10 +201,27 @@ int kvm_arch_init_vcpu(CPUState *env)
 
      cpuid_i = 0;
 
+#ifdef CONFIG_KVM_PARA
+    /* Paravirtualization CPUIDs */
+    memcpy(signature, "KVMKVMKVM\0\0\0", 12);
+    c =&cpuid_data.entries[cpuid_i++];
+    memset(c, 0, sizeof(*c));
+    c->function = KVM_CPUID_SIGNATURE;
+    c->eax = 0;
+    c->ebx = signature[0];
+    c->ecx = signature[1];
+    c->edx = signature[2];
+
+    c =&cpuid_data.entries[cpuid_i++];
+    memset(c, 0, sizeof(*c));
+    c->function = KVM_CPUID_FEATURES;
+    c->eax = env->cpuid_kvm_features&  get_para_features(env);
+#endif
+
      cpu_x86_cpuid(env, 0, 0,&limit,&unused,&unused,&unused);
 
      for (i = 0; i<= limit; i++) {
-        struct kvm_cpuid_entry2 *c =&cpuid_data.entries[cpuid_i++];
+        c =&cpuid_data.entries[cpuid_i++];
 
          switch (i) {
          case 2: {

@@ -213,7 +271,7 @@ int kvm_arch_init_vcpu(CPUState *env)
      cpu_x86_cpuid(env, 0x80000000, 0,&limit,&unused,&unused,&unused);
 
      for (i = 0x80000000; i<= limit; i++) {
-        struct kvm_cpuid_entry2 *c =&cpuid_data.entries[cpuid_i++];
+        c =&cpuid_data.entries[cpuid_i++];
 
          c->function = i;
          c->flags = 0;
--

Gleb.