From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [PATCH 1/2] kvmclock - the host part. Date: Mon, 18 Feb 2008 11:06:54 -0300 Message-ID: <20080218140654.GA10446@dmt> References: <1202753274-15252-1-git-send-email-gcosta@redhat.com> <1202753274-15252-2-git-send-email-gcosta@redhat.com> <47B2CB46.1090804@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel@lists.sourceforge.net, chrisw@sous-sol.org, Glauber de Oliveira Costa To: Avi Kivity Return-path: Content-Disposition: inline In-Reply-To: <47B2CB46.1090804@qumranet.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces@lists.sourceforge.net Errors-To: kvm-devel-bounces@lists.sourceforge.net List-Id: kvm.vger.kernel.org On Wed, Feb 13, 2008 at 12:49:42PM +0200, Avi Kivity wrote: > Glauber de Oliveira Costa wrote: > > This is the host part of kvm clocksource implementation. As it does > > not include clockevents, it is a fairly simple implementation. We > > only have to register a per-vcpu area, and start writting to it periodically. > > > > The area is binary compatible with xen, as we use the same shadow_info structure. > > > > > > > +static void kvm_write_wall_clock(struct kvm_vcpu *v, gpa_t wall_clock) > > +{ > > + int version = 1; > > + struct kvm_wall_clock wc; > > + unsigned long flags; > > + struct timespec wc_ts; > > + > > + local_irq_save(flags); > > + kvm_get_msr(v, MSR_IA32_TIME_STAMP_COUNTER, > > + &v->arch.hv_clock.tsc_timestamp); > > > > Why is this needed? IIRC the wall clock is not tied to any vcpu. > > If we can remove this, the argument to the function should be kvm, not > kvm_vcpu. We can remove the irq games as well. > > > + wc_ts = current_kernel_time(); > > + local_irq_restore(flags); > > + > > + down_write(¤t->mm->mmap_sem); > > + kvm_write_guest(v->kvm, wall_clock, &version, sizeof(version)); > > + up_write(¤t->mm->mmap_sem); > > > > Why down_write? accidentally or on purpose? > > For mutual exclusion, I suggest taking kvm->lock instead (for the entire > function). You need slots_lock in read-mode for kvm_write_guest(). kvm->lock does not protect against memslot changes and the dirty bitmap. ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/