| Message ID | 20140423210444.GA4305@amt.cnet |
|---|---|
| State | New |
| Headers | show |
On Wed, Apr 23, 2014 at 06:04:45PM -0300, Marcelo Tosatti wrote: > > Invariant TSC documentation mentions that "invariant TSC will run at a > constant rate in all ACPI P-, C-. and T-states". > > This is not the case if migration to a host with different TSC frequency > is allowed, or if savevm is performed. So block migration/savevm. > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > [...] > @@ -702,6 +706,16 @@ int kvm_arch_init_vcpu(CPUState *cs) > !!(c->ecx & CPUID_EXT_SMX); > } > > + c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0); > + if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) { > + /* for migration */ > + error_set(&invtsc_mig_blocker, > + QERR_DEVICE_FEATURE_BLOCKS_MIGRATION, "invtsc", "cpu"); > + migrate_add_blocker(invtsc_mig_blocker); > + /* for savevm */ > + vmstate_x86_cpu.unmigratable = 1; Did you ensure this will always happen before vmstate_register() is called for vmstate_x86_cpu? I believe kvm_arch_init_vcpu() is called a long long time after device_set_realized() (which is where vmstate_register() is called for DeviceState objects).
On Thu, Apr 24, 2014 at 04:21:59PM -0300, Eduardo Habkost wrote: > On Wed, Apr 23, 2014 at 06:04:45PM -0300, Marcelo Tosatti wrote: > > > > Invariant TSC documentation mentions that "invariant TSC will run at a > > constant rate in all ACPI P-, C-. and T-states". > > > > This is not the case if migration to a host with different TSC frequency > > is allowed, or if savevm is performed. So block migration/savevm. > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > [...] > > @@ -702,6 +706,16 @@ int kvm_arch_init_vcpu(CPUState *cs) > > !!(c->ecx & CPUID_EXT_SMX); > > } > > > > + c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0); > > + if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) { > > + /* for migration */ > > + error_set(&invtsc_mig_blocker, > > + QERR_DEVICE_FEATURE_BLOCKS_MIGRATION, "invtsc", "cpu"); > > + migrate_add_blocker(invtsc_mig_blocker); > > + /* for savevm */ > > + vmstate_x86_cpu.unmigratable = 1; > > Did you ensure this will always happen before vmstate_register() is > called for vmstate_x86_cpu? I believe kvm_arch_init_vcpu() is called a > long long time after device_set_realized() (which is where > vmstate_register() is called for DeviceState objects). x86_cpu_realizefn -> qemu_init_vcpu -> qemu_kvm_start_vcpu -> qemu_kvm_cpu_thread_fn -> kvm_init_vcpu -> kvm_arch_init_vcpu @@ -2573,6 +2598,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) CPUX86State *env = &cpu->env; Error *local_err = NULL; + printf("%s: dev->realized=%d\n", __func__, dev->realized); + if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) { env->cpuid_level = 7; } QEMU 1.7.93 monitor - type 'help' for more information (qemu) x86_cpu_realizefn: dev->realized=0 x86_cpu_realizefn: dev->realized=0 audio: Could not init `oss' audio driver
On Thu, Apr 24, 2014 at 06:32:42PM -0300, Marcelo Tosatti wrote: > On Thu, Apr 24, 2014 at 04:21:59PM -0300, Eduardo Habkost wrote: > > On Wed, Apr 23, 2014 at 06:04:45PM -0300, Marcelo Tosatti wrote: > > > > > > Invariant TSC documentation mentions that "invariant TSC will run at a > > > constant rate in all ACPI P-, C-. and T-states". > > > > > > This is not the case if migration to a host with different TSC frequency > > > is allowed, or if savevm is performed. So block migration/savevm. > > > > > > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com> > > > > > [...] > > > @@ -702,6 +706,16 @@ int kvm_arch_init_vcpu(CPUState *cs) > > > !!(c->ecx & CPUID_EXT_SMX); > > > } > > > > > > + c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0); > > > + if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) { > > > + /* for migration */ > > > + error_set(&invtsc_mig_blocker, > > > + QERR_DEVICE_FEATURE_BLOCKS_MIGRATION, "invtsc", "cpu"); > > > + migrate_add_blocker(invtsc_mig_blocker); > > > + /* for savevm */ > > > + vmstate_x86_cpu.unmigratable = 1; > > > > Did you ensure this will always happen before vmstate_register() is > > called for vmstate_x86_cpu? I believe kvm_arch_init_vcpu() is called a > > long long time after device_set_realized() (which is where > > vmstate_register() is called for DeviceState objects). > > x86_cpu_realizefn x86_cpu_realizefn is called by device_set_realized (via DeviceClass.realize), but vmstate_register*() is called _after_ DeviceClass.realize. Continuing: > -> qemu_init_vcpu -> qemu_kvm_start_vcpu -> > qemu_kvm_cpu_thread_fn ^^^^ this is run in a new thread > -> kvm_init_vcpu -> kvm_arch_init_vcpu ^^^^ this is called after acquiring qemu_global_mutex. Then it gets tricky: qemu_kvm_start_vcpu() returns only after cpu->created is set, which is done after kvm_arch_init_vcpu() is called. BUT: I was mistaken. This is not where vmstate_x86_cpu is registered. It is at cpu_exec_init(). And cpu_exec_init() is called much earlier, at x86_cpu_initfn(). See the printf() results below. Have you tested if your patch actually blocks savevm? > > > @@ -2573,6 +2598,8 @@ static void x86_cpu_realizefn(DeviceState *dev, > Error **errp) > CPUX86State *env = &cpu->env; > Error *local_err = NULL; > > + printf("%s: dev->realized=%d\n", __func__, dev->realized); > + > if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) { > env->cpuid_level = 7; > } > > > > QEMU 1.7.93 monitor - type 'help' for more information > (qemu) x86_cpu_realizefn: dev->realized=0 > x86_cpu_realizefn: dev->realized=0 > audio: Could not init `oss' audio driver diff --git a/exec.c b/exec.c index 91513c6..1fd3e12 100644 --- a/exec.c +++ b/exec.c @@ -505,6 +505,7 @@ void cpu_exec_init(CPUArchState *env) assert(qdev_get_vmsd(DEVICE(cpu)) == NULL); #endif if (cc->vmsd != NULL) { + printf("registering CPUClass.vmsd\n"); vmstate_register(NULL, cpu_index, cc->vmsd, cpu); } } diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 73643d7..42c874c 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -468,6 +468,7 @@ int kvm_arch_init_vcpu(CPUState *cs) int kvm_base = KVM_CPUID_SIGNATURE; int r; + printf("%s: realized=%d\n", __func__, DEVICE(cpu)->realized); memset(&cpuid_data, 0, sizeof(cpuid_data)); cpuid_i = 0; QEMU 2.0.50 monitor - type 'help' for more information (qemu) registering CPUClass.vmsd kvm_arch_init_vcpu: realized=0
On Fri, Apr 25, 2014 at 05:38:08PM -0300, Eduardo Habkost wrote:
> Have you tested if your patch actually blocks savevm?
Yes, with -smp 2.
Can you please include the savevm.c patch in your queue?
TIA.
Index: qemu-invariant-tsc/target-i386/kvm.c =================================================================== --- qemu-invariant-tsc.orig/target-i386/kvm.c +++ qemu-invariant-tsc/target-i386/kvm.c @@ -33,6 +33,8 @@ #include "exec/ioport.h" #include <asm/hyperv.h> #include "hw/pci/pci.h" +#include "migration/migration.h" +#include "qapi/qmp/qerror.h" //#define DEBUG_KVM @@ -447,6 +449,8 @@ static bool hyperv_enabled(X86CPU *cpu) cpu->hyperv_relaxed_timing); } +Error *invtsc_mig_blocker; + #define KVM_MAX_CPUID_ENTRIES 100 int kvm_arch_init_vcpu(CPUState *cs) @@ -702,6 +706,16 @@ int kvm_arch_init_vcpu(CPUState *cs) !!(c->ecx & CPUID_EXT_SMX); } + c = cpuid_find_entry(&cpuid_data.cpuid, 0x80000007, 0); + if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) { + /* for migration */ + error_set(&invtsc_mig_blocker, + QERR_DEVICE_FEATURE_BLOCKS_MIGRATION, "invtsc", "cpu"); + migrate_add_blocker(invtsc_mig_blocker); + /* for savevm */ + vmstate_x86_cpu.unmigratable = 1; + } + cpuid_data.cpuid.padding = 0; r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data); if (r) { Index: qemu-invariant-tsc/target-i386/cpu-qom.h =================================================================== --- qemu-invariant-tsc.orig/target-i386/cpu-qom.h +++ qemu-invariant-tsc/target-i386/cpu-qom.h @@ -116,7 +116,7 @@ static inline X86CPU *x86_env_get_cpu(CP #define ENV_OFFSET offsetof(X86CPU, env) #ifndef CONFIG_USER_ONLY -extern const struct VMStateDescription vmstate_x86_cpu; +extern struct VMStateDescription vmstate_x86_cpu; #endif /** Index: qemu-invariant-tsc/target-i386/machine.c =================================================================== --- qemu-invariant-tsc.orig/target-i386/machine.c +++ qemu-invariant-tsc/target-i386/machine.c @@ -613,7 +613,7 @@ static const VMStateDescription vmstate_ } }; -const VMStateDescription vmstate_x86_cpu = { +VMStateDescription vmstate_x86_cpu = { .name = "cpu", .version_id = 12, .minimum_version_id = 3,
Invariant TSC documentation mentions that "invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states". This is not the case if migration to a host with different TSC frequency is allowed, or if savevm is performed. So block migration/savevm. Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>