From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Yang, Sheng" Subject: Re: [RFC][PATCH 1/2] KVM: In-kernel PIT model Date: Wed, 23 Jan 2008 14:00:35 +0800 Message-ID: <200801231400.36063.sheng.yang@intel.com> References: <200801211718.23664.sheng.yang@intel.com> <4795F582.7050802@qumranet.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org To: Avi Kivity Return-path: In-Reply-To: <4795F582.7050802-atKUWr5tajBWk0Htik3J/w@public.gmane.org> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org On Tuesday 22 January 2008 21:54:10 Avi Kivity wrote: > Yang, Sheng wrote: > > + > > +/* Compute with 96 bit intermediate result: (a*b)/c */ > > +static u64 muldiv64(u64 a, u32 b, u32 c) > > Why do we need such high accuracy for the pit? The direct reason is we using TSC as the reference of pit count_load_time. I think the only alternative is using host ACPI PM timer. But it will wrapped soon... > > > + > > +static int pit_get_out(struct kvm *kvm, int channel) > > +{ > > + struct PITChannelState *c = > > + &kvm->arch.vpit->pit_state.channels[channel]; > > + struct kvm_vcpu *vcpu = kvm->vcpus[0]; > > + u64 d, t; > > + int out; > > + > > + ASSERT(mutex_is_locked(&kvm->arch.vpit->pit_state.lock)); > > + > > + kvm_get_msr(vcpu, MSR_IA32_TIME_STAMP_COUNTER, &t); > > + d = muldiv64(t - c->count_load_time, PIT_FREQ, cpu_khz * 1000); > > I assume this is to correlate the tsc and the pit counters. Doesn't > this break with cpu frequency scaling or with vcpu migrations? I think the cpu frequency scaling will affect TSC, though cpu_khz would be changed along with it. > > What if this is called within the context of vcpu 1? Not sure > kvm_get_msr() will work correctly at all. I don't think so. kvm_get_msr() with TSC only read guest tsc, which is indentity for the domain, not specific for vcpu. And I indicated vcpu0 as vcpu(though won't be refered). But I may do it more clear in the future. > > > +struct PITTimer { > > > > + > > +struct PITChannelState { > > + > > +struct PITState { > > Please use the traditional naming convention (pit_timer, > pit_channel_state, pit_state) Sure. -- Thanks Yang, Sheng ------------------------------------------------------------------------- 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/