From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marcelo Tosatti <mtosatti@redhat.com>
Subject: Re: [qemu patch 2/2] kvmclock: reduce kvmclock difference on
 migration
Date: Mon, 14 Nov 2016 15:13:21 -0200
Message-ID: <20161114171318.GA6336@amt.cnet>
References: <20161114123628.703911091@redhat.com>
 <20161114123700.158592605@redhat.com>
 <a3e00c38-4d4d-d353-231d-9eb0233139e2@redhat.com>
 <20161114140028.GA25935@amt.cnet>
 <62d634ab-70ad-4be7-1622-f2e3a9d865fe@redhat.com>
 <20161114145054.GA28663@amt.cnet>
 <67bffd95-2e4e-7273-c154-a3fdfe622387@redhat.com>
 <20161114154015.GA30048@amt.cnet>
 <cbcac3d2-88aa-3cb4-7257-e70061a22a2c@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: kvm@vger.kernel.org, qemu-devel@nongnu.org,
        "Dr. David Alan Gilbert" <dgilbert@redhat.com>,
        Juan Quintela <quintela@redhat.com>,
        Radim Krcmar <rkrcmar@redhat.com>,
        Eduardo Habkost <ehabkost@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:42348 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S933841AbcKNRNo (ORCPT <rfc822;kvm@vger.kernel.org>);
        Mon, 14 Nov 2016 12:13:44 -0500
Content-Disposition: inline
In-Reply-To: <cbcac3d2-88aa-3cb4-7257-e70061a22a2c@redhat.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Nov 14, 2016 at 05:43:33PM +0100, Paolo Bonzini wrote:
> 
> 
> On 14/11/2016 16:40, Marcelo Tosatti wrote:
> > static bool kvmclock_src_use_reliable_get_clock(void *opaque)
> > {
> >     KVMClockState *s = opaque;
> > 
> >     /*
> >      * On machine types that support reliable KVM_GET_CLOCK,
> >      * if host kernel does provide reliable KVM_GET_CLOCK,
> >      * set src_use_reliable_get_clock=true so that destination
> >      * avoids reading kvmclock from memory.
> >      */
> >     if (s->mach_use_reliable_get_clock && kvm_has_adjust_clock_stable())
> >     {
> >         s->src_use_reliable_get_clock = true;
> >     }
> > 
> >     return s->mach_use_reliable_get_clock;
> > }
> > 
> > 
> > Ah, OK, done.
> 
> s->src_use_reliable_get_clock should not be set with
> KVM_CHECK_EXTENSION, but rather from the flags returned by KVM_GET_CLOCK.

Well, thats not right: What matters is the presence of get_kvmclock_ns 
which returns a value that the guest sees. 

                get_kernel_monotonic_clock() + kvmclock_offset +
                (rdtsc() - tsc_timestamp)

IOW what the guest sees. And you changed that in 

commit 108b249c453dd7132599ab6dc7e435a7036c193f
Author: Paolo Bonzini <pbonzini@redhat.com>
Date:   Thu Sep 1 14:21:03 2016 +0200

    KVM: x86: introduce get_kvmclock_ns

And the correct behaviour (once KVM_GET_CLOCK is fixed per 
previous message to return rdtsc - tsc_timestamp for the 
non masterclock case) depends on this commit above, 
not on masterclock.

> > So s->src_use_reliable_get_clock is only used to indicate 
> > to the destination that: "you can use KVM_GET_CLOCK value, 
> > its safe".
> 
> Yes, we agree.  I was listing all the points, not just those where we
> disagree.  Actually I'm not sure where we disagree, except on using
> flags from KVM_CHECK_EXTENSION vs. flags from KVM_GET_CLOCK...
> 
> Paolo