From mboxrd@z Thu Jan 1 00:00:00 1970 From: Gleb Natapov Subject: Re: [patch 01/16] KVM: x86: retain pvclock guest stopped bit in guest memory Date: Thu, 1 Nov 2012 12:39:30 +0200 Message-ID: <20121101103930.GP25650@redhat.com> References: <20121031224656.417434866@redhat.com> <20121031224823.865958085@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, johnstul@us.ibm.com, jeremy@goop.org, glommer@parallels.com, zamsden@gmail.com, avi@redhat.com, pbonzini@redhat.com To: Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:37160 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755802Ab2KAKlK (ORCPT ); Thu, 1 Nov 2012 06:41:10 -0400 Content-Disposition: inline In-Reply-To: <20121031224823.865958085@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Wed, Oct 31, 2012 at 08:46:57PM -0200, Marcelo Tosatti wrote: > Otherwise its possible for an unrelated KVM_REQ_UPDATE_CLOCK (such as due to CPU > migration) to clear the bit. > > Noticed by Paolo Bonzini. > > Signed-off-by: Marcelo Tosatti > Reviewed-by: Gleb Natapov Small nitpick bellow. Also do we really need to call kvm_guest_time_update() on a guest pause? Wouldn't separate request bit, which only sets the flag, suffice? > Index: vsyscall/arch/x86/kvm/x86.c > =================================================================== > --- vsyscall.orig/arch/x86/kvm/x86.c > +++ vsyscall/arch/x86/kvm/x86.c > @@ -1143,6 +1143,7 @@ static int kvm_guest_time_update(struct > unsigned long this_tsc_khz; > s64 kernel_ns, max_kernel_ns; > u64 tsc_timestamp; > + struct pvclock_vcpu_time_info *guest_hv_clock; > u8 pvclock_flags; > > /* Keep irq disabled to prevent changes to the clock */ > @@ -1226,13 +1227,6 @@ static int kvm_guest_time_update(struct > vcpu->last_kernel_ns = kernel_ns; > vcpu->last_guest_tsc = tsc_timestamp; > > - pvclock_flags = 0; > - if (vcpu->pvclock_set_guest_stopped_request) { > - pvclock_flags |= PVCLOCK_GUEST_STOPPED; > - vcpu->pvclock_set_guest_stopped_request = false; > - } > - > - vcpu->hv_clock.flags = pvclock_flags; > > /* > * The interface expects us to write an even number signaling that the > @@ -1243,6 +1237,18 @@ static int kvm_guest_time_update(struct > > shared_kaddr = kmap_atomic(vcpu->time_page); > > + guest_hv_clock = shared_kaddr + vcpu->time_offset; > + > + /* retain PVCLOCK_GUEST_STOPPED if set in guest copy */ > + pvclock_flags = (guest_hv_clock->flags & PVCLOCK_GUEST_STOPPED); > + > + if (vcpu->pvclock_set_guest_stopped_request) { > + pvclock_flags |= PVCLOCK_GUEST_STOPPED; > + vcpu->pvclock_set_guest_stopped_request = false; > + } > + > + vcpu->hv_clock.flags = pvclock_flags; > + > memcpy(shared_kaddr + vcpu->time_offset, &vcpu->hv_clock, We can use guest_hv_clock here now. > sizeof(vcpu->hv_clock)); > > -- Gleb.