On 08/09/2012 05:01 PM, Avi Kivity wrote:
> On 08/09/2012 04:57 PM, Gerd Hoffmann wrote:
>> Hi,
>>
>>>> +u64 kvm_tsc_khz(void)
>>>> +{
>>>> + u32 eax, ebx, ecx, edx, msr;
>>>> + struct pvclock_vcpu_time_info time;
>>>> + u32 addr = (u32)(&time);
>>>> + u64 khz;
>>>> +
>>>> + /* check presence and figure msr number */
>>>> + cpuid(KVM_CPUID_FEATURES, &eax, &ebx, &ecx, &edx);
>>>> + if (eax & KVM_FEATURE_CLOCKSOURCE2) {
>>>> + msr = MSR_KVM_SYSTEM_TIME_NEW;
>>>> + } else if (eax & KVM_FEATURE_CLOCKSOURCE) {
>>>> + msr = MSR_KVM_SYSTEM_TIME;
>>>> + } else {
>>>> + return 0;
>>>> + }
>>>> +
>>>> + /* ask kvm hypervisor to fill struct */
>>>> + memset(&time, 0, sizeof(time));
>>>> + wrmsr(msr, addr | 1);
>>>
>>> How can this work?
>>
>> It did in my testing, although maybe by pure luck ...
>>
>>> There is a 64-byte alignment requirement.
>>
>> 64 bytes? Sure? The whole struct is only 32 bytes in size ...
>
> er, the documentation says 4 bytes (so stack alignment works). I
> distinctly remember having a large alignment requirement so we don't
> cross a page or slot boundary... something's wrong here.
case MSR_KVM_SYSTEM_TIME: {
kvmclock_reset(vcpu);
vcpu->arch.time = data;
kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
/* we verify if the enable bit is set... */
if (!(data & 1))
break;
/* ...but clean it before doing the actual write */
vcpu->arch.time_offset = data & ~(PAGE_MASK | 1);
vcpu->arch.time_page =
gfn_to_page(vcpu->kvm, data >> PAGE_SHIFT);
if (is_error_page(vcpu->arch.time_page))
vcpu->arch.time_page = NULL;
break;
So your tests worked by pure luck, but the bug is in kvm. We need to
grab two pages here.
--
error compiling committee.c: too many arguments to function
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
