Re: [Qemu-devel] [PATCH 1/3] target-i386: disable pv eoi to fix migration across QEMU versions

[Eduardo Habkost]

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 <address@hidden>

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

Reviewed-by: Eduardo Habkost <address@hidden>
Tested-by: Eduardo Habkost <address@hidden>

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

-- 
Eduardo