From mboxrd@z Thu Jan  1 00:00:00 1970
From: Roman Kagan <rkagan@virtuozzo.com>
Subject: Re: [PATCH kvm-unit-tests] KVM: x86: add hyperv clock test case
Date: Thu, 26 May 2016 17:47:06 +0300
Message-ID: <20160526144706.GA28436@rkaganb.sw.ru>
References: <56BB41CB.8010106@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Cc: <kvm@vger.kernel.org>, "Denis V. Lunev" <den@openvz.org>,
	Marcelo Tosatti <mtosatti@redhat.com>,
	Owen Hofmann <osh@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-db3on0144.outbound.protection.outlook.com ([157.55.234.144]:29755
	"EHLO emea01-db3-obe.outbound.protection.outlook.com"
	rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
	id S1753380AbcEZOrV (ORCPT <rfc822;kvm@vger.kernel.org>);
	Thu, 26 May 2016 10:47:21 -0400
Content-Disposition: inline
In-Reply-To: <56BB41CB.8010106@redhat.com>
 <20160525183306.GB18943@rkaganb.sw.ru>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Wed, May 25, 2016 at 09:33:07PM +0300, Roman Kagan wrote:
> On Tue, Apr 26, 2016 at 01:34:56PM +0300, Roman Kagan wrote:
> > On Mon, Apr 25, 2016 at 11:47:23AM +0300, Roman Kagan wrote:
> > > AFAICT the root cause is the following: KVM master clock uses the same
> > > multiplier/shift as the vsyscall time in host userspace.  However, the
> > > offsets in vsyscall_gtod_data get updated all the time with corrections
> > > from NTP and so on.  Therefore even if the TSC rate is somewhat
> > > miscalibrated, the error is kept small in vsyscall time functions.  OTOH
> > > the offsets in KVM clock are basically never updated, so the error keeps
> > > linearly growing over time.
> > 
> > This seems to be due to a typo:
> > 
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5819,7 +5819,7 @@ static int pvclock_gtod_notify(struct notifier_block *nb, unsigned long unused,
> >         /* disable master clock if host does not trust, or does not
> >          * use, TSC clocksource
> >          */
> > -       if (gtod->clock.vclock_mode != VCLOCK_TSC &&
> > +       if (gtod->clock.vclock_mode == VCLOCK_TSC &&
> >             atomic_read(&kvm_guest_has_master_clock) != 0)
> >                 queue_work(system_long_wq, &pvclock_gtod_work);
> > 
> > 
> > as a result, the global pvclock_gtod_data was kept up to date, but the
> > requests to update per-vm copies were never issued.

Looks like it has already been noticed and acknowledged as a bug during
review, but made it into the upstream tree nonetheless:

On Wed, Feb 10, 2016 at 02:57:31PM +0100, Paolo Bonzini wrote:
> On 09/02/2016 19:41, Owen Hofmann wrote:
> > Should this patch change the condition in pvclock_gtod_notify?
> > Currently it looks like we'll only request a masterclock update when
> > tsc is no longer a good clocksource.
> 
> Yes, you're right.
> 
> Paolo

I'm gonna post a patch to fix it.

Roman.