Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
Hi, Isnt pmtimer ioport usable? 14MHz. Can give it a try. 14 MHz looks wrong though, apci.h says: /* PM Timer ticks per second (HZ) */ #define PM_TIMER_FREQUENCY 3579545 Is this fixed? Or hardware specific? cheers, Gerd ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On Mon, Aug 13, 2012 at 12:37:11PM +0200, Gerd Hoffmann wrote: Hi, Isnt pmtimer ioport usable? 14MHz. Can give it a try. 14 MHz looks wrong though, apci.h says: /* PM Timer ticks per second (HZ) */ #define PM_TIMER_FREQUENCY 3579545 Is this fixed? Or hardware specific? 3.579545 MHz clock required by ACPI spec. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
Add a comment about it in the source code. -#define PM_TIMER_FREQUENCY 3579545 +#define PM_TIMER_FREQUENCY 3579545 // 3.579545 MHz clock required by ACPI spec. On Mon, Aug 13, 2012 at 12:46 PM, Gleb Natapov g...@redhat.com wrote: On Mon, Aug 13, 2012 at 12:37:11PM +0200, Gerd Hoffmann wrote: Hi, Isnt pmtimer ioport usable? 14MHz. Can give it a try. 14 MHz looks wrong though, apci.h says: /* PM Timer ticks per second (HZ) */ #define PM_TIMER_FREQUENCY 3579545 Is this fixed? Or hardware specific? 3.579545 MHz clock required by ACPI spec. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On 08/09/2012 09:59 PM, Marcelo Tosatti wrote: +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. Thats why there exists hardware tsc frequency scaling and the software equivalent for that on kvm. The hardware is only available on a minority of processors in the field. The software equivalent isn't there. -- error compiling committee.c: too many arguments to function ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On 08/09/2012 10:02 PM, Marcelo Tosatti wrote: On Thu, Aug 09, 2012 at 05:20:11PM +0300, Avi Kivity wrote: On 08/09/2012 05:18 PM, Gerd Hoffmann wrote: Hi, 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. All sorts of timeouts, from a few miliseconds to seconds. The problematic ones are the longer timeouts, which wait for I/O stuff like disk reads complete. The stuff with smaller timeouts (like waiting for AHCI link become ready) tend to finish instantly in kvm. That's not guaranteed. The AHCI adapter might be real hardware. Or the emulation may change. What's wrong with having a full kvmclock implementation? Instead of issuing rdtsc call a function pointer. Its not necessary (someone is going to maintain the kvmclock frequency retrieve, which patch is already here, versus maintainance of full kvmclock). The frequency is meaninless. Frequency scaling (or the software equivalent: TSC trapping) are required for other reasons anyway. One thing we can do is enable TSC trapping, then disable it if the guest activates kvmclock. That gives us accurate time either way. -- error compiling committee.c: too many arguments to function ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On Fri, Aug 10, 2012 at 10:18:00AM +0300, Gleb Natapov wrote: can fix the in-kernel PIT issues with GRUB (see Michaels message) while testing. What message exactly? found it. -- Gleb. ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
Hi,
(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.
I vote for 3 so nobody has to maintain kvmclock code in SeaBIOS and Gerd
can fix the in-kernel PIT issues with GRUB (see Michaels message) while
testing.
(2) turned out to be not too bad when taking a shortcut: Go through an
enable/disable cycle each time we read the clock, then just grab
system_time. Not that efficient, but should be ok for seabios. Usually
it checks the clock when sitting around idle, waiting for something to
happen. And it simplifies the implementation alot as we can just skip
all the tsc frequency delta calculations.
Draft patch attached. Comments?
cheers,
Gerd
From e42d62e90ae4b8a00413a0665d4022069154a516 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann kra...@redhat.com
Date: Thu, 9 Aug 2012 13:26:18 +0200
Subject: [PATCH] kvmclock clocksource
Signed-off-by: Gerd Hoffmann kra...@redhat.com
---
Makefile |4 +-
src/clock.c| 13 +++
src/paravirt.c | 65
src/paravirt.h |3 ++
4 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index 72ee152..b692a96 100644
--- a/Makefile
+++ b/Makefile
@@ -13,11 +13,11 @@ SRCBOTH=misc.c stacks.c pmm.c output.c util.c block.c
floppy.c ata.c mouse.c \
pnpbios.c pirtable.c vgahooks.c ramdisk.c pcibios.c blockcmd.c \
usb.c usb-uhci.c usb-ohci.c usb-ehci.c usb-hid.c usb-msc.c \
virtio-ring.c virtio-pci.c virtio-blk.c virtio-scsi.c apm.c ahci.c \
-usb-uas.c lsi-scsi.c esp-scsi.c
+usb-uas.c lsi-scsi.c esp-scsi.c paravirt.c
SRC16=$(SRCBOTH) system.c disk.c font.c
SRC32FLAT=$(SRCBOTH) post.c shadow.c memmap.c coreboot.c boot.c \
acpi.c smm.c mptable.c smbios.c pciinit.c optionroms.c mtrr.c \
-lzmadecode.c bootsplash.c jpeg.c usb-hub.c paravirt.c \
+lzmadecode.c bootsplash.c jpeg.c usb-hub.c \
biostables.c xen.c bmp.c romfile.c
SRC32SEG=util.c output.c pci.c pcibios.c apm.c stacks.c
diff --git a/src/clock.c b/src/clock.c
index 69e9f17..15921fa 100644
--- a/src/clock.c
+++ b/src/clock.c
@@ -13,6 +13,7 @@
#include bregs.h // struct bregs
#include biosvar.h // GET_GLOBAL
#include usb-hid.h // usb_check_event
+#include paravirt.h // kvm clock
// RTC register flags
#define RTC_A_UIP 0x80
@@ -64,6 +65,7 @@
u32 cpu_khz VAR16VISIBLE;
u8 no_tsc VAR16VISIBLE;
+u8 use_kvmclock VAR16VISIBLE;
static void
calibrate_tsc(void)
@@ -80,6 +82,15 @@ calibrate_tsc(void)
return;
}
+if (kvm_para_available()) {
+u32 hz = kvmclock_init();
+if (hz != 0) {
+SET_GLOBAL(use_kvmclock, 1);
+SET_GLOBAL(cpu_khz, hz / 1000);
+return;
+}
+}
+
// Setup timer2
u8 orig = inb(PORT_PS2_CTRLB);
outb((orig ~PPCB_SPKR) | PPCB_T2GATE, PORT_PS2_CTRLB);
@@ -134,6 +145,8 @@ get_tsc(void)
{
if (unlikely(GET_GLOBAL(no_tsc)))
return emulate_tsc();
+if (unlikely(GET_GLOBAL(use_kvmclock)))
+return kvmclock_get();
return rdtscll();
}
diff --git a/src/paravirt.c b/src/paravirt.c
index 2a98d53..07aa926 100644
--- a/src/paravirt.c
+++ b/src/paravirt.c
@@ -12,6 +12,7 @@
#include ioport.h // outw
#include paravirt.h // qemu_cfg_port_probe
#include smbios.h // struct smbios_structure_header
+#include biosvar.h // GET_GLOBAL
int qemu_cfg_present;
@@ -346,3 +347,67 @@ void qemu_cfg_romfile_setup(void)
dprintf(3, Found fw_cfg file: %s (size=%d)\n, file-name,
file-size);
}
}
+
+#define KVM_CPUID_SIGNATURE 0x4000
+#define KVM_CPUID_FEATURES0x4001
+#define KVM_FEATURE_CLOCKSOURCE0
+#define KVM_FEATURE_CLOCKSOURCE2 3
+#define MSR_KVM_SYSTEM_TIME 0x12
+#define MSR_KVM_SYSTEM_TIME_NEW 0x4b564d01
+
+struct pvclock_vcpu_time_info {
+ u32 version;
+ u32 pad0;
+ u64 tsc_timestamp;
+ u64 system_time;
+ u32 tsc_to_system_mul;
+ s8tsc_shift;
+ u8flags;
+ u8pad[2];
+} PACKED;
+
+/* kvmclock system time runs with nanoseconds */
+#define KVM_SYSTIME_HZ 10
+
+u32 kvm_systime_msr VAR16VISIBLE;
+
+static void kvmclock_fetch(struct pvclock_vcpu_time_info *time)
+{
+u32 addr = (u32)MAKE_FLATPTR(GET_SEG(SS), time);
+u32 msr = GET_GLOBAL(kvm_systime_msr);
+
+memset(time, 0, sizeof(*time));
+wrmsr(msr, addr | 1);
+wrmsr(msr, 0);
+}
+
+u64 kvmclock_get(void)
+{
+struct pvclock_vcpu_time_info time;
+
+kvmclock_fetch(time);
+return time.system_time;
+}
+
+u32 kvmclock_init(void)
+{
+u32 eax, ebx, ecx, edx;
+struct pvclock_vcpu_time_info time;
+
+cpuid(KVM_CPUID_FEATURES, eax,
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On Fri, Aug 10, 2012 at 10:10:27AM +0200, Gerd Hoffmann wrote: Hi, (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. I vote for 3 so nobody has to maintain kvmclock code in SeaBIOS and Gerd can fix the in-kernel PIT issues with GRUB (see Michaels message) while testing. (2) turned out to be not too bad when taking a shortcut: Go through an enable/disable cycle each time we read the clock, then just grab system_time. Not that efficient, but should be ok for seabios. Usually it checks the clock when sitting around idle, waiting for something to happen. And it simplifies the implementation alot as we can just skip all the tsc frequency delta calculations. Draft patch attached. Comments? Given the history of problems with kvmclock, would rather see it not being used for delays, if possible. Your shortcut gets rid of a class of problems, but there might be others (...). Isnt pmtimer ioport usable? 14MHz. Error handling in kvmclock_init is awkward. Thanks ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On 08/09/2012 02:57 PM, Gerd Hoffmann wrote:
Use kvmclock for tsc calibration when running on kvm. Without this the
tsc frequency calibrated by seabios can be *way* off in case the virtual
machine is booted on a loaded host. I've seen seabios calibrating 27
instead of ca. 2800 MHz, resulting in timeouts being to short by factor
100. Which in turn leads to disk I/O errors due to timeouts, especially
as I/O requests tend to take a bit longer than usual on a loaded box ...
+
+struct pvclock_vcpu_time_info {
+ u32 version;
+ u32 pad0;
+ u64 tsc_timestamp;
+ u64 system_time;
+ u32 tsc_to_system_mul;
+ s8tsc_shift;
+ u8flags;
+ u8pad[2];
+} PACKED;
+
+
+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? There is a 64-byte alignment requirement.
+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.
--
error compiling committee.c: too many arguments to function
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
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 ...
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 ...
cheers,
Gerd
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On 08/09/2012 05:12 PM, Gerd Hoffmann wrote:
Hi,
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: {
[ ... ]
So your tests worked by pure luck, but the bug is in kvm. We need to
grab two pages here.
Ok, so better use memalign(32,32) to make sure the struct doesn't cross
a page border ...
No, we need to fix kvm, no need to complicate the guest for that.
--
error compiling committee.c: too many arguments to function
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
Hi, 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. All sorts of timeouts, from a few miliseconds to seconds. The problematic ones are the longer timeouts, which wait for I/O stuff like disk reads complete. The stuff with smaller timeouts (like waiting for AHCI link become ready) tend to finish instantly in kvm. cheers, Gerd ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On 08/09/2012 05:18 PM, Gerd Hoffmann wrote: Hi, 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. All sorts of timeouts, from a few miliseconds to seconds. The problematic ones are the longer timeouts, which wait for I/O stuff like disk reads complete. The stuff with smaller timeouts (like waiting for AHCI link become ready) tend to finish instantly in kvm. That's not guaranteed. The AHCI adapter might be real hardware. Or the emulation may change. What's wrong with having a full kvmclock implementation? Instead of issuing rdtsc call a function pointer. -- error compiling committee.c: too many arguments to function ___ SeaBIOS mailing list SeaBIOS@seabios.org http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
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
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
It should be kHz not khz.
-msr, (u32)khz / 1000);
+msr, (u32)kHz / 1000);
On Thu, Aug 9, 2012 at 2:53 PM, Avi Kivity a...@redhat.com wrote:
On 08/09/2012 02:57 PM, Gerd Hoffmann wrote:
Use kvmclock for tsc calibration when running on kvm. Without this the
tsc frequency calibrated by seabios can be *way* off in case the virtual
machine is booted on a loaded host. I've seen seabios calibrating 27
instead of ca. 2800 MHz, resulting in timeouts being to short by factor
100. Which in turn leads to disk I/O errors due to timeouts, especially
as I/O requests tend to take a bit longer than usual on a loaded box ...
+
+struct pvclock_vcpu_time_info {
+ u32 version;
+ u32 pad0;
+ u64 tsc_timestamp;
+ u64 system_time;
+ u32 tsc_to_system_mul;
+ s8tsc_shift;
+ u8flags;
+ u8pad[2];
+} PACKED;
+
+
+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? There is a 64-byte alignment requirement.
+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.
--
error compiling committee.c: too many arguments to function
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On Thu, Aug 09, 2012 at 05:01:34PM +0300, 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.
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.
I vote for 3 so nobody has to maintain kvmclock code in SeaBIOS and Gerd
can fix the in-kernel PIT issues with GRUB (see Michaels message) while testing.
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH] tsc: use kvmclock for calibration
On Thu, Aug 09, 2012 at 03:53:24PM +0300, Avi Kivity wrote:
On 08/09/2012 02:57 PM, Gerd Hoffmann wrote:
Use kvmclock for tsc calibration when running on kvm. Without this the
tsc frequency calibrated by seabios can be *way* off in case the virtual
machine is booted on a loaded host. I've seen seabios calibrating 27
instead of ca. 2800 MHz, resulting in timeouts being to short by factor
100. Which in turn leads to disk I/O errors due to timeouts, especially
as I/O requests tend to take a bit longer than usual on a loaded box ...
+
+struct pvclock_vcpu_time_info {
+ u32 version;
+ u32 pad0;
+ u64 tsc_timestamp;
+ u64 system_time;
+ u32 tsc_to_system_mul;
+ s8tsc_shift;
+ u8flags;
+ u8pad[2];
+} PACKED;
+
+
+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? There is a 64-byte alignment requirement.
+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.
Thats why there exists hardware tsc frequency scaling and the software
equivalent for that on kvm.
You want accurate time on kvm, don't use the tsc.
--
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
___
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios
