From mboxrd@z Thu Jan  1 00:00:00 1970
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [PATCH] KVM: x86: fix KVM_SET_CLOCK relative to setting correct
 clock value
Date: Wed, 3 May 2017 15:55:36 +0200
Message-ID: <568f71c4-440a-dcfd-3bd0-544275d104ee@redhat.com>
References: <20170502213616.GA24837@amt.cnet>
 <2499ef65-1dfe-8460-ec41-661b05cc5023@redhat.com>
 <20170503134057.GA10468@amt.cnet>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 7bit
Cc: kvm-devel <kvm@vger.kernel.org>,
        =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>
To: Marcelo Tosatti <mtosatti@redhat.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:50442 "EHLO mx1.redhat.com"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1752299AbdECNzr (ORCPT <rfc822;kvm@vger.kernel.org>);
        Wed, 3 May 2017 09:55:47 -0400
Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22])
        (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
        (No client certificate requested)
        by mx1.redhat.com (Postfix) with ESMTPS id EF0F881229
        for <kvm@vger.kernel.org>; Wed,  3 May 2017 13:55:41 +0000 (UTC)
In-Reply-To: <20170503134057.GA10468@amt.cnet>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>



On 03/05/2017 15:40, Marcelo Tosatti wrote:
>>> In the masterclock enabled case, kvmclock_offset must be adjusted so
>>> that user_ns.clock = master_kernel_ns + kvmclock_offset (that is, the
>>> value set from KVM_SET_CLOCK is the one visible at system_timestamp).
>>>
>>> This way the guest clock:
>>>
>>> 	1. Starts counting when KVM_SET_CLOCK executes.
>>> 	2. With the value provided by userspace.
>>
>> So this fixes rounding errors?
> 
> No. It just does the correct 
> 
> "user_ns.clock = master_kernel_ns + kvmclock_offset = return value of
> get_kvmclock_ns(kvm) at moment of KVM_SET_CLOCK" 
> 
> (that is, when guest_rdtsc() == tsc_timestamp, so guest_rdtsc() - tsc_timestamp = 0, 
> you want get_kvmclock_ns() to return user_ns.clock).

Yep, and it currently doesn't match because the current code introduces
rounding errors?

>>> +			kvm_for_each_vcpu(i, vcpu, kvm)
>>> +				kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
>> kvm_gen_update_masterclock already does that, why did you move that to
>> before the assignment?
> Its after the assignment of kvmclock_offset because
> KVM_REQ_CLOCK_UPDATES processing by vcpus make use of kvmclock_offset.

Sure, but why did you move kvm_gen_update_masterclock before?

If you left kvm_gen_update_masterclock after the update of
kvm->arch.kvmclock_offset, the duplicate KVM_REQ_CLOCK_UPDATE would not
be necessary.

Paolo

> So if you change that value, you have to request the vcpus to
> recalculate their kvmclock areas using the new kvmclock_offset.