From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: KVM: x86: fix tsc catchup issue with tsc scaling Date: Wed, 15 Jan 2014 17:53:22 +0100 Message-ID: <52D6BD02.5050807@redhat.com> References: <20140106141859.GA27260@amt.cnet> <52D67446.3020403@redhat.com> <20140115123444.GA27345@amt.cnet> <52D68556.8020303@redhat.com> <20140115163758.GA7161@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Cc: kvm-devel , Jeremy Fitzhardinge , Glauber Costa To: Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:4922 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751776AbaAOQxc (ORCPT ); Wed, 15 Jan 2014 11:53:32 -0500 In-Reply-To: <20140115163758.GA7161@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: Il 15/01/2014 17:37, Marcelo Tosatti ha scritto: > > Right. Another question, what about this comment: > > > > /* Reset of TSC must disable overshoot protection below */ > > vcpu->arch.hv_clock.tsc_timestamp = 0; > > vcpu->arch.last_guest_tsc = data; > > > > Should it be instead like this: > > > > vcpu->arch.hv_clock.tsc_timestamp += data - vcpu->arch.last_guest_tsc; > > vcpu->arch.last_guest_tsc = data; > > > > ? > > Don't see why it should? Setting tsc_timestamp to 0 makes no sense with the current code, I'm trying to understand if the right fix is to delete that line or something else. What I proposed (plus a version increase) would make a pvclock read continuous before and after the change. But looking more at the surrounding code, the calls to kvm_write_tsc ultimately result in a master clock update as soon as all TSCs agree and the master clock is re-enabled. This master clock update rewrites tsc_timestamp. Then I think that the line that sets tsc_timestamp to 0 can be deleted. Paolo