Patchwork [PATCHv2] Add KVM paravirt cpuid leaf

login
register
mail settings
Submitter Gleb Natapov
Date Jan. 7, 2010, 4:24 p.m.
Message ID <20100107162427.GF4905@redhat.com>
Download mbox | patch
Permalink /patch/42447/
State New
Headers show

Comments

Gleb Natapov - Jan. 7, 2010, 4:24 p.m.
Initialize KVM paravirt cpuid leaf and allow user to control guest
visible PV features through -cpu flag.
 
Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
v1->v2
 fix indentation
 remove unneeded ifdefs

--
			Gleb.
Anthony Liguori - Jan. 11, 2010, 7:18 p.m.
On 01/07/2010 10:24 AM, Gleb Natapov wrote:
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 4084503..6a841de 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>

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
Anwar Ghani - Jan. 11, 2010, 7:33 p.m.
Hi All

guys I am a bit new to this stuff. I want to call a method after user presses a combination of keys lets say alt+s or whatever. How can I do it using which event handler.

Best Regards



Anwar Ghani

+31 647 344 773

--- On Mon, 1/11/10, Anthony Liguori <anthony@codemonkey.ws> wrote:

From: Anthony Liguori <anthony@codemonkey.ws>
Subject: Re: [Qemu-devel] [PATCHv2] Add KVM paravirt cpuid leaf
To: "Gleb Natapov" <gleb@redhat.com>
Cc: "Marcelo Tosatti" <mtosatti@redhat.com>, "Jan Kiszka" <jan.kiszka@web.de>, qemu-devel@nongnu.org, "kvm-devel" <kvm@vger.kernel.org>, "Avi Kivity" <avi@redhat.com>
Date: Monday, January 11, 2010, 7:18 PM

On 01/07/2010 10:24 AM, Gleb Natapov wrote:
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 4084503..6a841de 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>

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
Jan Kiszka - Jan. 11, 2010, 8:40 p.m.
Anthony Liguori wrote:
> On 01/07/2010 10:24 AM, Gleb Natapov wrote:
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 4084503..6a841de 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>
> 
> 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?

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
Anthony Liguori - Jan. 11, 2010, 9:36 p.m.
On 01/11/2010 02:40 PM, Jan Kiszka wrote:
> Anthony Liguori wrote:
>    
>> On 01/07/2010 10:24 AM, Gleb Natapov wrote:
>>      
>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>> index 4084503..6a841de 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>
>>>        
>> 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?
>>      
> 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

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

It's less than ideal, but I can live with it.

Regards,

Anthony Liguori

> Jan
>
>
Jan Kiszka - Jan. 11, 2010, 11:22 p.m.
Anthony Liguori wrote:
> On 01/11/2010 02:40 PM, Jan Kiszka wrote:
>> Anthony Liguori wrote:
>>   
>>> On 01/07/2010 10:24 AM, Gleb Natapov wrote:
>>>     
>>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>>> index 4084503..6a841de 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>
>>>>        
>>> 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?
>>>      
>> 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

I does, I just failed to cherry-pick them from the temporary tree it
creates during 'make sync'.

Jan
Gleb Natapov - Jan. 12, 2010, 7:23 a.m.
On Mon, Jan 11, 2010 at 01:18:32PM -0600, Anthony Liguori wrote:
> On 01/07/2010 10:24 AM, Gleb Natapov wrote:
> >diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >index 4084503..6a841de 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>
> 
> 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?

> 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

--
			Gleb.
Avi Kivity - Jan. 12, 2010, 8:40 a.m.
On 01/12/2010 09:23 AM, Gleb Natapov wrote:
> On Mon, Jan 11, 2010 at 01:18:32PM -0600, Anthony Liguori wrote:
>    
>> On 01/07/2010 10:24 AM, Gleb Natapov wrote:
>>      
>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>> index 4084503..6a841de 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>
>>>        
>> 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?
>
>    

include/linux/Kbuild has:

ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm.h \
                   $(srctree)/include/asm-$(SRCARCH)/kvm.h),)
unifdef-y += kvm.h
endif
ifneq ($(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/kvm_para.h \
                   $(srctree)/include/asm-$(SRCARCH)/kvm_para.h),)
unifdef-y += kvm_para.h
endif

So it should be installed.  Unfortunately this is starting 2.6.32, so we 
need to backport the patch (da18acffc3) to the F12 kernel.

qemu-kvm doesn't depend on installed headers.
Avi Kivity - Jan. 12, 2010, 8:42 a.m.
On 01/11/2010 09:18 PM, Anthony Liguori wrote:
> On 01/07/2010 10:24 AM, Gleb Natapov wrote:
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 4084503..6a841de 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>
>
> 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?
>

Two options:

- make kvm detection depend on kvm_para.h being includable, fix all 
relevant distro kernels to supply it, and live with the breakage
- add a new CONFIG_KVM_PARA, detect it at configure time, and only 
include it if present

Patch

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index f3834b3..fbe11b8 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -701,6 +701,7 @@  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 */
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 4084503..6a841de 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,40 @@  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
+#ifdef KVM_CAP_HYPERV
+    { KVM_CAP_HYPERV, KVM_FEATURE_HYPERV },
+#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 +177,8 @@  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;
+    uint32_t signature[3];
 
     env->mp_state = KVM_MP_STATE_RUNNABLE;
 
@@ -160,10 +197,25 @@  int kvm_arch_init_vcpu(CPUState *env)
 
     cpuid_i = 0;
 
+    /* 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);
+
     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 +265,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;