Re: [KVM timekeeping fixes 4/4] TSC catchup mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Sep 20, 2010 at 03:11:30PM -1000, Zachary Amsden wrote:
> On 09/20/2010 05:38 AM, Marcelo Tosatti wrote:
> >On Sat, Sep 18, 2010 at 02:38:15PM -1000, Zachary Amsden wrote:
> >>Negate the effects of AN TYM spell while kvm thread is preempted by tracking
> >>conversion factor to the highest TSC rate and catching the TSC up when it has
> >>fallen behind the kernel view of time.  Note that once triggered, we don't
> >>turn off catchup mode.
> >>
> >>A slightly more clever version of this is possible, which only does catchup
> >>when TSC rate drops, and which specifically targets only CPUs with broken
> >>TSC, but since these all are considered unstable_tsc(), this patch covers
> >>all necessary cases.
> >>
> >>Signed-off-by: Zachary Amsden<zamsden@xxxxxxxxxx>
> >>---
> >>  arch/x86/include/asm/kvm_host.h |    6 +++
> >>  arch/x86/kvm/x86.c              |   87 +++++++++++++++++++++++++++++---------
> >>  2 files changed, 72 insertions(+), 21 deletions(-)
> >>
> >>diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> >>index 8c5779d..e209078 100644
> >>--- a/arch/x86/include/asm/kvm_host.h
> >>+++ b/arch/x86/include/asm/kvm_host.h
> >>@@ -384,6 +384,9 @@ struct kvm_vcpu_arch {
> >>  	u64 last_host_tsc;
> >>  	u64 last_guest_tsc;
> >>  	u64 last_kernel_ns;
> >>+	u64 last_tsc_nsec;
> >>+	u64 last_tsc_write;
> >>+	bool tsc_catchup;
> >>
> >>  	bool nmi_pending;
> >>  	bool nmi_injected;
> >>@@ -444,6 +447,9 @@ struct kvm_arch {
> >>  	u64 last_tsc_nsec;
> >>  	u64 last_tsc_offset;
> >>  	u64 last_tsc_write;
> >>+	u32 virtual_tsc_khz;
> >>+	u32 virtual_tsc_mult;
> >>+	s8 virtual_tsc_shift;
> >>
> >>  	struct kvm_xen_hvm_config xen_hvm_config;
> >>
> >>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> >>index 09f468a..9152156 100644
> >>--- a/arch/x86/kvm/x86.c
> >>+++ b/arch/x86/kvm/x86.c
> >>@@ -962,6 +962,7 @@ static inline u64 get_kernel_ns(void)
> >>  }
> >>
> >>  static DEFINE_PER_CPU(unsigned long, cpu_tsc_khz);
> >>+unsigned long max_tsc_khz;
> >>
> >>  static inline int kvm_tsc_changes_freq(void)
> >>  {
> >>@@ -985,6 +986,24 @@ static inline u64 nsec_to_cycles(u64 nsec)
> >>  	return ret;
> >>  }
> >>
> >>+static void kvm_arch_set_tsc_khz(struct kvm *kvm, u32 this_tsc_khz)
> >>+{
> >>+	/* Compute a scale to convert nanoseconds in TSC cycles */
> >>+	kvm_get_time_scale(this_tsc_khz, NSEC_PER_SEC / 1000,
> >>+			&kvm->arch.virtual_tsc_shift,
> >>+			&kvm->arch.virtual_tsc_mult);
> >>+	kvm->arch.virtual_tsc_khz = this_tsc_khz;
> >>+}
> >>+
> >>+static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns)
> >>+{
> >>+	u64 tsc = pvclock_scale_delta(kernel_ns-vcpu->arch.last_tsc_nsec,
> >>+				      vcpu->kvm->arch.virtual_tsc_mult,
> >>+				      vcpu->kvm->arch.virtual_tsc_shift);
> >>+	tsc += vcpu->arch.last_tsc_write;
> >>+	return tsc;
> >>+}
> >>+
> >>  void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data)
> >>  {
> >>  	struct kvm *kvm = vcpu->kvm;
> >>@@ -1029,6 +1048,8 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data)
> >>
> >>  	/* Reset of TSC must disable overshoot protection below */
> >>  	vcpu->arch.hv_clock.tsc_timestamp = 0;
> >>+	vcpu->arch.last_tsc_write = data;
> >>+	vcpu->arch.last_tsc_nsec = ns;
> >>  }
> >>  EXPORT_SYMBOL_GPL(kvm_write_tsc);
> >>
> >>@@ -1041,22 +1062,42 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
> >>  	s64 kernel_ns, max_kernel_ns;
> >>  	u64 tsc_timestamp;
> >>
> >>-	if ((!vcpu->time_page))
> >>-		return 0;
> >>-
> >>  	/* Keep irq disabled to prevent changes to the clock */
> >>  	local_irq_save(flags);
> >>  	kvm_get_msr(v, MSR_IA32_TSC,&tsc_timestamp);
> >>  	kernel_ns = get_kernel_ns();
> >>  	this_tsc_khz = __get_cpu_var(cpu_tsc_khz);
> >>-	local_irq_restore(flags);
> >>
> >>  	if (unlikely(this_tsc_khz == 0)) {
> >>+		local_irq_restore(flags);
> >>  		kvm_make_request(KVM_REQ_CLOCK_UPDATE, v);
> >>  		return 1;
> >>  	}
> >>
> >>  	/*
> >>+	 * We may have to catch up the TSC to match elapsed wall clock
> >>+	 * time for two reasons, even if kvmclock is used.
> >>+	 *   1) CPU could have been running below the maximum TSC rate
> >kvmclock handles frequency changes?
> >
> >>+	 *   2) Broken TSC compensation resets the base at each VCPU
> >>+	 *      entry to avoid unknown leaps of TSC even when running
> >>+	 *      again on the same CPU.  This may cause apparent elapsed
> >>+	 *      time to disappear, and the guest to stand still or run
> >>+	 *	very slowly.
> >I don't get this. Please explain.
> 
> This compensation in arch_vcpu_load, for unstable TSC case, causes
> time while preempted to disappear from the TSC by adjusting the TSC
> back to match the last observed TSC.
> 
>         if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) {
>                 /* Make sure TSC doesn't go backwards */
>                 s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
>                                 native_read_tsc() -
> vcpu->arch.last_host_tsc;
>                 if (tsc_delta < 0)
>                         mark_tsc_unstable("KVM discovered backwards TSC");
>                 if (check_tsc_unstable())
>                         kvm_x86_ops->adjust_tsc_offset(vcpu,
> -tsc_delta); <<<<<
> 
> Note that this is the correct thing to do if there are cross-CPU
> deltas, when switching CPUs, or if the TSC becomes inherently
> unpredictable while preempted (CPU bugs, kernel resets TSC).
> 
> However, all the time that elapsed while not running disappears from
> the TSC (and thus even from kvmclock, without recalibration, as it
> is based off the TSC).  Since we've got to recalibrate the kvmclock
> anyways, we might as well catch the TSC up to the proper value.

Updating kvmclock's tsc_timestamp and system_time should be enough then,
to fix this particular issue?

The problem is you're sneaking in parts of trap mode (virtual_tsc_khz),
without dealing with the issues raised in the past iteration. The
interactions between catch and trap mode are not clear, migration is not
handled, etc.

> And if kvmclock is not in use, we must catch the tsc up to the proper value.
> 
> Zach
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux