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

[Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org>]

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?

> +
> +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?

What if this is called within the context of vcpu 1?  Not sure 
kvm_get_msr() will work correctly at all.

> +struct PITTimer {
>   
> +
> +struct PITChannelState {
> +
> +struct PITState {
>   

Please use the traditional naming convention (pit_timer, 
pit_channel_state, pit_state)

-- 
error compiling committee.c: too many arguments to function


-------------------------------------------------------------------------
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/