From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] tsc: use kvmclock for calibration Date: Sun, 12 Aug 2012 12:00:07 +0300 Message-ID: <50277097.9030502@redhat.com> References: <1344513463-7329-1-git-send-email-kraxel@redhat.com> <5023B2C4.90302@redhat.com> <5023C1D2.5090103@redhat.com> <5023C2BE.5070702@redhat.com> <20120809190913.GG20889@amt.cnet> <5024C1F3.80103@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , seabios@seabios.org, kvm@vger.kernel.org To: Gerd Hoffmann Return-path: Received: from mx1.redhat.com ([209.132.183.28]:11113 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750796Ab2HLJAf (ORCPT ); Sun, 12 Aug 2012 05:00:35 -0400 In-Reply-To: <5024C1F3.80103@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 08/10/2012 11:10 AM, Gerd Hoffmann wrote: > Hi, > >>>> >>> (1) Use this patch (with alignment issue fixed of course). >>>> >>> (2) Do a full kvmclock implementation. Feels a bit like overkill. >>>> >>> (3) SeaBIOS can fallback to the PIT for timing on machines which >>>> >>> have no TSC. We could do that too in case we detect kvm ... >>> >> >>> >> What sort of timeouts are these? If seconds, maybe the rtc would be best. >> > >> > I vote for 3 so nobody has to maintain kvmclock code in SeaBIOS and Gerd >> > can fix the in-kernel PIT issues with GRUB (see Michaels message) while testing. > (2) turned out to be not too bad when taking a shortcut: Go through an > enable/disable cycle each time we read the clock, then just grab > system_time. Not that efficient, but should be ok for seabios. Usually > it checks the clock when sitting around idle, waiting for something to > happen. And it simplifies the implementation alot as we can just skip > all the tsc frequency & delta calculations. > > Draft patch attached. Comments? > > + > +static void kvmclock_fetch(struct pvclock_vcpu_time_info *time) > +{ > + u32 addr = (u32)MAKE_FLATPTR(GET_SEG(SS), time); > + u32 msr = GET_GLOBAL(kvm_systime_msr); > + > + memset(time, 0, sizeof(*time)); > + wrmsr(msr, addr | 1); I'd put the time calculations in here. We don't specify what happens to the data area after disabling kvmclock; it could be in the middle of an update. > + wrmsr(msr, 0); > +} > + > +u64 kvmclock_get(void) > +{ > + struct pvclock_vcpu_time_info time; > + > + kvmclock_fetch(&time); > + return time.system_time; That's just a random number. You have to do the full calculation. -- error compiling committee.c: too many arguments to function