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

Marcelo Tosatti
2014-11-05T20:58:29Z

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.