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

[Paolo Bonzini <pbonzini@redhat.com>]

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