From mboxrd@z Thu Jan  1 00:00:00 1970
From: Roman Kagan <rkagan@virtuozzo.com>
Subject: Re: [PATCH] x86/kvm: fix condition to update kvm master clocks
Date: Tue, 21 Jun 2016 17:40:32 +0300
Message-ID: <20160621144032.pmwc7baeo6247rzq@rkaganb.sw.ru>
References: <1464274195-31296-1-git-send-email-rkagan@virtuozzo.com>
 <20160529233844.GA14374@amt.cnet>
 <20160609032710.GA13318@amt.cnet>
 <20160609120945.GB2570@rkaganb.sw.ru>
 <20160609182501.GA24024@amt.cnet>
 <20160613171929.exgvqiw5hmnza2rp@rkaganb.sw.ru>
 <20160617222119.GA15883@amt.cnet>
 <20160620172249.3mo5aazrehdzttpa@rkaganb.sw.ru>
 <20160620212908.GA17813@amt.cnet>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Cc: <kvm@vger.kernel.org>, "Denis V. Lunev" <den@openvz.org>,
	Owen Hofmann <osh@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mail-am1on0102.outbound.protection.outlook.com ([157.56.112.102]:35488
	"EHLO emea01-am1-obe.outbound.protection.outlook.com"
	rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP
	id S1751398AbcFUOm1 (ORCPT <rfc822;kvm@vger.kernel.org>);
	Tue, 21 Jun 2016 10:42:27 -0400
Content-Disposition: inline
In-Reply-To: <20160620212908.GA17813@amt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

On Mon, Jun 20, 2016 at 06:29:10PM -0300, Marcelo Tosatti wrote:
> On Mon, Jun 20, 2016 at 08:22:49PM +0300, Roman Kagan wrote:
> > The per-vcpu hv_clock is updated when the vcpu processes
> > KVM_REQ_CLOCK_UPDATE request.
> 
> Yes.
> 
> > Once kvm_gen_update_masterclock() sets KVM_REQ_CLOCK_UPDATE and
> > clears KVM_REQ_MCLOCK_INPROGRESS for all vcpus, one vcpu can process the
> > requests, enter the guest, and read another vcpu's hv_clock, before that
> > other vcpu had a chance to process its KVM_REQ_CLOCK_UPDATE request.
> 
> Yes. But guest code should be reading its local kvmclock area:
> 
>                 /*
>                  * Test we're still on the cpu as well as the version.
>                  * We could have been migrated just after the first
>                  * vgetcpu but before fetching the version, so we
>                  * wouldn't notice a version change.
>                  */
>                 cpu1 = __getcpu() & VGETCPU_CPU_MASK;
> 
> (vclock_gettime.c)

This code is from an older version.  The latest always reads the clock
of the CPU #0:

        /*
         * Note: The kernel and hypervisor must guarantee that cpu ID
         * number maps 1:1 to per-CPU pvclock time info.
         *
         * Because the hypervisor is entirely unaware of guest userspace
         * preemption, it cannot guarantee that per-CPU pvclock time
         * info is updated if the underlying CPU changes or that that
         * version is increased whenever underlying CPU changes.
         *
         * On KVM, we are guaranteed that pvti updates for any vCPU are
         * atomic as seen by *all* vCPUs.  This is an even stronger
         * guarantee than we get with a normal seqlock.
         *
         * On Xen, we don't appear to have that guarantee, but Xen still
         * supplies a valid seqlock using the version field.
         *
         * We only do pvclock vdso timing at all if
         * PVCLOCK_TSC_STABLE_BIT is set, and we interpret that bit to
         * mean that all vCPUs have matching pvti and that the TSC is
         * synced, so we can just look at vCPU 0's pvti.
         */

> > Is there anything that prevents this?
> 
> Guest code confirming both version and cpu do not change across 
> a kvmclock read. Other than this, no.

So is the code reading another vcpu's hv_clock wrong?

Roman.