From mboxrd@z Thu Jan 1 00:00:00 1970 From: Haozhong Zhang Subject: Re: [PATCH v4 2/2] target-i386: add support to migrate vcpu's TSC rate Date: Mon, 16 Nov 2015 22:30:08 +0800 Message-ID: <20151116143008.GA22212@hzzhang-OptiPlex-9020.sh.intel.com> References: <1447661048-31048-1-git-send-email-haozhong.zhang@intel.com> <1447661048-31048-3-git-send-email-haozhong.zhang@intel.com> <20151116134311.GW4180@thinpad.lan.raisama.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: , "Dr. David Alan Gilbert" , Paolo Bonzini , Richard Henderson , "Michael S. Tsirkin" , , Marcelo Tosatti , To: Eduardo Habkost Return-path: Received: from mga03.intel.com ([134.134.136.65]:37141 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750777AbbKPOaM (ORCPT ); Mon, 16 Nov 2015 09:30:12 -0500 Content-Disposition: inline In-Reply-To: <20151116134311.GW4180@thinpad.lan.raisama.net> Sender: kvm-owner@vger.kernel.org List-ID: On 11/16/15 11:43, Eduardo Habkost wrote: > On Mon, Nov 16, 2015 at 04:04:08PM +0800, Haozhong Zhang wrote: > > This patch enables migrating vcpu's TSC rate. If KVM on the destination > > machine supports TSC scaling, guest programs will observe a consistent > > TSC rate across the migration. > > > > If TSC scaling is not supported on the destination machine, the > > migration will not be aborted and QEMU on the destination will not set > > vcpu's TSC rate to the migrated value. > > > > If vcpu's TSC rate specified by CPU option 'tsc-freq' on the destination > > machine is inconsistent with the migrated TSC rate, the migration will > > be aborted. > > > > For backwards compatibility, the migration of vcpu's TSC rate is > > disabled on pc-*-2.4 and older machine types. > > > > Signed-off-by: Haozhong Zhang > > --- > > hw/i386/pc.c | 1 + > > hw/i386/pc_piix.c | 1 + > > hw/i386/pc_q35.c | 1 + > > include/hw/i386/pc.h | 1 + > > target-i386/cpu.c | 2 +- > > target-i386/cpu.h | 1 + > > target-i386/kvm.c | 26 ++++++++++++++++++++++++-- > > target-i386/machine.c | 28 ++++++++++++++++++++++++++++ > > 8 files changed, 58 insertions(+), 3 deletions(-) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 0cb8afd..2f2fc93 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1952,6 +1952,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > > HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc); > > > > pcmc->get_hotplug_handler = mc->get_hotplug_handler; > > + pcmc->save_tsc_khz = true; > > mc->get_hotplug_handler = pc_get_hotpug_handler; > > mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id; > > mc->default_boot_order = "cad"; > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > > index 07d0baa..7c5b0d2 100644 > > --- a/hw/i386/pc_piix.c > > +++ b/hw/i386/pc_piix.c > > @@ -489,6 +489,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m) > > m->alias = NULL; > > m->is_default = 0; > > pcmc->broken_reserved_end = true; > > + pcmc->save_tsc_khz = false; > > SET_MACHINE_COMPAT(m, PC_COMPAT_2_4); > > } > > > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > > index 0fdae09..fd8efe3 100644 > > --- a/hw/i386/pc_q35.c > > +++ b/hw/i386/pc_q35.c > > @@ -387,6 +387,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m) > > m->hw_version = "2.4.0"; > > m->alias = NULL; > > pcmc->broken_reserved_end = true; > > + pcmc->save_tsc_khz = false; > > SET_MACHINE_COMPAT(m, PC_COMPAT_2_4); > > } > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > index 4bbc0ff..fea0f28 100644 > > --- a/include/hw/i386/pc.h > > +++ b/include/hw/i386/pc.h > > @@ -60,6 +60,7 @@ struct PCMachineClass { > > > > /*< public >*/ > > bool broken_reserved_end; > > + bool save_tsc_khz; > > HotplugHandler *(*get_hotplug_handler)(MachineState *machine, > > DeviceState *dev); > > }; > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index e5f1c5b..98c6a4c 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -1724,7 +1724,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque, > > return; > > } > > > > - cpu->env.tsc_khz = value / 1000; > > + cpu->env.tsc_khz = cpu->env.user_tsc_khz = value / 1000; > > } > > > > static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque, > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > > index fc4a605..1ad1da8 100644 > > --- a/target-i386/cpu.h > > +++ b/target-i386/cpu.h > > @@ -973,6 +973,7 @@ typedef struct CPUX86State { > > uint32_t sipi_vector; > > bool tsc_valid; > > int64_t tsc_khz; > > + int64_t user_tsc_khz; > > void *kvm_xsave_buf; > > > > uint64_t mcg_cap; > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > > index 9084b29..8448248 100644 > > --- a/target-i386/kvm.c > > +++ b/target-i386/kvm.c > > @@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs) > > int r; > > > > /* > > - * If no user-specified TSC frequency is present, we will try to > > - * set env->tsc_khz to the value used by KVM. > > + * For other cases of env->tsc_khz and env->user_tsc_khz: > > + * > > + * - We have eliminated all cases that satisfy > > + * env->tsc_khz && env->user_tsc_khz && > > + * env->tsc_khz != env->user_tsc_khz > > + * in cpu_post_load(). > > + * > > + * - If env->user_tsc_khz is not zero, then it must be equal to > > + * env->tsc_khz (if the latter is not zero) and has been set in > > + * kvm_arch_init_vcpu(). > > + */ > > + if (env->tsc_khz && !env->user_tsc_khz) { > > + r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ? > > + kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) : -ENOTSUP; > > Please don't duplicate the code from kvm_arch_init_vcpu(). We can > handle both cases in kvm_arch_put_registers(KVM_PUT_FULL_STATE), > can't we? > No. If KVM_SET_TSC_KHZ fails in kvm_arch_init_vcpu(), QEMU will exit. But if KVM_SET_TSC_KHZ fails in the migration, QEMU will not abort. And because the return value of kvm_arch_put_registers(KVM_PUT_FULL_STATE) is not checked by its caller do_kvm_cpu_synchronize_post_init(), I have to handle them in different ways. Haozhong > > + if (r < 0) { > > + error_report("warning: TSC frequency mismatch between VM and host, " > > + "and TSC scaling unavailable"); > > + } > > + } > > + > > + /* > > + * If neither the user-specified TSC frequency nor the migrated > > + * TSC frequency is present, we will try to set env->tsc_khz > > + * to the value used by KVM. > > */ > > if (!env->tsc_khz) { > > r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ? > > diff --git a/target-i386/machine.c b/target-i386/machine.c > > index a18e16e..eb062d2 100644 > > --- a/target-i386/machine.c > > +++ b/target-i386/machine.c > > @@ -331,6 +331,13 @@ static int cpu_post_load(void *opaque, int version_id) > > CPUX86State *env = &cpu->env; > > int i; > > > > + if (env->tsc_khz && env->user_tsc_khz && > > + env->tsc_khz != env->user_tsc_khz) { > > + fprintf(stderr, "Mismatch between user-specified TSC frequency and " > > + "migrated TSC frequency"); > > + return -1; > > + } > > + > > /* > > * Real mode guest segments register DPL should be zero. > > * Older KVM version were setting it wrongly. > > @@ -775,6 +782,26 @@ static const VMStateDescription vmstate_xss = { > > } > > }; > > > > +static bool tsc_khz_needed(void *opaque) > > +{ > > + X86CPU *cpu = opaque; > > + CPUX86State *env = &cpu->env; > > + MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); > > + PCMachineClass *pcmc = PC_MACHINE_CLASS(mc); > > + return env->tsc_khz && pcmc->save_tsc_khz; > > +} > > + > > +static const VMStateDescription vmstate_tsc_khz = { > > + .name = "cpu/tsc_khz", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .needed = tsc_khz_needed, > > + .fields = (VMStateField[]) { > > + VMSTATE_INT64(env.tsc_khz, X86CPU), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > + > > VMStateDescription vmstate_x86_cpu = { > > .name = "cpu", > > .version_id = 12, > > @@ -895,6 +922,7 @@ VMStateDescription vmstate_x86_cpu = { > > &vmstate_msr_hyperv_runtime, > > &vmstate_avx512, > > &vmstate_xss, > > + &vmstate_tsc_khz, > > NULL > > } > > }; > > -- > > 2.4.8 > > > > -- > Eduardo From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39508) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyKnT-0005dx-Mj for qemu-devel@nongnu.org; Mon, 16 Nov 2015 09:30:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZyKnO-0001d0-E0 for qemu-devel@nongnu.org; Mon, 16 Nov 2015 09:30:19 -0500 Received: from mga14.intel.com ([192.55.52.115]:63335) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZyKnO-0001cY-4H for qemu-devel@nongnu.org; Mon, 16 Nov 2015 09:30:14 -0500 Date: Mon, 16 Nov 2015 22:30:08 +0800 From: Haozhong Zhang Message-ID: <20151116143008.GA22212@hzzhang-OptiPlex-9020.sh.intel.com> References: <1447661048-31048-1-git-send-email-haozhong.zhang@intel.com> <1447661048-31048-3-git-send-email-haozhong.zhang@intel.com> <20151116134311.GW4180@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151116134311.GW4180@thinpad.lan.raisama.net> Subject: Re: [Qemu-devel] [PATCH v4 2/2] target-i386: add support to migrate vcpu's TSC rate List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: kvm@vger.kernel.org, "Michael S. Tsirkin" , Marcelo Tosatti , qemu-devel@nongnu.org, "Dr. David Alan Gilbert" , Paolo Bonzini , afaerber@suse.de, Richard Henderson On 11/16/15 11:43, Eduardo Habkost wrote: > On Mon, Nov 16, 2015 at 04:04:08PM +0800, Haozhong Zhang wrote: > > This patch enables migrating vcpu's TSC rate. If KVM on the destination > > machine supports TSC scaling, guest programs will observe a consistent > > TSC rate across the migration. > > > > If TSC scaling is not supported on the destination machine, the > > migration will not be aborted and QEMU on the destination will not set > > vcpu's TSC rate to the migrated value. > > > > If vcpu's TSC rate specified by CPU option 'tsc-freq' on the destination > > machine is inconsistent with the migrated TSC rate, the migration will > > be aborted. > > > > For backwards compatibility, the migration of vcpu's TSC rate is > > disabled on pc-*-2.4 and older machine types. > > > > Signed-off-by: Haozhong Zhang > > --- > > hw/i386/pc.c | 1 + > > hw/i386/pc_piix.c | 1 + > > hw/i386/pc_q35.c | 1 + > > include/hw/i386/pc.h | 1 + > > target-i386/cpu.c | 2 +- > > target-i386/cpu.h | 1 + > > target-i386/kvm.c | 26 ++++++++++++++++++++++++-- > > target-i386/machine.c | 28 ++++++++++++++++++++++++++++ > > 8 files changed, 58 insertions(+), 3 deletions(-) > > > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > index 0cb8afd..2f2fc93 100644 > > --- a/hw/i386/pc.c > > +++ b/hw/i386/pc.c > > @@ -1952,6 +1952,7 @@ static void pc_machine_class_init(ObjectClass *oc, void *data) > > HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc); > > > > pcmc->get_hotplug_handler = mc->get_hotplug_handler; > > + pcmc->save_tsc_khz = true; > > mc->get_hotplug_handler = pc_get_hotpug_handler; > > mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id; > > mc->default_boot_order = "cad"; > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > > index 07d0baa..7c5b0d2 100644 > > --- a/hw/i386/pc_piix.c > > +++ b/hw/i386/pc_piix.c > > @@ -489,6 +489,7 @@ static void pc_i440fx_2_4_machine_options(MachineClass *m) > > m->alias = NULL; > > m->is_default = 0; > > pcmc->broken_reserved_end = true; > > + pcmc->save_tsc_khz = false; > > SET_MACHINE_COMPAT(m, PC_COMPAT_2_4); > > } > > > > diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c > > index 0fdae09..fd8efe3 100644 > > --- a/hw/i386/pc_q35.c > > +++ b/hw/i386/pc_q35.c > > @@ -387,6 +387,7 @@ static void pc_q35_2_4_machine_options(MachineClass *m) > > m->hw_version = "2.4.0"; > > m->alias = NULL; > > pcmc->broken_reserved_end = true; > > + pcmc->save_tsc_khz = false; > > SET_MACHINE_COMPAT(m, PC_COMPAT_2_4); > > } > > > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > > index 4bbc0ff..fea0f28 100644 > > --- a/include/hw/i386/pc.h > > +++ b/include/hw/i386/pc.h > > @@ -60,6 +60,7 @@ struct PCMachineClass { > > > > /*< public >*/ > > bool broken_reserved_end; > > + bool save_tsc_khz; > > HotplugHandler *(*get_hotplug_handler)(MachineState *machine, > > DeviceState *dev); > > }; > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > > index e5f1c5b..98c6a4c 100644 > > --- a/target-i386/cpu.c > > +++ b/target-i386/cpu.c > > @@ -1724,7 +1724,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque, > > return; > > } > > > > - cpu->env.tsc_khz = value / 1000; > > + cpu->env.tsc_khz = cpu->env.user_tsc_khz = value / 1000; > > } > > > > static void x86_cpuid_get_apic_id(Object *obj, Visitor *v, void *opaque, > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > > index fc4a605..1ad1da8 100644 > > --- a/target-i386/cpu.h > > +++ b/target-i386/cpu.h > > @@ -973,6 +973,7 @@ typedef struct CPUX86State { > > uint32_t sipi_vector; > > bool tsc_valid; > > int64_t tsc_khz; > > + int64_t user_tsc_khz; > > void *kvm_xsave_buf; > > > > uint64_t mcg_cap; > > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > > index 9084b29..8448248 100644 > > --- a/target-i386/kvm.c > > +++ b/target-i386/kvm.c > > @@ -2334,8 +2334,30 @@ static void kvm_arch_set_tsc_khz(CPUState *cs) > > int r; > > > > /* > > - * If no user-specified TSC frequency is present, we will try to > > - * set env->tsc_khz to the value used by KVM. > > + * For other cases of env->tsc_khz and env->user_tsc_khz: > > + * > > + * - We have eliminated all cases that satisfy > > + * env->tsc_khz && env->user_tsc_khz && > > + * env->tsc_khz != env->user_tsc_khz > > + * in cpu_post_load(). > > + * > > + * - If env->user_tsc_khz is not zero, then it must be equal to > > + * env->tsc_khz (if the latter is not zero) and has been set in > > + * kvm_arch_init_vcpu(). > > + */ > > + if (env->tsc_khz && !env->user_tsc_khz) { > > + r = kvm_check_extension(cs->kvm_state, KVM_CAP_TSC_CONTROL) ? > > + kvm_vcpu_ioctl(cs, KVM_SET_TSC_KHZ, env->tsc_khz) : -ENOTSUP; > > Please don't duplicate the code from kvm_arch_init_vcpu(). We can > handle both cases in kvm_arch_put_registers(KVM_PUT_FULL_STATE), > can't we? > No. If KVM_SET_TSC_KHZ fails in kvm_arch_init_vcpu(), QEMU will exit. But if KVM_SET_TSC_KHZ fails in the migration, QEMU will not abort. And because the return value of kvm_arch_put_registers(KVM_PUT_FULL_STATE) is not checked by its caller do_kvm_cpu_synchronize_post_init(), I have to handle them in different ways. Haozhong > > + if (r < 0) { > > + error_report("warning: TSC frequency mismatch between VM and host, " > > + "and TSC scaling unavailable"); > > + } > > + } > > + > > + /* > > + * If neither the user-specified TSC frequency nor the migrated > > + * TSC frequency is present, we will try to set env->tsc_khz > > + * to the value used by KVM. > > */ > > if (!env->tsc_khz) { > > r = kvm_check_extension(cs->kvm_state, KVM_CAP_GET_TSC_KHZ) ? > > diff --git a/target-i386/machine.c b/target-i386/machine.c > > index a18e16e..eb062d2 100644 > > --- a/target-i386/machine.c > > +++ b/target-i386/machine.c > > @@ -331,6 +331,13 @@ static int cpu_post_load(void *opaque, int version_id) > > CPUX86State *env = &cpu->env; > > int i; > > > > + if (env->tsc_khz && env->user_tsc_khz && > > + env->tsc_khz != env->user_tsc_khz) { > > + fprintf(stderr, "Mismatch between user-specified TSC frequency and " > > + "migrated TSC frequency"); > > + return -1; > > + } > > + > > /* > > * Real mode guest segments register DPL should be zero. > > * Older KVM version were setting it wrongly. > > @@ -775,6 +782,26 @@ static const VMStateDescription vmstate_xss = { > > } > > }; > > > > +static bool tsc_khz_needed(void *opaque) > > +{ > > + X86CPU *cpu = opaque; > > + CPUX86State *env = &cpu->env; > > + MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine()); > > + PCMachineClass *pcmc = PC_MACHINE_CLASS(mc); > > + return env->tsc_khz && pcmc->save_tsc_khz; > > +} > > + > > +static const VMStateDescription vmstate_tsc_khz = { > > + .name = "cpu/tsc_khz", > > + .version_id = 1, > > + .minimum_version_id = 1, > > + .needed = tsc_khz_needed, > > + .fields = (VMStateField[]) { > > + VMSTATE_INT64(env.tsc_khz, X86CPU), > > + VMSTATE_END_OF_LIST() > > + } > > +}; > > + > > VMStateDescription vmstate_x86_cpu = { > > .name = "cpu", > > .version_id = 12, > > @@ -895,6 +922,7 @@ VMStateDescription vmstate_x86_cpu = { > > &vmstate_msr_hyperv_runtime, > > &vmstate_avx512, > > &vmstate_xss, > > + &vmstate_tsc_khz, > > NULL > > } > > }; > > -- > > 2.4.8 > > > > -- > Eduardo