From mboxrd@z Thu Jan 1 00:00:00 1970 From: Zachary Amsden Subject: Re: Bug in KVM clock backwards compensation Date: Thu, 28 Apr 2011 10:48:50 -0700 Message-ID: <4DB9A882.90000@redhat.com> References: <4DB9106D.6040203@redhat.com> <4DB911D9.3010409@web.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm , Joerg Roedel To: Jan Kiszka Return-path: Received: from mx1.redhat.com ([209.132.183.28]:50787 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756266Ab1D1Rs7 (ORCPT ); Thu, 28 Apr 2011 13:48:59 -0400 In-Reply-To: <4DB911D9.3010409@web.de> Sender: kvm-owner@vger.kernel.org List-ID: On 04/28/2011 12:06 AM, Jan Kiszka wrote: > On 2011-04-28 08:59, Zachary Amsden wrote: > >> So I've been going over the new code changes to the TSC related code and >> I don't like one particular set of changes. In particular, here: >> >> kvm_x86_ops->vcpu_load(vcpu, cpu); >> if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) { >> /* Make sure TSC doesn't go backwards */ >> s64 tsc_delta; >> u64 tsc; >> >> kvm_get_msr(vcpu, MSR_IA32_TSC,&tsc); >> tsc_delta = !vcpu->arch.last_guest_tsc ? 0 : >> tsc - vcpu->arch.last_guest_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); >> vcpu->arch.tsc_catchup = 1; >> } >> >> >> The point of this code fragment is to test the host clock to see if it >> is stable, because we may have just come back from an idle phase which >> stopped the TSC, switched CPUs, or come back from a deep sleep state >> which reset the host TSC. >> >> However, the above code is fetching the guest TSC instead of the host >> TSC, which isn't the way it is supposed to work. >> >> I saw a patch floating around that touched this code recently, but I >> think there's a definite issue here that needs addressing. >> > And /me still wonders (like I did when this first popped up) if the > proper place of determining TSC stability really have to be KVM. > No, it's not. I have a patch which fixes that. Was in the process of merging it into my local tree when I found this. > If the Linux core fails to detect some instability and KVM has to jump > in, shouldn't we better improve the core's detection abilities and make > use of them in KVM? Conceptually this looks like we are currently just > working around a core deficit in KVM. > 100% correct you are. Zach