Re: [Qemu-devel] [PATCH] Add KVM paravirt cpuid leaf

[Gleb Natapov]

On Thu, Jan 07, 2010 at 10:09:16AM -0600, Anthony Liguori wrote:
> On 01/07/2010 06:44 AM, Gleb Natapov wrote:
> >Initialize KVM paravirt cpuid leaf and allow user to control guest
> >visible PV features through -cpu flag.
> >
> >Signed-off-by: Gleb Natapov<address@hidden>
> >diff --git a/target-i386/cpu.h b/target-i386/cpu.h
> >index f3834b3..216b00e 100644
> >--- a/target-i386/cpu.h
> >+++ b/target-i386/cpu.h
> >@@ -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;
> >diff --git a/target-i386/helper.c b/target-i386/helper.c
> >index c39a993..20ec4b6 100644
> >--- a/target-i386/helper.c
> >+++ b/target-i386/helper.c
> >@@ -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;
> >diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >index de79eb7..1d93e63 100644
> >--- a/target-i386/kvm.c
> >+++ b/target-i386/kvm.c
> >@@ -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;
> >      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.
> 
It is there in qemu-kvm and we suppose to bring knowledge from there :)
But I can drop it if you like.

--
                        Gleb.