From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: KVM: x86: update masterclock when kvmclock_offset is calculated Date: Thu, 22 Aug 2013 19:05:20 +0200 Message-ID: <521644D0.9050902@redhat.com> References: <20130820182012.GA814@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: kvm-devel , Gleb Natapov To: Marcelo Tosatti Return-path: Received: from mail-wi0-f172.google.com ([209.85.212.172]:40701 "EHLO mail-wi0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753438Ab3HVTUq (ORCPT ); Thu, 22 Aug 2013 15:20:46 -0400 Received: by mail-wi0-f172.google.com with SMTP id hj13so1117673wib.17 for ; Thu, 22 Aug 2013 12:20:45 -0700 (PDT) In-Reply-To: <20130820182012.GA814@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: Il 20/08/2013 20:20, Marcelo Tosatti ha scritto: > > The offset to add to the hosts monotonic time, kvmclock_offset, is > calculated against the monotonic time at KVM_SET_CLOCK ioctl time. > > Request a master clock update at this time, to reduce a potentially > unbounded difference between the values of the masterclock and > the clock value used to calculate kvmclock_offset. > > Signed-off-by: Marcelo Tosatti > > Index: linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c > =================================================================== > --- linux-2.6-kvmclock-fixes.orig/arch/x86/kvm/x86.c > +++ linux-2.6-kvmclock-fixes/arch/x86/kvm/x86.c > @@ -3806,6 +3806,7 @@ long kvm_arch_vm_ioctl(struct file *filp > delta = user_ns.clock - now_ns; > local_irq_enable(); > kvm->arch.kvmclock_offset = delta; > + kvm_gen_update_masterclock(kvm); > break; > } > case KVM_GET_CLOCK: { Reviewed-by: Paolo Bonzini While reviewing this patch, which BTW looks good, I noticed the handling of KVM_REQ_MCLOCK_INPROGRESS, the dummy request that is never processed and is only used to block guest entry. It seems to me that this bit is not necessary. After KVM_REQ_CLOCK_UPDATE is issued, no guest entries will happen because kvm_guest_time_update will try to take the pvclock_gtod_sync_lock, currently taken by kvm_gen_update_masterclock. Thus, you do not need the dummy request. You can simply issue KVM_REQ_CLOCK_UPDATE before calling pvclock_update_vm_gtod_copy (with the side effect of exiting VCPUs). VCPUs will stall in kvm_guest_time_update until pvclock_gtod_sync_lock is released by kvm_gen_update_masterclock. What do you think? On top of this, optionally the spinlock could become an rw_semaphore so that clock updates for different VCPUs will not be serialized. The effect is probably not visible, though. Paolo > -- > To unsubscribe from this list: send the line "unsubscribe kvm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >