On 2011-02-02 13:16, Glauber Costa wrote: > If the machine is stopped, we should not record two different tsc values > upon a save operation. The same problem happens with kvmclock. > > But kvmclock is taking a different diretion, being now seen as a separate > device. Since this is unlikely to happen with the tsc, I am taking the > approach here of simply registering a handler for state change, and > using a per-CPUState variable that prevents double updates for the TSC. > > Signed-off-by: Glauber Costa <glommer@xxxxxxxxxx> > CC: Jan Kiszka <jan.kiszka@xxxxxx> > > --- > v2: updated tsc validation logic, as asked by Jan > --- > target-i386/cpu.h | 1 + > target-i386/kvm.c | 18 +++++++++++++++++- > 2 files changed, 18 insertions(+), 1 deletions(-) > > diff --git a/target-i386/cpu.h b/target-i386/cpu.h > index 6d619e8..6bb2e87 100644 > --- a/target-i386/cpu.h > +++ b/target-i386/cpu.h > @@ -732,6 +732,7 @@ typedef struct CPUX86State { > uint32_t sipi_vector; > uint32_t cpuid_kvm_features; > uint32_t cpuid_svm_features; > + bool tsc_valid; > > /* in order to simplify APIC support, we leave this pointer to the > user */ > diff --git a/target-i386/kvm.c b/target-i386/kvm.c > index ecb8405..9cc198a 100644 > --- a/target-i386/kvm.c > +++ b/target-i386/kvm.c > @@ -302,6 +302,15 @@ void kvm_inject_x86_mce(CPUState *cenv, int bank, uint64_t status, > > static int _kvm_arch_init_vcpu(CPUState *env); > > +static void cpu_update_state(void *opaque, int running, int reason) > +{ > + CPUState *env = opaque; > + > + if (running) { > + env->tsc_valid = false; > + } > +} > + > int kvm_arch_init_vcpu(CPUState *env) > { > int r; > @@ -444,6 +453,8 @@ int kvm_arch_init_vcpu(CPUState *env) > } > #endif > > + qemu_add_vm_change_state_handler(cpu_update_state, env); > + > return kvm_vcpu_ioctl(env, KVM_SET_CPUID2, &cpuid_data); > } > > @@ -1093,7 +1104,12 @@ static int kvm_get_msrs(CPUState *env) > msrs[n++].index = MSR_STAR; > if (kvm_has_msr_hsave_pa(env)) > msrs[n++].index = MSR_VM_HSAVE_PA; > - msrs[n++].index = MSR_IA32_TSC; > + > + if (!env->tsc_valid) { > + msrs[n++].index = MSR_IA32_TSC; > + env->tsc_valid = !vm_running; > + } > + > #ifdef TARGET_X86_64 > if (lm_capable_kernel) { > msrs[n++].index = MSR_CSTAR; Yep. Reviewed-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx> -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html