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

[Marcelo Tosatti <mtosatti@redhat.com>]

On Wed, Nov 05, 2014 at 08:49:38PM +0100, Paolo Bonzini wrote:
> 
> 
> 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

Looks good, thanks.