Re: [PATCH 0/2] Fix build on i386 due to the latest tsc changes

[Marcelo Tosatti <mtosatti@redhat.com>]

On Thu, Feb 16, 2012 at 03:53:40PM +0200, Avi Kivity wrote:
> On 02/16/2012 03:48 PM, Avi Kivity wrote:
> > The code fixed by the second patch looks suspect though:
> >
> >   nsdiff = data - kvm->arch.last_tsc_write;
> >   nsdiff = (nsdiff * 1000) / vcpu->arch.virtual_tsc_khz;
> >
> > before the division, nsdiff is in tsc units.  Dividing it by
> > tsc_khz/1000 is equivalent to multiplying it by 1000000 and dividing it by
> > tsc_hz, so the result is in units of mega-seconds.  I expect we want
> >
> 
> Actually it results in units of microseconds, while we want nanoseconds.
> 
> So maybe the correct code is
> 
>   nsdiff = (nsdiff * 1000000) / vcpu->arch.virtual_tsc_khz;
> 
> returning nanoseconds.
> 
> I note that if the guest writes a large value into the tsc, this breaks.

You are right, the division is wrong. I'll fix it and run some tests
later. Nice catch.