Re: Bug in KVM clock backwards compensation

[Joerg Roedel <joro@8bytes.org>]

On Thu, Apr 28, 2011 at 11:34:44AM -0700, Zachary Amsden wrote:
> On 04/28/2011 12:13 AM, Roedel, Joerg wrote:

>> I see it different. This code wants to check if the _guest_ tsc moves
>> forwared (or at least not backwards). So it is fully legitimate to just
>> do this by reading the guest-tsc and compare it to the last one the
>> guest had.
>
> That wasn't the intention when I wrote that code.  It's simply there to  
> detect backwards motion of the host TSC.  The guest TSC can legally go  
> backwards whenever the guest decides to change it, so checking the guest  
> TSC doesn't make sense here.

This code checks how many guest tsc cycles have passed since this vCPU
was de-scheduled last time (and before it is running again). So since
the vCPU hasn't run in the meantime it had no chance to change its TSC.
Further, the other parameters like the TSC offset and the scaling
multiplier havn't changed too, so the only variable in the guest-tsc
calculation is the host-tsc.
So this calculation using the guest-tsc can detect backwards going
host-tsc as good as the old one. The benefit here is that we can feed
consistent values into adjust_tsc_offset().

> Yes, with tsc-scaling, the machines already have stable TSCs - the above  
> test is for older hardware which could have problems, and can be  
> reverted back to the original code without worrying about switching 
> units.

This is the case pratically. But architecturally the tsc-scaling feature
does not depend on a constant tsc, so we can make no such assumtions.
Additionally, it may happen that Linux mis-detects an unstable tsc for
some reason (broken BIOS, bug in the code, ...).  Therefore I think it
is dangerous to assume that this code will never run on tsc-scaling
capable hosts. And if it does and we don't manage the tsc-offset units
right, we may see very weird behavior.

Regards,

	Joerg