Re: RFC: VMX: initialize TSC offset relative to vm creation time

["David S. Ahern" <daahern@cisco.com>]

Marcelo Tosatti wrote:
> On Sat, Sep 13, 2008 at 07:55:02AM +0300, Avi Kivity wrote:
>> Marcelo Tosatti wrote:
>>> VMX initializes the TSC offset for each vcpu at different times, and
>>> also reinitializes it for vcpus other than 0 on APIC SIPI message.
>>>
>>> This bug causes the TSC's to appear unsynchronized in the guest, even if
>>> the host is good.
>>>
>>> Older Linux kernels don't handle the situation very well, so
>>> gettimeofday is likely to go backwards in time:
>>>
>>> http://www.mail-archive.com/kvm@vger.kernel.org/msg02955.html
>>> http://sourceforge.net/tracker/index.php?func=detail&aid=2025534&group_id=180599&atid=893831
>>>
>>> Fix it by initializating the offset of each vcpu relative to vm creation
>>> time, and moving it from vmx_vcpu_reset to vmx_vcpu_setup, out of the
>>> APIC MP init path.
>>>
>>>
>>>   
>> This is good in principle, but we need to detect if we're on a multiple
>> board host (or a host with unsynced tscs) and do something else in that
>> case.
> 
> I think this is a separate, and difficult, problem. For instance older
> Linux guests that correct the TSC across CPU's are broken at the moment
> in the unsynced TSC case.
> 
> That is, the fact that KVM does not handle unsynced TSC's on the host is
> not an argument against this patch which clearly fixes a bug.
> 
> Take commit 019960ae9933161c2809fa4ee608ba30d9639fd2 for example.
> 

Has anything changed "recently" with the TSC code? Recently here being
the past 2 months since you first crafted the patch. I ask because in
the past few runs based on kvm.git trees (e.g., as recently as a pull on
10/26), this tsc offset patch no longer fixes the problem.

The following one does fix the problem with kvm.git pulled on 10/26/08:

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 64e2439..d5da717 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -860,7 +860,7 @@ static void guest_write_tsc(u64 guest_tsc)
        u64 host_tsc;

        rdtscll(host_tsc);
-       vmcs_write64(TSC_OFFSET, guest_tsc - host_tsc);
+       vmcs_write64(TSC_OFFSET, 0);
 }

 /*

This is the vmx counterpart (or at least to my understanding) to a
suggestion Ben had for the svm code.

david