Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934126AbcKQREf (ORCPT ); Thu, 17 Nov 2016 12:04:35 -0500 Received: from mx1.redhat.com ([209.132.183.28]:36726 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S934025AbcKQREb (ORCPT ); Thu, 17 Nov 2016 12:04:31 -0500 Subject: Re: [PATCH v3] KVM: x86: do not go through vcpu in __get_kvmclock_ns To: Marcelo Tosatti References: <20161116173130.24461-1-pbonzini@redhat.com> <20161117113818.GA11473@amt.cnet> Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, rkrcmar@redhat.com From: Paolo Bonzini Message-ID: <13d50b94-f884-e976-158a-508bcbcc6fa1@redhat.com> Date: Thu, 17 Nov 2016 13:14:35 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161117113818.GA11473@amt.cnet> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Thu, 17 Nov 2016 12:14:38 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2370 Lines: 67 On 17/11/2016 12:38, Marcelo Tosatti wrote: > 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? We don't have a vcpu, so we cannot use either kvm_scale_tsc or kvm_read_l1_tsc. But luckily it's not necessary here, because we're using cpu_tsc_khz and rdtsc. Paolo