From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: KVM: x86: fix kvm_write_tsc() TSC matching thinko
Date: Wed, 14 Mar 2012 22:42:25 -0300
Message-ID: <20120315014225.GA10879@amt.cnet>
References: <20120308214657.GB21223@amt.cnet>
 <4F607AD1.5050605@siemens.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm <kvm@vger.kernel.org>, Avi Kivity <avi@redhat.com>,
	Zachary Amsden <zamsden@gmail.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:44036 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1031222Ab2COBpb (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 14 Mar 2012 21:45:31 -0400
Content-Disposition: inline
In-Reply-To: <4F607AD1.5050605@siemens.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, Mar 14, 2012 at 12:02:41PM +0100, Jan Kiszka wrote:
> On 2012-03-08 22:46, Marcelo Tosatti wrote:
> > 
> > kvm_write_tsc() converts from guest TSC to microseconds, not nanoseconds
> > as intended. The result is that the window for matching is 1000 seconds,
> > not 1 second.
> > 
> > Microsecond precision is enough for checking whether the TSC write delta
> > is within the heuristic values, so use it instead of nanoseconds.
> > 
> > Noted by Avi Kivity.
> > 
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 32096cf..9732e98 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -1025,7 +1025,7 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data)
> >  	struct kvm *kvm = vcpu->kvm;
> >  	u64 offset, ns, elapsed;
> >  	unsigned long flags;
> > -	s64 nsdiff;
> > +	s64 usdiff;
> >  
> >  	raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags);
> >  	offset = kvm_x86_ops->compute_tsc_offset(vcpu, data);
> > @@ -1033,18 +1033,18 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data)
> >  	elapsed = ns - kvm->arch.last_tsc_nsec;
> >  
> >  	/* n.b - signed multiplication and division required */
> > -	nsdiff = data - kvm->arch.last_tsc_write;
> > +	usdiff = data - kvm->arch.last_tsc_write;
> >  #ifdef CONFIG_X86_64
> > -	nsdiff = (nsdiff * 1000) / vcpu->arch.virtual_tsc_khz;
> > +	usdiff = (usdiff * 1000) / vcpu->arch.virtual_tsc_khz;
> >  #else
> >  	/* do_div() only does unsigned */
> >  	asm("idivl %2; xor %%edx, %%edx"
> > -	    : "=A"(nsdiff)
> > -	    : "A"(nsdiff * 1000), "rm"(vcpu->arch.virtual_tsc_khz));
> > +	    : "=A"(usdiff)
> > +	    : "A"(usdiff * 1000), "rm"(vcpu->arch.virtual_tsc_khz));
> >  #endif
> > -	nsdiff -= elapsed;
> > -	if (nsdiff < 0)
> > -		nsdiff = -nsdiff;
> > +	usdiff -= elapsed / 1000;
> 
> do_div, to please x86-32.
> 
> Jan

Fixed, thanks Jan.