Re: [Qemu-devel] [PATCH 4/4] kvm: Allow migration with invtsc

[Marcelo Tosatti]

On Tue, Dec 27, 2016 at 05:21:20PM -0200, Eduardo Habkost wrote:
> Instead of blocking migration on the source when invtsc is
> enabled, rely on the migration destination to ensure there's no
> TSC frequency mismatch.
> 
> We can't allow migration unconditionally because we don't know if
> the destination is a QEMU version that is really going to ensure
> there's no TSC frequency mismatch. To ensure we are migrating to
> a destination that won't ignore SET_TSC_KHZ errors, allow invtsc
> migration only on pc-*-2.9 and newer.
> 
> Signed-off-by: Eduardo Habkost <address@hidden>
> ---
>  include/hw/i386/pc.h |  7 ++++++-
>  target/i386/cpu.h    |  1 +
>  target/i386/cpu.c    |  1 +
>  target/i386/kvm.c    | 15 +++++++++------
>  4 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index ceeacca..4270923 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -375,7 +375,12 @@ int e820_get_num_entries(void);
>  bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
>  
>  #define PC_COMPAT_2_8 \
> -    HW_COMPAT_2_8
> +    HW_COMPAT_2_8 \
> +    {\
> +        .driver   = TYPE_X86_CPU,\
> +        .property = "invtsc-migration",\
> +        .value    = "off",\
> +    },
>  
>  #define PC_COMPAT_2_7 \
>      HW_COMPAT_2_7 \
> diff --git a/target/i386/cpu.h b/target/i386/cpu.h
> index a7f2f60..ec8cdbc 100644
> --- a/target/i386/cpu.h
> +++ b/target/i386/cpu.h
> @@ -1208,6 +1208,7 @@ struct X86CPU {
>      bool expose_kvm;
>      bool migratable;
>      bool host_features;
> +    bool invtsc_migration;
>      uint32_t apic_id;
>  
>      /* if true the CPUID code directly forward host cache leaves to the 
> guest */
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index b0640f1..cc93b81 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3678,6 +3678,7 @@ static Property x86_cpu_properties[] = {
>      DEFINE_PROP_BOOL("cpuid-0xb", X86CPU, enable_cpuid_0xb, true),
>      DEFINE_PROP_BOOL("lmce", X86CPU, enable_lmce, false),
>      DEFINE_PROP_BOOL("l3-cache", X86CPU, enable_l3_cache, true),
> +    DEFINE_PROP_BOOL("invtsc-migration", X86CPU, invtsc_migration, true),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 6a51399..2c3ee7b 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -962,7 +962,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>          has_msr_mcg_ext_ctl = has_msr_feature_control = true;
>      }
>  
> -    if (!env->user_tsc_khz) {
> +    if (!cpu->invtsc_migration && !env->user_tsc_khz) {
>          if ((env->features[FEAT_8000_0007_EDX] & CPUID_APM_INVTSC) &&
>              invtsc_mig_blocker == NULL) {
>              /* for migration */
> @@ -972,6 +972,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
>              migrate_add_blocker(invtsc_mig_blocker);
>              /* for savevm */
>              vmstate_x86_cpu.unmigratable = 1;
> +        }
>      }
>  
>      cpuid_data.cpuid.padding = 0;
> @@ -2655,12 +2656,14 @@ int kvm_arch_put_registers(CPUState *cpu, int level)
>      }
>  
>      if (level == KVM_PUT_FULL_STATE) {
> -        /* We don't check for kvm_arch_set_tsc_khz() errors here,
> -         * because TSC frequency mismatch shouldn't abort migration,
> -         * unless the user explicitly asked for a more strict TSC
> -         * setting (e.g. using an explicit "tsc-freq" option).
> +        /* Migration TSC frequency mismatch is fatal only if we are
> +         * actually reporting Invariant TSC to the guest.
>           */
> -        kvm_arch_set_tsc_khz(cpu);
> +        ret = kvm_arch_set_tsc_khz(cpu);
> +        if ((x86_cpu->env.features[FEAT_8000_0007_EDX] & CPUID_APM_INVTSC) &&
> +            ret < 0) {
> +            return ret;
> +        }
>      }

Will the guest continue in the source in this case?

I think this is past the point where migration has been declared
successful. 

Otherwise looks good.