Re: [Qemu-devel] [QEMU 1.1 PATCH v3] Expose CPUID leaf 7 only for -cpu host

2012-05-30 Thread Anthony Liguori

On 05/21/2012 10:27 PM, Eduardo Habkost wrote:

Changes v2 -  v3;
   - Check for kvm_enabled() before setting cpuid_7_0_ebx_features

Changes v1 -  v2:
   - Use kvm_arch_get_supported_cpuid() instead of host_cpuid() on
 cpu_x86_fill_host().

   We should use GET_SUPPORTED_CPUID for all bits on -cpu host
   eventually, but I am not changing all the other CPUID leaves because
   we may not be able to test such an intrusive change in time for 1.1.


Applied.  Thanks.

Regards,

Anthony Liguori



Description of the bug:

Since QEMU 0.15, the CPUID information on CPUID[EAX=7,ECX=0] is being
returned unfiltered to the guest, directly from the GET_SUPPORTED_CPUID
return value.

The problem is that this makes the resulting CPU feature flags
unpredictable and dependent on the host CPU and kernel version. This
breaks live-migration badly if migrating from a host CPU that supports
some features on that CPUID leaf (running a recent kernel) to a kernel
or host CPU that doesn't support it.

Migration also is incorrect (the virtual CPU changes under the guest's
feet) if you migrate in the opposite direction (from an old CPU/kernel
to a new CPU/kernel), but with less serious consequences (guests
normally query CPUID information only once on boot).

Fortunately, the bug affects only users using cpudefs with level= 7.

The right behavior should be to explicitly enable those features on
[cpudef] config sections or on the -cpu command-line arguments. Right
now there is no predefined CPU model on QEMU that has those features:
the latest Intel model we have is Sandy Bridge.

I would like to get this fixed on 1.1, so I am submitting this patch,
that enables those features only if -cpu host is being used (as we
don't have any pre-defined CPU model that actually have those features).
After 1.1 is released, we can make those features properly configurable
on [cpudef] and -cpu configuration.

One problem is: with this patch, users with the following setup:
- Running QEMU 1.0;
- Using a cpudef having level= 7;
- Running a kernel that supports the features on CPUID leaf 7; and
- Running on a CPU that supports some features on CPUID leaf 7
won't be able to live-migrate to QEMU 1.1. But for these users
live-migration is already broken (they can't live-migrate to hosts with
older CPUs or older kernels, already), I don't see how to avoid this
problem.

Signed-off-by: Eduardo Habkostehabk...@redhat.com
---
  target-i386/cpu.c |   22 +++---
  target-i386/cpu.h |2 ++
  2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 89b4ac7..388bc5c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -238,6 +238,8 @@ typedef struct x86_def_t {
  /* Store the results of Centaur's CPUID instructions */
  uint32_t ext4_features;
  uint32_t xlevel2;
+/* The feature bits on CPUID[EAX=7,ECX=0].EBX */
+uint32_t cpuid_7_0_ebx_features;
  } x86_def_t;

  #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -521,6 +523,12 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
  x86_cpu_def-ext_features = ecx;
  x86_cpu_def-features = edx;

+if (kvm_enabled()  x86_cpu_def-level= 7) {
+x86_cpu_def-cpuid_7_0_ebx_features = 
kvm_arch_get_supported_cpuid(kvm_state, 0x7, 0, R_EBX);
+} else {
+x86_cpu_def-cpuid_7_0_ebx_features = 0;
+}
+
  host_cpuid(0x8000, 0,eax,ebx,ecx,edx);
  x86_cpu_def-xlevel = eax;

@@ -1185,6 +1193,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
  env-cpuid_kvm_features = def-kvm_features;
  env-cpuid_svm_features = def-svm_features;
  env-cpuid_ext4_features = def-ext4_features;
+env-cpuid_7_0_ebx = def-cpuid_7_0_ebx_features;
  env-cpuid_xlevel2 = def-xlevel2;
  object_property_set_int(OBJECT(cpu), (int64_t)def-tsc_khz * 1000,
  tsc-frequency,error);
@@ -1451,13 +1460,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
  *edx = 0;
  break;
  case 7:
-if (kvm_enabled()) {
-KVMState *s = env-kvm_state;
-
-*eax = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EAX);
-*ebx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EBX);
-*ecx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_ECX);
-*edx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EDX);
+/* Structured Extended Feature Flags Enumeration Leaf */
+if (count == 0) {
+*eax = 0; /* Maximum ECX value for sub-leaves */
+*ebx = env-cpuid_7_0_ebx; /* Feature flags */
+*ecx = 0; /* Reserved */
+*edx = 0; /* Reserved */
  } else {
  *eax = 0;
  *ebx = 0;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index b5b9a50..2460f63 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -741,6 +741,8 @@ typedef struct CPUX86State {
  /* Store the results of Centaur's 

Re: [Qemu-devel] [QEMU 1.1 PATCH v3] Expose CPUID leaf 7 only for -cpu host

2012-05-24 Thread Gleb Natapov
On Mon, May 21, 2012 at 11:27:02AM -0300, Eduardo Habkost wrote:
 Changes v2 - v3;
   - Check for kvm_enabled() before setting cpuid_7_0_ebx_features
 
 Changes v1 - v2:
   - Use kvm_arch_get_supported_cpuid() instead of host_cpuid() on
 cpu_x86_fill_host().
 
   We should use GET_SUPPORTED_CPUID for all bits on -cpu host
   eventually, but I am not changing all the other CPUID leaves because
   we may not be able to test such an intrusive change in time for 1.1.
 
 Description of the bug:
 
 Since QEMU 0.15, the CPUID information on CPUID[EAX=7,ECX=0] is being
 returned unfiltered to the guest, directly from the GET_SUPPORTED_CPUID
 return value.
 
 The problem is that this makes the resulting CPU feature flags
 unpredictable and dependent on the host CPU and kernel version. This
 breaks live-migration badly if migrating from a host CPU that supports
 some features on that CPUID leaf (running a recent kernel) to a kernel
 or host CPU that doesn't support it.
 
 Migration also is incorrect (the virtual CPU changes under the guest's
 feet) if you migrate in the opposite direction (from an old CPU/kernel
 to a new CPU/kernel), but with less serious consequences (guests
 normally query CPUID information only once on boot).
 
 Fortunately, the bug affects only users using cpudefs with level = 7.
 
 The right behavior should be to explicitly enable those features on
 [cpudef] config sections or on the -cpu command-line arguments. Right
 now there is no predefined CPU model on QEMU that has those features:
 the latest Intel model we have is Sandy Bridge.
 
 I would like to get this fixed on 1.1, so I am submitting this patch,
 that enables those features only if -cpu host is being used (as we
 don't have any pre-defined CPU model that actually have those features).
 After 1.1 is released, we can make those features properly configurable
 on [cpudef] and -cpu configuration.
 
 One problem is: with this patch, users with the following setup:
 - Running QEMU 1.0;
 - Using a cpudef having level = 7;
 - Running a kernel that supports the features on CPUID leaf 7; and
 - Running on a CPU that supports some features on CPUID leaf 7
 won't be able to live-migrate to QEMU 1.1. But for these users
 live-migration is already broken (they can't live-migrate to hosts with
 older CPUs or older kernels, already), I don't see how to avoid this
 problem.
 
 Signed-off-by: Eduardo Habkost ehabk...@redhat.com
Looks good to me.

 ---
  target-i386/cpu.c |   22 +++---
  target-i386/cpu.h |2 ++
  2 files changed, 17 insertions(+), 7 deletions(-)
 
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index 89b4ac7..388bc5c 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -238,6 +238,8 @@ typedef struct x86_def_t {
  /* Store the results of Centaur's CPUID instructions */
  uint32_t ext4_features;
  uint32_t xlevel2;
 +/* The feature bits on CPUID[EAX=7,ECX=0].EBX */
 +uint32_t cpuid_7_0_ebx_features;
  } x86_def_t;
  
  #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
 @@ -521,6 +523,12 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
  x86_cpu_def-ext_features = ecx;
  x86_cpu_def-features = edx;
  
 +if (kvm_enabled()  x86_cpu_def-level = 7) {
 +x86_cpu_def-cpuid_7_0_ebx_features = 
 kvm_arch_get_supported_cpuid(kvm_state, 0x7, 0, R_EBX);
 +} else {
 +x86_cpu_def-cpuid_7_0_ebx_features = 0;
 +}
 +
  host_cpuid(0x8000, 0, eax, ebx, ecx, edx);
  x86_cpu_def-xlevel = eax;
  
 @@ -1185,6 +1193,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
  env-cpuid_kvm_features = def-kvm_features;
  env-cpuid_svm_features = def-svm_features;
  env-cpuid_ext4_features = def-ext4_features;
 +env-cpuid_7_0_ebx = def-cpuid_7_0_ebx_features;
  env-cpuid_xlevel2 = def-xlevel2;
  object_property_set_int(OBJECT(cpu), (int64_t)def-tsc_khz * 1000,
  tsc-frequency, error);
 @@ -1451,13 +1460,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
 uint32_t count,
  *edx = 0;
  break;
  case 7:
 -if (kvm_enabled()) {
 -KVMState *s = env-kvm_state;
 -
 -*eax = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EAX);
 -*ebx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EBX);
 -*ecx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_ECX);
 -*edx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EDX);
 +/* Structured Extended Feature Flags Enumeration Leaf */
 +if (count == 0) {
 +*eax = 0; /* Maximum ECX value for sub-leaves */
 +*ebx = env-cpuid_7_0_ebx; /* Feature flags */
 +*ecx = 0; /* Reserved */
 +*edx = 0; /* Reserved */
  } else {
  *eax = 0;
  *ebx = 0;
 diff --git a/target-i386/cpu.h b/target-i386/cpu.h
 index b5b9a50..2460f63 100644
 --- a/target-i386/cpu.h
 +++ b/target-i386/cpu.h
 @@ -741,6 

[Qemu-devel] [QEMU 1.1 PATCH v3] Expose CPUID leaf 7 only for -cpu host

2012-05-21 Thread Eduardo Habkost
Changes v2 - v3;
  - Check for kvm_enabled() before setting cpuid_7_0_ebx_features

Changes v1 - v2:
  - Use kvm_arch_get_supported_cpuid() instead of host_cpuid() on
cpu_x86_fill_host().

  We should use GET_SUPPORTED_CPUID for all bits on -cpu host
  eventually, but I am not changing all the other CPUID leaves because
  we may not be able to test such an intrusive change in time for 1.1.

Description of the bug:

Since QEMU 0.15, the CPUID information on CPUID[EAX=7,ECX=0] is being
returned unfiltered to the guest, directly from the GET_SUPPORTED_CPUID
return value.

The problem is that this makes the resulting CPU feature flags
unpredictable and dependent on the host CPU and kernel version. This
breaks live-migration badly if migrating from a host CPU that supports
some features on that CPUID leaf (running a recent kernel) to a kernel
or host CPU that doesn't support it.

Migration also is incorrect (the virtual CPU changes under the guest's
feet) if you migrate in the opposite direction (from an old CPU/kernel
to a new CPU/kernel), but with less serious consequences (guests
normally query CPUID information only once on boot).

Fortunately, the bug affects only users using cpudefs with level = 7.

The right behavior should be to explicitly enable those features on
[cpudef] config sections or on the -cpu command-line arguments. Right
now there is no predefined CPU model on QEMU that has those features:
the latest Intel model we have is Sandy Bridge.

I would like to get this fixed on 1.1, so I am submitting this patch,
that enables those features only if -cpu host is being used (as we
don't have any pre-defined CPU model that actually have those features).
After 1.1 is released, we can make those features properly configurable
on [cpudef] and -cpu configuration.

One problem is: with this patch, users with the following setup:
- Running QEMU 1.0;
- Using a cpudef having level = 7;
- Running a kernel that supports the features on CPUID leaf 7; and
- Running on a CPU that supports some features on CPUID leaf 7
won't be able to live-migrate to QEMU 1.1. But for these users
live-migration is already broken (they can't live-migrate to hosts with
older CPUs or older kernels, already), I don't see how to avoid this
problem.

Signed-off-by: Eduardo Habkost ehabk...@redhat.com
---
 target-i386/cpu.c |   22 +++---
 target-i386/cpu.h |2 ++
 2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 89b4ac7..388bc5c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -238,6 +238,8 @@ typedef struct x86_def_t {
 /* Store the results of Centaur's CPUID instructions */
 uint32_t ext4_features;
 uint32_t xlevel2;
+/* The feature bits on CPUID[EAX=7,ECX=0].EBX */
+uint32_t cpuid_7_0_ebx_features;
 } x86_def_t;
 
 #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -521,6 +523,12 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
 x86_cpu_def-ext_features = ecx;
 x86_cpu_def-features = edx;
 
+if (kvm_enabled()  x86_cpu_def-level = 7) {
+x86_cpu_def-cpuid_7_0_ebx_features = 
kvm_arch_get_supported_cpuid(kvm_state, 0x7, 0, R_EBX);
+} else {
+x86_cpu_def-cpuid_7_0_ebx_features = 0;
+}
+
 host_cpuid(0x8000, 0, eax, ebx, ecx, edx);
 x86_cpu_def-xlevel = eax;
 
@@ -1185,6 +1193,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
 env-cpuid_kvm_features = def-kvm_features;
 env-cpuid_svm_features = def-svm_features;
 env-cpuid_ext4_features = def-ext4_features;
+env-cpuid_7_0_ebx = def-cpuid_7_0_ebx_features;
 env-cpuid_xlevel2 = def-xlevel2;
 object_property_set_int(OBJECT(cpu), (int64_t)def-tsc_khz * 1000,
 tsc-frequency, error);
@@ -1451,13 +1460,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 *edx = 0;
 break;
 case 7:
-if (kvm_enabled()) {
-KVMState *s = env-kvm_state;
-
-*eax = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EAX);
-*ebx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EBX);
-*ecx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_ECX);
-*edx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EDX);
+/* Structured Extended Feature Flags Enumeration Leaf */
+if (count == 0) {
+*eax = 0; /* Maximum ECX value for sub-leaves */
+*ebx = env-cpuid_7_0_ebx; /* Feature flags */
+*ecx = 0; /* Reserved */
+*edx = 0; /* Reserved */
 } else {
 *eax = 0;
 *ebx = 0;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index b5b9a50..2460f63 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -741,6 +741,8 @@ typedef struct CPUX86State {
 /* Store the results of Centaur's CPUID instructions */
 uint32_t cpuid_xlevel2;
 uint32_t cpuid_ext4_features;
+/* Flags from