From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932534AbcBISlR (ORCPT ); Tue, 9 Feb 2016 13:41:17 -0500 Received: from mail-ig0-f171.google.com ([209.85.213.171]:33473 "EHLO mail-ig0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756036AbcBISlP (ORCPT ); Tue, 9 Feb 2016 13:41:15 -0500 MIME-Version: 1.0 In-Reply-To: <1454944711-33022-5-git-send-email-pbonzini@redhat.com> References: <1454944711-33022-1-git-send-email-pbonzini@redhat.com> <1454944711-33022-5-git-send-email-pbonzini@redhat.com> Date: Tue, 9 Feb 2016 10:41:14 -0800 Message-ID: Subject: Re: [PATCH 4/4] KVM: x86: track actual TSC frequency from the timekeeper struct From: Owen Hofmann To: Paolo Bonzini Cc: LKML , KVM General , Marcelo Tosatti Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Should this patch change the condition in pvclock_gtod_notify? Currently it looks like we'll only request a masterclock update when tsc is no longer a good clocksource. On Mon, Feb 8, 2016 at 7:18 AM, Paolo Bonzini wrote: > When an NTP server is running, it may adjust the time substantially > compared to the "official" frequency of the TSC. A 12 ppm change > sums up to one second per day. > > This already shows up if the guest compares kvmclock with e.g. the > PM timer. It shows up even more once we add support for the Hyper-V > TSC page, because the guest expects it to be in sync with the time > reference counter; effectively the time reference counter is just a > slow path to access the same clock that is in the TSC page. > > Therefore, we want kvmclock to provide the host kernel's > ktime_get_boot_ns() value, at least if the master clock is active. > To do so, reverse-compute the host's "actual" TSC frequency from > pvclock_gtod_data and return it from kvm_get_time_and_clockread. > > Signed-off-by: Paolo Bonzini > --- > arch/x86/include/asm/kvm_host.h | 3 ++- > arch/x86/kvm/x86.c | 46 +++++++++++++++++++++++++++-------------- > 2 files changed, 33 insertions(+), 16 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 7b5459982433..7dd6d55b4868 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -538,7 +538,7 @@ struct kvm_vcpu_arch { > > gpa_t time; > struct pvclock_vcpu_time_info hv_clock; > - unsigned int hw_tsc_khz; > + u64 hv_clock_tsc_hz; > struct gfn_to_hva_cache pv_time; > bool pv_time_enabled; > /* set guest stopped flag in pvclock flags field */ > @@ -731,6 +731,7 @@ struct kvm_arch { > > spinlock_t pvclock_gtod_sync_lock; > bool use_master_clock; > + u64 master_tsc_hz; > u64 master_kernel_ns; > cycle_t master_cycle_now; > struct delayed_work kvmclock_update_work; > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index e0bf8f10dc22..c99101d4cc5e 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -1580,7 +1580,16 @@ static inline u64 vgettsc(cycle_t *cycle_now) > return v * gtod->clock.mult; > } > > -static int do_monotonic_boot(s64 *t, cycle_t *cycle_now) > +static u64 pvclock_gtod_tsc_hz(void) > +{ > + struct pvclock_gtod_data *gtod = &pvclock_gtod_data; > + > + u64 gtod_tsc_hz = 1000000000ULL << gtod->clock.shift; > + do_div(gtod_tsc_hz, gtod->clock.mult); > + return gtod_tsc_hz; > +} > + > +static int do_monotonic_boot(s64 *t, cycle_t *cycle_now, u64 *tsc_hz) > { > struct pvclock_gtod_data *gtod = &pvclock_gtod_data; > unsigned long seq; > @@ -1589,6 +1598,7 @@ static int do_monotonic_boot(s64 *t, cycle_t *cycle_now) > > do { > seq = read_seqcount_begin(>od->seq); > + *tsc_hz = pvclock_gtod_tsc_hz(); > mode = gtod->clock.vclock_mode; > ns = gtod->nsec_base; > ns += vgettsc(cycle_now); > @@ -1601,13 +1611,14 @@ static int do_monotonic_boot(s64 *t, cycle_t *cycle_now) > } > > /* returns true if host is using tsc clocksource */ > -static bool kvm_get_time_and_clockread(s64 *kernel_ns, cycle_t *cycle_now) > +static bool kvm_get_time_and_clockread(s64 *kernel_ns, cycle_t *cycle_now, > + u64 *tsc_hz) > { > /* checked again under seqlock below */ > if (pvclock_gtod_data.clock.vclock_mode != VCLOCK_TSC) > return false; > > - return do_monotonic_boot(kernel_ns, cycle_now) == VCLOCK_TSC; > + return do_monotonic_boot(kernel_ns, cycle_now, tsc_hz) == VCLOCK_TSC; > } > #endif > > @@ -1668,7 +1679,8 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm) > */ > host_tsc_clocksource = kvm_get_time_and_clockread( > &ka->master_kernel_ns, > - &ka->master_cycle_now); > + &ka->master_cycle_now, > + &ka->master_tsc_hz); > > ka->use_master_clock = host_tsc_clocksource && vcpus_matched > && !backwards_tsc_observed > @@ -1713,7 +1725,8 @@ static void kvm_gen_update_masterclock(struct kvm *kvm) > > static int kvm_guest_time_update(struct kvm_vcpu *v) > { > - unsigned long flags, tgt_tsc_khz; > + unsigned long flags; > + u64 tgt_tsc_hz; > struct kvm_vcpu_arch *vcpu = &v->arch; > struct kvm_arch *ka = &v->kvm->arch; > s64 kernel_ns; > @@ -1724,6 +1737,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > > kernel_ns = 0; > host_tsc = 0; > + tgt_tsc_hz = 0; > > /* > * If the host uses TSC clock, then passthrough TSC as stable > @@ -1734,20 +1748,22 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > if (use_master_clock) { > host_tsc = ka->master_cycle_now; > kernel_ns = ka->master_kernel_ns; > + tgt_tsc_hz = ka->master_tsc_hz; > } > spin_unlock(&ka->pvclock_gtod_sync_lock); > > /* Keep irq disabled to prevent changes to the clock */ > local_irq_save(flags); > - tgt_tsc_khz = __this_cpu_read(cpu_tsc_khz); > - if (unlikely(tgt_tsc_khz == 0)) { > - local_irq_restore(flags); > - kvm_make_request(KVM_REQ_CLOCK_UPDATE, v); > - return 1; > - } > if (!use_master_clock) { > host_tsc = rdtsc(); > kernel_ns = get_kernel_ns(); > + tgt_tsc_hz = __this_cpu_read(cpu_tsc_khz) * 1000; > + } > + > + if (unlikely(tgt_tsc_hz == 0)) { > + local_irq_restore(flags); > + kvm_make_request(KVM_REQ_CLOCK_UPDATE, v); > + return 1; > } > > tsc_timestamp = kvm_read_l1_tsc(v, host_tsc); > @@ -1776,13 +1792,13 @@ static int kvm_guest_time_update(struct kvm_vcpu *v) > return 0; > > if (kvm_has_tsc_control) > - tgt_tsc_khz = kvm_scale_tsc(v, tgt_tsc_khz); > + tgt_tsc_hz = kvm_scale_tsc(v, tgt_tsc_hz); > > - if (unlikely(vcpu->hw_tsc_khz != tgt_tsc_khz)) { > - kvm_get_time_scale(NSEC_PER_SEC, tgt_tsc_khz * 1000LL, > + if (unlikely(vcpu->hv_clock_tsc_hz != tgt_tsc_hz)) { > + kvm_get_time_scale(NSEC_PER_SEC, tgt_tsc_hz, > &vcpu->hv_clock.tsc_shift, > &vcpu->hv_clock.tsc_to_system_mul); > - vcpu->hw_tsc_khz = tgt_tsc_khz; > + vcpu->hv_clock_tsc_hz = tgt_tsc_hz; > } > > /* With all the info we got, fill in the values */ > -- > 1.8.3.1 > > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html