From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: KVM: x86: disable master clock if TSC is reset during suspend Date: Wed, 14 May 2014 11:09:36 +0200 Message-ID: <537332D0.1090505@redhat.com> References: <20140514072638.GA9285@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Alexander Graf To: Marcelo Tosatti , kvm-devel Return-path: Received: from mx1.redhat.com ([209.132.183.28]:51413 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752836AbaENJJo (ORCPT ); Wed, 14 May 2014 05:09:44 -0400 In-Reply-To: <20140514072638.GA9285@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: Il 14/05/2014 09:26, Marcelo Tosatti ha scritto: > > Updating system_time from the kernel clock once master clock > has been enabled can result in time backwards event, in case > kernel clock frequency is lower than TSC frequency. > > Disable master clock in case its necessary to update it > from the resume path. > > Signed-off-by: Marcelo Tosatti > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index de0931c..9f5dd40 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -106,6 +106,8 @@ EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz); > static u32 tsc_tolerance_ppm = 250; > module_param(tsc_tolerance_ppm, uint, S_IRUGO | S_IWUSR); > > +static bool suspend_backwards_tsc = false; > + > #define KVM_NR_SHARED_MSRS 16 > > struct kvm_shared_msrs_global { > @@ -1472,6 +1474,8 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm) > &ka->master_cycle_now); > > ka->use_master_clock = host_tsc_clocksource & vcpus_matched; > + if (suspend_backwards_tsc) > + ka->use_master_clock = 0; Use an && instead of an "if"? (Also, why "&" in the other assignment of ka->use_master_clock)? > if (ka->use_master_clock) > atomic_set(&kvm_guest_has_master_clock, 1); > @@ -6939,6 +6943,7 @@ int kvm_arch_hardware_enable(void *garbage) > */ > if (backwards_tsc) { > u64 delta_cyc = max_tsc - local_tsc; > + suspend_backwards_tsc = true; > list_for_each_entry(kvm, &vm_list, vm_list) { > kvm_for_each_vcpu(i, vcpu, kvm) { > vcpu->arch.tsc_offset_adjustment += delta_cyc; > Why a global and not a variable inside kvm? The same question holds for kvm_guest_has_master_clock. Paolo