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. > > Easily fixable though, just need to grab some memory with memalign > instead of using the stack. > >>> + wrmsr(msr, 0); >>> + if (time.version < 2 || time.tsc_to_system_mul == 0) >>> + return 0; >>> + >>> + /* go figure tsc frequency */ >>> + khz = pvclock_tsc_khz(&time); >>> + dprintf(1, "Using kvmclock, msr 0x%x, tsc %d MHz\n", >>> + msr, (u32)khz / 1000); >>> + return khz; >> >> That's a meaningless number. You can be migrated to a cpu or a machine >> with very different tsc. > >> You want accurate time on kvm, don't use the tsc. > > seabios uses the tsc for timeout calculations only, so it doesn't need > to be 100% accurate. The order of magnitude should be correct though. > The Linux kernel uses the value for delay loops too, so using it for the > given purpose can't be *that* horrible after all ... > > It is certainly an improvement over the current code which tries to > calibrate the tsc and gets totally broken results in case the busy host > happens to schedule the guest in the middle of calibration. > > So what do you suggest? The options I see are: > > (1) Use this patch (with alignment issue fixed of course). > (2) Do a full kvmclock implementation. Feels a bit like overkill. > (3) SeaBIOS can fallback to the PIT for timing on machines which > have no TSC. We could do that too in case we detect kvm ... What sort of timeouts are these? If seconds, maybe the rtc would be best. -- 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html