Re: [PATCH v2] KVM: x86: update masterclock values on TSC writes

Paolo Bonzini
2014-11-05T19:49:42Z

On 05/11/2014 20:47, Paolo Bonzini wrote:
>
>
> On 05/11/2014 19:19, Marcelo Tosatti wrote:
>>>>>> + * If the vcpus have matched TSCs and host clocksource is TSC,
>>>>>> + * perform request to enable masterclock.
>>>>>> + *
>>>>>> + * If the masterclock is enabled, perform request to update
>>>>>> + * masterclock values.
>>>>>> + */
>>>>>> + if ((vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC) ||
>>>>>> + ka->use_master_clock)
>>>>
>>>> This is not an explanation, it is a literal translation from C to
>>>> English. :) Can you also explain the why, especially for the first half
>>>> of the condition?
>>>>
>>>> Paolo
>> The comment on top of pvclock_update_vm_gtod_copy says:
>>
>> "Rely on synchronization of host TSCs and guest TSCs for monotonicity."
>>
>> Then with this patch
>>
>> "If the vcpus have matched TSCs and host clocksource is TSC"
>>
>> Is it sufficient to add
>>
>> "Masterclock requires synchronized guest TSC and host clocksource TSC"
>> to this patch?
>
> Got it now, thanks. I think I misread one "enable" as "update" in your
> comment, sorry. I'll apply v2 tomorrow.

Slightly reworded:

/*
* Once the masterclock is enabled, always perform request in
* order to update it.
*
* Until then, if the vcpus have matched TSCs and host clocksource
* is TSC, perform request to enable masterclock.
*/
if (ka->use_master_clock ||
(vcpus_matched && gtod->clock.vclock_mode == VCLOCK_TSC))
kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu);

Paolo