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.