Re: [PATCH 09/18] Robust TSC compensation

Zachary Amsden
2010-07-13T21:15:39Z

On 07/13/2010 10:34 AM, Marcelo Tosatti wrote:
> On Mon, Jul 12, 2010 at 04:25:29PM -1000, Zachary Amsden wrote:
>
>> Make the match of TSC find TSC writes that are close to each other
>> instead of perfectly identical; this allows the compensator to also
>> work in migration / suspend scenarios.
>>
>> Signed-off-by: Zachary Amsden<zamsden@redhat.com>
>> ---
>> arch/x86/kvm/x86.c | 14 ++++++++++----
>> 1 files changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>> index 79c4608..51d3f3e 100644
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -926,21 +926,27 @@ void guest_write_tsc(struct kvm_vcpu *vcpu, u64 data)
>> struct kvm *kvm = vcpu->kvm;
>> u64 offset, ns, elapsed;
>> struct timespec ts;
>> + s64 sdiff;
>>
>> spin_lock(&kvm->arch.tsc_write_lock);
>> offset = data - native_read_tsc();
>> ns = get_kernel_ns();
>> elapsed = ns - kvm->arch.last_tsc_nsec;
>> + sdiff = data - kvm->arch.last_tsc_write;
>> + if (sdiff< 0)
>> + sdiff = -sdiff;
>>
>> /*
>> - * Special case: identical write to TSC within 5 seconds of
>> + * Special case: close write to TSC within 5 seconds of
>> * another CPU is interpreted as an attempt to synchronize
>> - * (the 5 seconds is to accomodate host load / swapping).
>> + * The 5 seconds is to accomodate host load / swapping as
>> + * well as any reset of TSC during the boot process.
>> *
>> * In that case, for a reliable TSC, we can match TSC offsets,
>> - * or make a best guest using kernel_ns value.
>> + * or make a best guest using elapsed value.
>> */
>> - if (data == kvm->arch.last_tsc_write&& elapsed< 5ULL * NSEC_PER_SEC) {
>> + if (sdiff< nsec_to_cycles(5ULL * NSEC_PER_SEC)&&
>> + elapsed< 5ULL * NSEC_PER_SEC) {
>> if (!check_tsc_unstable()) {
>> offset = kvm->arch.last_tsc_offset;
>> pr_debug("kvm: matched tsc offset for %llu\n", data);
>>
> What prevents a vcpu from seeing its TSC go backwards, in case the first
> write in the 5 second window is smaller than the victim vcpu's last
> visible TSC value ?
>

Nothing, unfortunately. However, the TSC would already have to be out
of sync in order for the problem to occur. It can never happen in
normal circumstances on a stable hardware TSC except in one case;
migration. During the CPU state transfer phase of migration, however,
all the VCPUs should already be stopped, so the maximum TSC that can be
observed by any CPU is bounded.

The problem, of course is that the TSC write will latch the first TSC to
be written, which, if you stop in order, and start in order, will be the
lowest TSC; so later VCPUs can observe a negative TSC drift (timing is
additionally complicated by the VCPU teardown / create time).

There are a couple solutions; some of which are ugly and some of which
are very ugly.

1) Make some global state available about whether the VM is running or
not, use the global TSC lock and record the last TSC when the VM is
stopped. Return this TSC from any reads when the VM is stopped. I'm
not sure the kernel model is equipped to do this properly; in theory,
userspace could stop and start running VCPUs using the ioctls whenever
it feels like and requires no protocol to do so.

2) Make userspace deal with it; when starting up a VM, read the VCPU
state for all VCPUs in first, then take the maximum TSC and set all TSCs
to this value before starting the VCPUs. I'm not sure the userspace
model is equipped to do this properly, it could start running earlier
CPUs before reading later CPU states...

3) Drop passthrough TSC altogether and switch to trap / emulate TSC.

4) Pray to a deity of your choice.

Of course, none of these solutions work for a guest which deliberately
runs with desynchronized TSCs, but we needn't really be concerned with
that, no guest does it on purpose.