Patchwork [1/3] target-i386: disable pv eoi to fix migration across QEMU versions

login
register
mail settings
Submitter Anthony Liguori
Date Aug. 29, 2012, 3:52 p.m.
Message ID <1346255529-16534-1-git-send-email-aliguori@us.ibm.com>
Download mbox | patch
Permalink /patch/180718/
State New
Headers show

Comments

Anthony Liguori - Aug. 29, 2012, 3:52 p.m.
We have a problem with how we handle migration with KVM paravirt features.
We unconditionally enable paravirt features regardless of whether we know how
to migrate them.

We also don't tie paravirt features to specific machine types so an old QEMU on
a new kernel would expose features that never existed.

The 1.2 cycle is over and as things stand, migration is broken.  Michael has
another series that adds support for migrating PV EOI and attempts to make it
work correctly for different machine types.

After speaking with Michael on IRC, we agreed to take this patch plus 1 & 4
from his series.  This makes sure QEMU can migrate PV EOI if it's enabled, but
does not enable it by default.

This also means that we won't unconditionally enable new features for guests
future proofing us from this happening again in the future.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 target-i386/cpu.c |   13 ++++++++++++-
 1 files changed, 12 insertions(+), 1 deletions(-)
Eduardo Habkost - Aug. 29, 2012, 4:11 p.m.
On Wed, Aug 29, 2012 at 10:52:07AM -0500, Anthony Liguori wrote:
> We have a problem with how we handle migration with KVM paravirt features.
> We unconditionally enable paravirt features regardless of whether we know how
> to migrate them.
> 
> We also don't tie paravirt features to specific machine types so an old QEMU on
> a new kernel would expose features that never existed.
> 
> The 1.2 cycle is over and as things stand, migration is broken.  Michael has
> another series that adds support for migrating PV EOI and attempts to make it
> work correctly for different machine types.
> 
> After speaking with Michael on IRC, we agreed to take this patch plus 1 & 4
> from his series.  This makes sure QEMU can migrate PV EOI if it's enabled, but
> does not enable it by default.
> 
> This also means that we won't unconditionally enable new features for guests
> future proofing us from this happening again in the future.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Lots of trailing whitespaces below, but they can be fixed before pushing
the patch, so:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>
Tested-by: Eduardo Habkost <ehabkost@redhat.com>

(tested on a 3.5.2-1.fc17.x86_64 host. all features were enabled on
guest, except for PV_EOI and MMU_OP, as expected)


> ---
>  target-i386/cpu.c |   13 ++++++++++++-
>  1 files changed, 12 insertions(+), 1 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 120a2e3..f3cac49 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -33,6 +33,7 @@
>  #include "hyperv.h"
>  
>  #include "hw/hw.h"
> +#include <linux/kvm_para.h>
>  
>  /* feature flags taken from "Intel Processor Identification and the CPUID
>   * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
> @@ -887,7 +888,17 @@ 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 */
> +#if defined(CONFIG_KVM)
> +    plus_kvm_features = (1 << KVM_FEATURE_CLOCKSOURCE) |
> +        (1 << KVM_FEATURE_NOP_IO_DELAY) | 
> +        (1 << KVM_FEATURE_MMU_OP) |
> +        (1 << KVM_FEATURE_CLOCKSOURCE2) |
> +        (1 << KVM_FEATURE_ASYNC_PF) | 
> +        (1 << KVM_FEATURE_STEAL_TIME) |
> +        (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT);
> +#else
> +    plus_kvm_features = 0;
> +#endif
>  
>      add_flagname_to_bitmaps("hypervisor", &plus_features,
>          &plus_ext_features, &plus_ext2_features, &plus_ext3_features,
> -- 
> 1.7.5.4
> 
>

Patch

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 120a2e3..f3cac49 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -33,6 +33,7 @@ 
 #include "hyperv.h"
 
 #include "hw/hw.h"
+#include <linux/kvm_para.h>
 
 /* feature flags taken from "Intel Processor Identification and the CPUID
  * Instruction" and AMD's "CPUID Specification".  In cases of disagreement
@@ -887,7 +888,17 @@  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 */
+#if defined(CONFIG_KVM)
+    plus_kvm_features = (1 << KVM_FEATURE_CLOCKSOURCE) |
+        (1 << KVM_FEATURE_NOP_IO_DELAY) | 
+        (1 << KVM_FEATURE_MMU_OP) |
+        (1 << KVM_FEATURE_CLOCKSOURCE2) |
+        (1 << KVM_FEATURE_ASYNC_PF) | 
+        (1 << KVM_FEATURE_STEAL_TIME) |
+        (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT);
+#else
+    plus_kvm_features = 0;
+#endif
 
     add_flagname_to_bitmaps("hypervisor", &plus_features,
         &plus_ext_features, &plus_ext2_features, &plus_ext3_features,