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

Avi Kivity
2008-01-23T09:46:22Z

Yang, Sheng wrote:
> 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.
>
>

It breaks the calculation, though.

I suggest moving to the Linux clock API (which is in nanoseconds).

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

guest_read_tsc() uses vmcs_readl() which implicitly refers to the
current vcpu. So you don't actually read vcpu0 tsc.

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