From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: KVM: x86: fix tsc catchup issue with tsc scaling Date: Wed, 15 Jan 2014 10:34:44 -0200 Message-ID: <20140115123444.GA27345@amt.cnet> References: <20140106141859.GA27260@amt.cnet> <52D67446.3020403@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm-devel To: Paolo Bonzini Return-path: Received: from mx1.redhat.com ([209.132.183.28]:11885 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751451AbaAOMld (ORCPT ); Wed, 15 Jan 2014 07:41:33 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s0FCfWJa003570 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 15 Jan 2014 07:41:33 -0500 Content-Disposition: inline In-Reply-To: <52D67446.3020403@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Jan 15, 2014 at 12:43:02PM +0100, Paolo Bonzini wrote: > Il 06/01/2014 15:18, Marcelo Tosatti ha scritto: > > > > To fix a problem related to different resolution of TSC and system clock, > > the offset in TSC units is approximated by > > > > delta = vcpu->hv_clock.tsc_timestamp - vcpu->last_guest_tsc > > > > (Guest TSC value at (Guest TSC value at last VM-exit) > > the last kvm_guest_time_update > > call) > > > > Delta is then later scaled using mult,shift pair found in hv_clock > > structure (which is correct against tsc_timestamp in that > > structure). > > > > However, if a frequency change is performed between these two points, > > this delta is measured using different TSC frequencies, but scaled using > > mult,shift pair for one frequency only. > > > > The end result is an incorrect delta. > > > > The bug which this code works around is not the only cause for > > clock backwards events. The global accumulator is still > > necessary, so remove the max_kernel_ns fix and rely on the > > global accumulator for no clock backwards events. > > This is basically reverting commit 1d5f066 (KVM: x86: Fix a possible > backwards warp of kvmclock, 2010-08-19). > > Your commit message basically says that the guest-side 489fb49 (x86, > paravirt: Add a global synchronization point for pvclock, 2010-05-11) is > the real solution to the problem that the host-side commit 1d5f066 was > trying to fix. Right? > > This patch makes vcpu->hv_clock.tsc_timestamp write only. Please > provide a follow up that drops the field entirely, then I'll apply both. > In the meanwhile: > > Reviewed-by: Paolo Bonzini Can't do that: its inside hv_clock structure.