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

Avi Kivity
2008-01-22T13:54:10Z

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