From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [PATCH v2] KVM: x86: update masterclock values on TSC writes
Date: Wed, 5 Nov 2014 17:58:30 -0200
Message-ID: <20141105195829.GB608@amt.cnet>
References: <20141104233044.GB29191@amt.cnet>
 <5459EF85.5010206@redhat.com>
 <20141105181928.GA608@amt.cnet>
 <545A7ED1.3050504@redhat.com>
 <545A7F52.8070408@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm-devel <kvm@vger.kernel.org>
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:49579 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1750966AbaKEU63 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Wed, 5 Nov 2014 15:58:29 -0500
Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sA5KwS16028937
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL)
	for <kvm@vger.kernel.org>; Wed, 5 Nov 2014 15:58:28 -0500
Content-Disposition: inline
In-Reply-To: <545A7F52.8070408@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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.