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

Marcelo Tosatti
2017-01-04T11:56:56Z

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 <ehabkost@redhat.com>
> ---
> 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.