Re: [PATCH kvm-unit-tests] KVM: x86: add hyperv clock test case

[Roman Kagan <rkagan@virtuozzo.com>]

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.