Re: [RFC][PATCH 1/2] KVM: In-kernel PIT model

["Yang, Sheng" <sheng.yang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.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/