Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935238AbcKQLkx (ORCPT ); Thu, 17 Nov 2016 06:40:53 -0500 Received: from mx1.redhat.com ([209.132.183.28]:45346 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752136AbcKQLku (ORCPT ); Thu, 17 Nov 2016 06:40:50 -0500 Date: Thu, 17 Nov 2016 09:38:19 -0200 From: Marcelo Tosatti To: Paolo Bonzini Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, rkrcmar@redhat.com Subject: Re: [PATCH v3] KVM: x86: do not go through vcpu in __get_kvmclock_ns Message-ID: <20161117113818.GA11473@amt.cnet> References: <20161116173130.24461-1-pbonzini@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161116173130.24461-1-pbonzini@redhat.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 17 Nov 2016 11:40:49 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2090 Lines: 58 On Wed, Nov 16, 2016 at 06:31:30PM +0100, Paolo Bonzini wrote: > Going through the first VCPU is wrong if you follow a KVM_SET_CLOCK with > a KVM_GET_CLOCK immediately after, without letting the VCPU run and > call kvm_guest_time_update. > > To fix this, compute the kvmclock value ourselves, using the master > clock (tsc, nsec) pair as the base and the host CPU frequency as > the scale. > > Reported-by: Marcelo Tosatti > Signed-off-by: Paolo Bonzini > --- > arch/x86/kvm/x86.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 1ba08278a9a9..bd138a79404a 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1724,18 +1724,23 @@ static void kvm_gen_update_masterclock(struct kvm *kvm) > > static u64 __get_kvmclock_ns(struct kvm *kvm) > { > - struct kvm_vcpu *vcpu = kvm_get_vcpu(kvm, 0); > struct kvm_arch *ka = &kvm->arch; > - s64 ns; > + struct pvclock_vcpu_time_info hv_clock; > > - if (vcpu->arch.hv_clock.flags & PVCLOCK_TSC_STABLE_BIT) { > - u64 tsc = kvm_read_l1_tsc(vcpu, rdtsc()); > - ns = __pvclock_read_cycles(&vcpu->arch.hv_clock, tsc); > - } else { > - ns = ktime_get_boot_ns() + ka->kvmclock_offset; > + spin_lock(&ka->pvclock_gtod_sync_lock); > + if (!ka->use_master_clock) { > + spin_unlock(&ka->pvclock_gtod_sync_lock); > + return ktime_get_boot_ns() + ka->kvmclock_offset; > } > > - return ns; > + hv_clock.tsc_timestamp = ka->master_cycle_now; > + hv_clock.system_time = ka->master_kernel_ns + ka->kvmclock_offset; > + spin_unlock(&ka->pvclock_gtod_sync_lock); > + > + kvm_get_time_scale(NSEC_PER_SEC, __this_cpu_read(cpu_tsc_khz) * 1000LL, > + &hv_clock.tsc_shift, > + &hv_clock.tsc_to_system_mul); > + return __pvclock_read_cycles(&hv_clock, rdtsc()); > } Missing TSC scaling? /* With all the info we got, fill in the values */ if (kvm_has_tsc_control) tgt_tsc_khz = kvm_scale_tsc(v, tgt_tsc_khz); Should use kvm_read_l1_tsc to convert as well?