Il 15/01/2014 13:34, Marcelo Tosatti ha scritto:
> On Wed, Jan 15, 2014 at 12:43:02PM +0100, Paolo Bonzini wrote:
>> Il 06/01/2014 15:18, Marcelo Tosatti ha scritto:
>>>
>>> To fix a problem related to different resolution of TSC and system clock,
>>> the offset in TSC units is approximated by
>>>
>>> delta = vcpu->hv_clock.tsc_timestamp - vcpu->last_guest_tsc
>>>
>>> (Guest TSC value at (Guest TSC value at last VM-exit)
>>> the last kvm_guest_time_update
>>> call)
>>>
>>> Delta is then later scaled using mult,shift pair found in hv_clock
>>> structure (which is correct against tsc_timestamp in that
>>> structure).
>>>
>>> However, if a frequency change is performed between these two points,
>>> this delta is measured using different TSC frequencies, but scaled using
>>> mult,shift pair for one frequency only.
>>>
>>> The end result is an incorrect delta.
>>>
>>> The bug which this code works around is not the only cause for
>>> clock backwards events. The global accumulator is still
>>> necessary, so remove the max_kernel_ns fix and rely on the
>>> global accumulator for no clock backwards events.
>>
>> This is basically reverting commit 1d5f066 (KVM: x86: Fix a possible
>> backwards warp of kvmclock, 2010-08-19).
>>
>> Your commit message basically says that the guest-side 489fb49 (x86,
>> paravirt: Add a global synchronization point for pvclock, 2010-05-11) is
>> the real solution to the problem that the host-side commit 1d5f066 was
>> trying to fix. Right?
>>
>> This patch makes vcpu->hv_clock.tsc_timestamp write only. Please
>> provide a follow up that drops the field entirely, then I'll apply both.
>> In the meanwhile:
>>
>> Reviewed-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
>
> Can't do that: its inside hv_clock structure.
Right. Another question, what about this comment:
/* Reset of TSC must disable overshoot protection below */
vcpu->arch.hv_clock.tsc_timestamp = 0;
vcpu->arch.last_guest_tsc = data;
Should it be instead like this:
vcpu->arch.hv_clock.tsc_timestamp += data - vcpu->arch.last_guest_tsc;
vcpu->arch.last_guest_tsc = data;
?
Paolo
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
