[Qemu-devel] [PATCH STABLE 0/3] Backport kvm cpuid feature trimming to stable

[Qemu-devel] [PATCH STABLE 0/3] Backport kvm cpuid feature trimming to stable

[Glauber Costa]

subject self explanatory

[Qemu-devel] [PATCH STABLE 1/3] Make x86 cpuid feature names available in file scope

[Glauber Costa]

From: Avi Kivity <avi@redhat.com>

To be used later.

Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 target-i386/helper.c |   55 +++++++++++++++++++++++++------------------------
 1 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 8213703..3eb9697 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -32,39 +32,40 @@
 
 //#define DEBUG_MMU
 
+/* feature flags taken from "Intel Processor Identification and the CPUID
+ * Instruction" and AMD's "CPUID Specification". In cases of disagreement
+ * about feature names, the Linux name is used. */
+static const char *feature_name[] = {
+    "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
+    "cx8", "apic", NULL, "sep", "mtrr", "pge", "mca", "cmov",
+    "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */, NULL, "ds" /* Intel dts */, "acpi", "mmx",
+    "fxsr", "sse", "sse2", "ss", "ht" /* Intel htt */, "tm", "ia64", "pbe",
+};
+static const char *ext_feature_name[] = {
+    "pni" /* Intel,AMD sse3 */, NULL, NULL, "monitor", "ds_cpl", "vmx", NULL /* Linux smx */, "est",
+    "tm2", "ssse3", "cid", NULL, NULL, "cx16", "xtpr", NULL,
+    NULL, NULL, "dca", NULL, NULL, NULL, NULL, "popcnt",
+       NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+};
+static const char *ext2_feature_name[] = {
+    "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
+    "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr", "pge", "mca", "cmov",
+    "pat", "pse36", NULL, NULL /* Linux mp */, "nx" /* Intel xd */, NULL, "mmxext", "mmx",
+    "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp", NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
+};
+static const char *ext3_feature_name[] = {
+    "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */, "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
+    "3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL, "skinit", "wdt", NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
+};
+
 static void add_flagname_to_bitmaps(char *flagname, uint32_t *features, 
                                     uint32_t *ext_features, 
                                     uint32_t *ext2_features, 
                                     uint32_t *ext3_features)
 {
     int i;
-    /* feature flags taken from "Intel Processor Identification and the CPUID
-     * Instruction" and AMD's "CPUID Specification". In cases of disagreement 
-     * about feature names, the Linux name is used. */
-    static const char *feature_name[] = {
-        "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
-        "cx8", "apic", NULL, "sep", "mtrr", "pge", "mca", "cmov",
-        "pat", "pse36", "pn" /* Intel psn */, "clflush" /* Intel clfsh */, NULL, "ds" /* Intel dts */, "acpi", "mmx",
-        "fxsr", "sse", "sse2", "ss", "ht" /* Intel htt */, "tm", "ia64", "pbe",
-    };
-    static const char *ext_feature_name[] = {
-       "pni" /* Intel,AMD sse3 */, NULL, NULL, "monitor", "ds_cpl", "vmx", NULL /* Linux smx */, "est",
-       "tm2", "ssse3", "cid", NULL, NULL, "cx16", "xtpr", NULL,
-       NULL, NULL, "dca", NULL, NULL, NULL, NULL, "popcnt",
-       NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-    };
-    static const char *ext2_feature_name[] = {
-       "fpu", "vme", "de", "pse", "tsc", "msr", "pae", "mce",
-       "cx8" /* AMD CMPXCHG8B */, "apic", NULL, "syscall", "mtrr", "pge", "mca", "cmov",
-       "pat", "pse36", NULL, NULL /* Linux mp */, "nx" /* Intel xd */, NULL, "mmxext", "mmx",
-       "fxsr", "fxsr_opt" /* AMD ffxsr */, "pdpe1gb" /* AMD Page1GB */, "rdtscp", NULL, "lm" /* Intel 64 */, "3dnowext", "3dnow",
-    };
-    static const char *ext3_feature_name[] = {
-       "lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD ExtApicSpace */, "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
-       "3dnowprefetch", "osvw", NULL /* Linux ibs */, NULL, "skinit", "wdt", NULL, NULL,
-       NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-       NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL,
-    };
 
     for ( i = 0 ; i < 32 ; i++ ) 
         if (feature_name[i] && !strcmp (flagname, feature_name[i])) {
-- 
1.5.6.6

[Qemu-devel] [PATCH STABLE 2/3] Fix x86 feature modifications for features that set multiple bits

[Glauber Costa]

From: Avi Kivity <avi@redhat.com>

QEMU allows adding or removing cpu features by using the syntax '-cpu +feature'
or '-cpu -feature'.  Some cpuid features cause more than one bit to be set or
cleared; but QEMU stops after just one bit has been modified, causing the
feature bits to be inconsistent.

Fix by allowing all feature bits corresponding to a given name to be set.

Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 target-i386/helper.c |   13 ++++++++-----
 1 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 3eb9697..1433857 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -66,28 +66,31 @@ static void add_flagname_to_bitmaps(char *flagname, uint32_t *features,
                                     uint32_t *ext3_features)
 {
     int i;
+    int found = 0;
 
     for ( i = 0 ; i < 32 ; i++ ) 
         if (feature_name[i] && !strcmp (flagname, feature_name[i])) {
             *features |= 1 << i;
-            return;
+            found = 1;
         }
     for ( i = 0 ; i < 32 ; i++ ) 
         if (ext_feature_name[i] && !strcmp (flagname, ext_feature_name[i])) {
             *ext_features |= 1 << i;
-            return;
+            found = 1;
         }
     for ( i = 0 ; i < 32 ; i++ ) 
         if (ext2_feature_name[i] && !strcmp (flagname, ext2_feature_name[i])) {
             *ext2_features |= 1 << i;
-            return;
+            found = 1;
         }
     for ( i = 0 ; i < 32 ; i++ ) 
         if (ext3_feature_name[i] && !strcmp (flagname, ext3_feature_name[i])) {
             *ext3_features |= 1 << i;
-            return;
+            found = 1;
         }
-    fprintf(stderr, "CPU feature %s not found\n", flagname);
+    if (!found) {
+        fprintf(stderr, "CPU feature %s not found\n", flagname);
+    }
 }
 
 typedef struct x86_def_t {
-- 
1.5.6.6

[Qemu-devel] [PATCH STABLE 3/3] kvm: Trim cpu features not supported by kvm

[Glauber Costa]

From: Avi Kivity <avi@redhat.com>

Remove cpu features that are not supported by kvm from the cpuid features
reported to the guest.

Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 target-i386/helper.c |   31 ++++++++++++++++++++++++++++++-
 1 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index 1433857..6af5d23 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -93,6 +93,21 @@ static void add_flagname_to_bitmaps(char *flagname, uint32_t *features,
     }
 }
 
+static void kvm_trim_features(uint32_t *features, uint32_t supported,
+                              const char *names[])
+{
+    int i;
+    uint32_t mask;
+
+    for (i = 0; i < 32; ++i) {
+        mask = 1U << i;
+        if ((*features & mask) && !(supported & mask)) {
+            printf("Processor feature %s not supported by kvm\n", names[i]);
+            *features &= ~mask;
+        }
+    }
+}
+
 typedef struct x86_def_t {
     const char *name;
     uint32_t level;
@@ -1672,7 +1687,21 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
 #ifdef USE_KQEMU
     kqemu_init(env);
 #endif
-    if (kvm_enabled())
+    if (kvm_enabled()) {
         kvm_init_vcpu(env);
+        kvm_trim_features(&env->cpuid_features,
+                          kvm_arch_get_supported_cpuid(env, 1, R_EDX),
+                          feature_name);
+        kvm_trim_features(&env->cpuid_ext_features,
+                          kvm_arch_get_supported_cpuid(env, 1, R_ECX),
+                          ext_feature_name);
+        kvm_trim_features(&env->cpuid_ext2_features,
+                          kvm_arch_get_supported_cpuid(env, 0x80000001, R_EDX),
+                          ext2_feature_name);
+        kvm_trim_features(&env->cpuid_ext3_features,
+                          kvm_arch_get_supported_cpuid(env, 0x80000001, R_ECX),
+                          ext3_feature_name);
+    }
+
     return env;
 }
-- 
1.5.6.6

Re: [Qemu-devel] [PATCH STABLE 0/3] Backport kvm cpuid feature trimming to stable

[Avi Kivity]

Glauber Costa wrote:
> subject self explanatory
>   

Right, so you have free space here to provide the motivation for the 
backport.  It's a large change and it isn't immediately obvious that 
it's stable branch material.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH STABLE 0/3] Backport kvm cpuid feature trimming to stable

[Avi Kivity]

(restored cc list)

Glauber Costa wrote:
> On Tue, May 19, 2009 at 2:02 AM, Avi Kivity <avi@redhat.com> wrote:
>   
>> Glauber Costa wrote:
>>     
>>> subject self explanatory
>>>
>>>       
>> Right, so you have free space here to provide the motivation for the
>> backport.  It's a large change and it isn't immediately obvious that it's
>> stable branch material.
>>
>>     
>
> This does not introduce any new features.
> Users expects the guest not to have any cpuid features that the host
> cannot honnor. There has been multiple bug reports regarding this in
> the past of Fedora users. I imagine other users have seen it too.
>   

If it hits users then it's obviously important.  Note though that Mark 
found some bugs there, so we need to either backport Mark's patches or 
look for a simpler workaround.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH STABLE 0/3] Backport kvm cpuid feature trimming to stable

[Glauber Costa]

On Tue, May 19, 2009 at 07:18:13PM +0300, Avi Kivity wrote:
> (restored cc list)
> 
> Glauber Costa wrote:
> >On Tue, May 19, 2009 at 2:02 AM, Avi Kivity <avi@redhat.com> wrote:
> >  
> >>Glauber Costa wrote:
> >>    
> >>>subject self explanatory
> >>>
> >>>      
> >>Right, so you have free space here to provide the motivation for the
> >>backport.  It's a large change and it isn't immediately obvious that it's
> >>stable branch material.
> >>
> >>    
> >
> >This does not introduce any new features.
> >Users expects the guest not to have any cpuid features that the host
> >cannot honnor. There has been multiple bug reports regarding this in
> >the past of Fedora users. I imagine other users have seen it too.
> >  
> 
> If it hits users then it's obviously important.  Note though that Mark 
> found some bugs there, so we need to either backport Mark's patches or 
> look for a simpler workaround.
I talked to mark , and I plan on backporting everything.