Skip to contentSkip to conversation

synchronized TSC between vcpu's on SMP guests

PATCH Applied Marcelo Tosatti 4 participants 15 messages Dec 9, 2008 – Dec 28, 2008

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Marcelo Tosatti

author
#0 lore raw

cover synchronized TSC between vcpu's on SMP guests

Most Intel hosts are supposed to have their TSC's synchronized. This patchset attempts to fix the sites which overwrite the TSC making them appear unsynchronized to the guest.

author
#1 lore raw

PATCH 1/3 QEMU/KVM: x86: separate TSC load from kvm_arch_load_regs

kvm_load_registers is a general interface to load registers, and is used by vmport, gdbstub, etc. The TSC MSR is continually counting, so it can't be simply read and written back as the other registers/MSR's (doing so overwrites the current count).

Introduce kvm_load_tsc and use it for x86's migration CPU load code.

Signed-off-by Marcelo Tosatti <[email protected]>
qemu/qemu-kvm-x86.c +12 -1
Index: kvm-userspace.tip/qemu/qemu-kvm-x86.c
===================================================================
@@ -257,7 +257,6 @@ void kvm_arch_load_regs(CPUState *env)
     set_msr_entry(&msrs[n++], MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
     if (kvm_has_msr_star)
 	set_msr_entry(&msrs[n++], MSR_STAR,              env->star);
-    set_msr_entry(&msrs[n++], MSR_IA32_TSC, env->tsc);
 #ifdef TARGET_X86_64
     if (lm_capable_kernel) {
         set_msr_entry(&msrs[n++], MSR_CSTAR,             env->cstar);

@@ -272,6 +271,18 @@ void kvm_arch_load_regs(CPUState *env)
         perror("kvm_set_msrs FAILED");
 }
 
+void kvm_load_tsc(CPUState *env)
+{
+    int rc;
+    struct kvm_msr_entry msr;
+
+    set_msr_entry(&msr, MSR_IA32_TSC, env->tsc);
+
+    rc = kvm_set_msrs(kvm_context, env->cpu_index, &msr, 1);
+    if (rc == -1)
+        perror("kvm_set_tsc FAILED.\n");
+}
+
 void kvm_save_mpstate(CPUState *env)
 {
 #ifdef KVM_CAP_MP_STATE
qemu/qemu-kvm-x86.h new file +13 -0
Index: kvm-userspace.tip/qemu/qemu-kvm-x86.h
===================================================================
@@ -0,0 +1,13 @@
+/*
+ * qemu/kvm integration
+ *
+ * Copyright (C) 2006-2008 Qumranet Technologies
+ *
+ * Licensed under the terms of the GNU GPL version 2 or higher.
+ */
+#ifndef QEMU_KVM_X86
+#define QEMU_KVM_X86
+
+void kvm_load_tsc(CPUState *env);
+
+#endif
qemu/target-i386/machine.c +2 -0
Index: kvm-userspace.tip/qemu/target-i386/machine.c
===================================================================
@@ -5,6 +5,7 @@
 
 #include "exec-all.h"
 #include "qemu-kvm.h"
+#include "qemu-kvm-x86.h"
 
 void register_machines(void)
 {

@@ -328,6 +329,7 @@ int cpu_load(QEMUFile *f, void *opaque, 
         }
         qemu_get_be64s(f, &env->tsc);
         kvm_load_registers(env);
+        kvm_load_tsc(env);
         if (version_id >= 5) {
             qemu_get_be32s(f, &env->mp_state);
             kvm_load_mpstate(env);
#2 lore raw

Re: [patch 1/3] QEMU/KVM: x86: separate TSC load from kvm_arch_load_regs

Quoted 10 lines · Marcelo Tosatti

kvm_load_registers is a general interface to load registers, and is used by vmport, gdbstub, etc. The TSC MSR is continually counting, so it can't be simply read and written back as the other registers/MSR's (doing so overwrites the current count).

Introduce kvm_load_tsc and use it for x86's migration CPU load code.

qemu/qemu-kvm-x86.h line 4

Quoted 4 lines
qemu/qemu-kvm-x86.h new file +4 -0
@@ -0,0 +1,13 @@
+/*
+ * qemu/kvm integration
+ *
+ * Copyright (C) 2006-2008 Qumranet Technologies

Qumranet is no more. But please drop this header and just put the declaration in qemu-kvm.h. We're merging it anyway into qemu upstream and this is just more more thing we'll need to drop.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#3 lore raw

PATCH 2/3 QEMU/KVM: BIOS: revert TSC zeroing

In reply to Marcelo Tosatti

The TSC is zeroed at RESET, and not at SMP initialization.

This avoids the TSC from going out of sync between vcpu's on SMP guests.

Signed-off-by Marcelo Tosatti <[email protected]>
bios/rombios32.c +0 -6
Index: kvm-userspace.tip/bios/rombios32.c
===================================================================
@@ -610,12 +610,6 @@ void smp_probe(void)
         writel(APIC_BASE + APIC_ICR_LOW, 0x000C4500);
         sipi_vector = AP_BOOT_ADDR >> 12;
         writel(APIC_BASE + APIC_ICR_LOW, 0x000C4600 | sipi_vector);
-	asm volatile(
-		     "xor %%eax, %%eax \n\t"
-		     "xor %%edx, %%edx \n\t"
-		     "mov $0x10, %%ecx \n\t"
-		     "wrmsr"
-		     : : : "eax", "ecx", "edx");
 
 #ifndef BX_QEMU
         delay_ms(10);
bios/rombios32start.S +0 -4
Index: kvm-userspace.tip/bios/rombios32start.S
===================================================================
@@ -43,10 +43,6 @@ smp_ap_boot_code_start:
   cli
   xor %ax, %ax
   mov %ax, %ds
-  xor %eax, %eax
-  xor %edx, %edx
-  mov $0x10, %ecx
-  wrmsr
 
   mov $SMP_MSR_ADDR, %ebx
 11:
author
#4 lore raw

PATCH 3/3 KVM: VMX: initialize TSC offset relative to vm creation time

In reply to Marcelo Tosatti

VMX initializes the TSC offset for each vcpu at different times, and also reinitializes it for vcpus other than 0 on APIC SIPI message.

This bug causes the TSC's to appear unsynchronized in the guest, even if the host is good.

Older Linux kernels don't handle the situation very well, so gettimeofday is likely to go backwards in time:

http://www.mail-archive.com/[email protected]/msg02955.html
http://sourceforge.net/tracker/index.php?func=detail&aid=2025534&group_id=180599&atid=893831

Fix it by initializating the offset of each vcpu relative to vm creation time, and moving it from vmx_vcpu_reset to vmx_vcpu_setup, out of the APIC MP init path.

Signed-off-by Marcelo Tosatti <[email protected]>
arch/x86/include/asm/kvm_host.h +1 -0
Index: kvm/arch/x86/include/asm/kvm_host.h
===================================================================
@@ -378,6 +378,7 @@ struct kvm_arch{
 
 	unsigned long irq_sources_bitmap;
 	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
+	u64 vm_init_tsc;
 };
 
 struct kvm_vm_stat {
arch/x86/kvm/vmx.c +5 -7
Index: kvm/arch/x86/kvm/vmx.c
===================================================================
@@ -856,11 +856,8 @@ static u64 guest_read_tsc(void)
  * writes 'guest_tsc' into guest's timestamp counter "register"
  * guest_tsc = host_tsc + tsc_offset ==> tsc_offset = guest_tsc - host_tsc
  */
-static void guest_write_tsc(u64 guest_tsc)
+static void guest_write_tsc(u64 guest_tsc, u64 host_tsc)
 {
-	u64 host_tsc;
-
-	rdtscll(host_tsc);
 	vmcs_write64(TSC_OFFSET, guest_tsc - host_tsc);
 }
 

@@ -924,6 +921,7 @@ static int vmx_set_msr(struct kvm_vcpu *
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	struct kvm_msr_entry *msr;
+	u64 host_tsc;
 	int ret = 0;
 
 	switch (msr_index) {

@@ -949,7 +947,8 @@ static int vmx_set_msr(struct kvm_vcpu *
 		vmcs_writel(GUEST_SYSENTER_ESP, data);
 		break;
 	case MSR_IA32_TIME_STAMP_COUNTER:
-		guest_write_tsc(data);
+		rdtscll(host_tsc);
+		guest_write_tsc(data, host_tsc);
 		break;
 	case MSR_P6_PERFCTR0:
 	case MSR_P6_PERFCTR1:

@@ -2239,6 +2238,7 @@ static int vmx_vcpu_setup(struct vcpu_vm
 	vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL);
 	vmcs_writel(CR4_GUEST_HOST_MASK, KVM_GUEST_CR4_MASK);
 
+	guest_write_tsc(0, vmx->vcpu.kvm->arch.vm_init_tsc);
 
 	return 0;
 }

@@ -2331,8 +2331,6 @@ static int vmx_vcpu_reset(struct kvm_vcp
 	vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, 0);
 	vmcs_write32(GUEST_PENDING_DBG_EXCEPTIONS, 0);
 
-	guest_write_tsc(0);
-
 	/* Special registers */
 	vmcs_write64(GUEST_IA32_DEBUGCTL, 0);
arch/x86/kvm/x86.c +2 -0
Index: kvm/arch/x86/kvm/x86.c
===================================================================
@@ -4122,6 +4122,8 @@ struct  kvm *kvm_arch_create_vm(void)
 	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
 	set_bit(KVM_USERSPACE_IRQ_SOURCE_ID, &kvm->arch.irq_sources_bitmap);
 
+	rdtscll(kvm->arch.vm_init_tsc);
+
 	return kvm;
 }
#5 lore raw

Re: [patch 3/3] KVM: VMX: initialize TSC offset relative to vm creation time

Quoted 9 lines · Marcelo Tosatti

VMX initializes the TSC offset for each vcpu at different times, and also reinitializes it for vcpus other than 0 on APIC SIPI message.

This bug causes the TSC's to appear unsynchronized in the guest, even if the host is good.

Older Linux kernels don't handle the situation very well, so gettimeofday is likely to go backwards in time:

Quoted 7 lines

http://www.mail-archive.com/[email protected]/msg02955.html
http://sourceforge.net/tracker/index.php?func=detail&aid=2025534&group_id=180599&atid=893831

Fix it by initializating the offset of each vcpu relative to vm creation time, and moving it from vmx_vcpu_reset to vmx_vcpu_setup, out of the APIC MP init path.

This looks fine, but have you tested it on a host with unsync tsc? I'm worried that we'll get regressions there even on uniprocessor guest. I'd like to keep the current behaviour for the special case of uniprocessor guest on unsync tsc host.

There's a further improvement possible: treating the tsc as shared among all vcpus, so that a guest write to one will update all of them, like on a hyperthreaded core. This will prevent the unsyncing caused by the host trying to sync tscs, and the vcpu being scheduled away at the same time.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#6 lore raw

Re: [patch 3/3] KVM: VMX: initialize TSC offset relative to vm creation time

Quoted 18 lines · Avi Kivity
Quoted 9 lines · Marcelo Tosatti

VMX initializes the TSC offset for each vcpu at different times, and also reinitializes it for vcpus other than 0 on APIC SIPI message.

This bug causes the TSC's to appear unsynchronized in the guest, even if the host is good.

Older Linux kernels don't handle the situation very well, so gettimeofday is likely to go backwards in time:

Quoted 7 lines

http://www.mail-archive.com/[email protected]/msg02955.html
http://sourceforge.net/tracker/index.php?func=detail&aid=2025534&group_id=180599&atid=893831

Fix it by initializating the offset of each vcpu relative to vm creation time, and moving it from vmx_vcpu_reset to vmx_vcpu_setup, out of the APIC MP init path.

Quoted 4 lines

This looks fine, but have you tested it on a host with unsync tsc? I'm worried that we'll get regressions there even on uniprocessor guest. I'd like to keep the current behaviour for the special case of uniprocessor guest on unsync tsc host.

I don't see how. For UP guests the TSC is initialized to zero during vcpu setup, similarly to the current behaviour.

Can you explain?

Quoted 5 lines

There's a further improvement possible: treating the tsc as shared among all vcpus, so that a guest write to one will update all of them, like on a hyperthreaded core. This will prevent the unsyncing caused by the host trying to sync tscs, and the vcpu being scheduled away at the same time.

You mean the guest trying to sync tscs? Indeed, that is sensitive to scheduling (like most of the calibration algorithms that are time sensitive). Your suggestion differs from the behaviour of actual hardware, though.

Anyway, this patch is trivial, and an improvement compared to the current situation.

What are the possible implications of simply not zeroing the guest TSC ? Guests will have access to the hosts TSC value, but how bad that is?

#7 lore raw

Re: [patch 3/3] KVM: VMX: initialize TSC offset relative to vm creation time

Quoted 10 lines · Marcelo Tosatti
Quoted 3 lines

This looks fine, but have you tested it on a host with unsync tsc? I'm worried that we'll get regressions there even on uniprocessor guest. I'd like to keep the current behaviour for the special case of
uniprocessor guest on unsync tsc host.

I don't see how. For UP guests the TSC is initialized to zero during vcpu setup, similarly to the current behaviour.

Can you explain?

On a host with an unsync tsc, when you move the vcpu to another cpu, the tsc may jump backwards.

Quoted 6 lines
Quoted 2 lines

There's a further improvement possible: treating the tsc as shared among all vcpus, so that a guest write to one will update all of them, like on a hyperthreaded core. This will prevent the unsyncing caused by the host trying to sync tscs, and the vcpu being scheduled away at the same time.

You mean the guest trying to sync tscs? Indeed, that is sensitive to scheduling (like most of the calibration algorithms that are time sensitive).

Yes.

Quoted 3 lines

Your suggestion differs from the behaviour of actual hardware, though.

It's similar to how hyperthreaded cpus work, so there is precedent.

Quoted 3 lines

Anyway, this patch is trivial, and an improvement compared to the current situation.

I'm worried that it will regress for single vcpu guests on unsync tsc hosts.

Quoted 3 lines

What are the possible implications of simply not zeroing the guest TSC ? Guests will have access to the hosts TSC value, but how bad that is?

We need to set the guest tsc anyway for live migration, so we may as well reset it on startup. It's not a correctness issue, just seems cleaner that way.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#8 lore raw

Re: [patch 3/3] KVM: VMX: initialize TSC offset relative to vm creation time

Quoted 14 lines · Avi Kivity
Quoted 10 lines · Marcelo Tosatti
Quoted 6 lines

This looks fine, but have you tested it on a host with unsync tsc? I'm worried that we'll get regressions there even on uniprocessor
guest. I'd like to keep the current behaviour for the special case
of
uniprocessor guest on unsync tsc host.

I don't see how. For UP guests the TSC is initialized to zero during vcpu setup, similarly to the current behaviour.

Quoted 6 lines
Quoted 3 lines

Can you explain?

On a host with an unsync tsc, when you move the vcpu to another cpu, the tsc may jump backwards.

Ok, this could cause the guest tsc to be initialized to a high value close to wraparound (in case the vcpu is migrated to a cpu with negative difference before vmx_vcpu_setup). What other regression could the updated patch introduce?

arch/x86/include/asm/kvm_host.h +1 -0
index 97215a4..5b70d83 100644
@@ -378,6 +378,7 @@ struct kvm_arch{
 
 	unsigned long irq_sources_bitmap;
 	unsigned long irq_states[KVM_IOAPIC_NUM_PINS];
+	u64 vm_init_tsc;
 };
 
 struct kvm_vm_stat {
arch/x86/kvm/vmx.c +11 -8
index e446f23..0879852 100644
@@ -856,11 +856,8 @@ static u64 guest_read_tsc(void)
  * writes 'guest_tsc' into guest's timestamp counter "register"
  * guest_tsc = host_tsc + tsc_offset ==> tsc_offset = guest_tsc - host_tsc
  */
-static void guest_write_tsc(u64 guest_tsc)
+static void guest_write_tsc(u64 guest_tsc, u64 host_tsc)
 {
-	u64 host_tsc;
-
-	rdtscll(host_tsc);
 	vmcs_write64(TSC_OFFSET, guest_tsc - host_tsc);
 }
 

@@ -924,6 +921,7 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	struct kvm_msr_entry *msr;
+	u64 host_tsc;
 	int ret = 0;
 
 	switch (msr_index) {

@@ -949,7 +947,8 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
 		vmcs_writel(GUEST_SYSENTER_ESP, data);
 		break;
 	case MSR_IA32_TIME_STAMP_COUNTER:
-		guest_write_tsc(data);
+		rdtscll(host_tsc);
+		guest_write_tsc(data, host_tsc);
 		break;
 	case MSR_P6_PERFCTR0:
 	case MSR_P6_PERFCTR1:

@@ -2111,7 +2110,7 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
 {
 	u32 host_sysenter_cs, msr_low, msr_high;
 	u32 junk;
-	u64 host_pat;
+	u64 host_pat, tsc_this, tsc_base;
 	unsigned long a;
 	struct descriptor_table dt;
 	int i;

@@ -2239,6 +2238,12 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
 	vmcs_writel(CR0_GUEST_HOST_MASK, ~0UL);
 	vmcs_writel(CR4_GUEST_HOST_MASK, KVM_GUEST_CR4_MASK);
 
+	tsc_base = vmx->vcpu.kvm->arch.vm_init_tsc;
+	rdtscll(tsc_this);
+	if (tsc_this < vmx->vcpu.kvm->arch.vm_init_tsc)
+		tsc_base = tsc_this;
+
+	guest_write_tsc(0, tsc_base);
 
 	return 0;
 }

@@ -2331,8 +2336,6 @@ static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 	vmcs_write32(GUEST_INTERRUPTIBILITY_INFO, 0);
 	vmcs_write32(GUEST_PENDING_DBG_EXCEPTIONS, 0);
 
-	guest_write_tsc(0);
-
 	/* Special registers */
 	vmcs_write64(GUEST_IA32_DEBUGCTL, 0);
arch/x86/kvm/x86.c +2 -0
index ba10287..b2d64eb 100644
@@ -4122,6 +4122,8 @@ struct  kvm *kvm_arch_create_vm(void)
 	/* Reserve bit 0 of irq_sources_bitmap for userspace irq source */
 	set_bit(KVM_USERSPACE_IRQ_SOURCE_ID, &kvm->arch.irq_sources_bitmap);
 
+	rdtscll(kvm->arch.vm_init_tsc);
+
 	return kvm;
 }
Applied
#9 lore raw

Re: [patch 3/3] KVM: VMX: initialize TSC offset relative to vm creation time

Quoted 3 lines · Marcelo Tosatti

Ok, this could cause the guest tsc to be initialized to a high value close to wraparound (in case the vcpu is migrated to a cpu with negative difference before vmx_vcpu_setup).

Applied, thanks.

Signature · 1 line
error compiling committee.c: too many arguments to function
#10 lore raw
In reply to Marcelo Tosatti
Quoted 3 lines · Marcelo Tosatti

Most Intel hosts are supposed to have their TSC's synchronized. This patchset attempts to fix the sites which overwrite the TSC making them appear unsynchronized to the guest.

By the way, phenoms are also supposed to have synced TSCs, at least the host reports TSC syncronization works. But guests still thinks TSC is unstable, even on uniprocessor guests using kvm_clock...

Is it related to the area you're patching?

Thanks!

/mjt

author
#11 lore raw
Quoted 8 lines · Michael Tokarev
Quoted 3 lines · Marcelo Tosatti

Most Intel hosts are supposed to have their TSC's synchronized. This patchset attempts to fix the sites which overwrite the TSC making them appear unsynchronized to the guest.

By the way, phenoms are also supposed to have synced TSCs, at least the host reports TSC syncronization works. But guests still thinks TSC is unstable, even on uniprocessor guests using kvm_clock...

On uniprocessor guests? Perhaps, if the guest uses the vmmouse for example. In such case, is the TSC reported unstable right after boot?

Quoted 10 lines

Is it related to the area you're patching?

Thanks!

/mjt

--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

#12 lore raw
Quoted 11 lines · Marcelo Tosatti
Quoted 7 lines · Michael Tokarev
Quoted 3 lines · Marcelo Tosatti

Most Intel hosts are supposed to have their TSC's synchronized. This patchset attempts to fix the sites which overwrite the TSC making them appear unsynchronized to the guest.

By the way, phenoms are also supposed to have synced TSCs, at least the host reports TSC syncronization works. But guests still thinks TSC is unstable, even on uniprocessor guests using kvm_clock...

On uniprocessor guests? Perhaps, if the guest uses the vmmouse for example. In such case, is the TSC reported unstable right after boot?

Hm. I see "TSC unsyncronized" messages on a bare linux-2.6.27 guests, without anything but virtio_{blk,net} loaded. "Uniprocessor guests" - I mean, a guest that has only one CPU, not necessary kernel that only supports one CPU.

Here are some lines from dmesg when booting a guest on a phenom machine:

Dec  4 19:17:42 iserv kernel: kvm-clock: cpu 0, msr 0:3d0801, boot clock
Dec  4 19:17:42 iserv kernel: SMP: Allowing 8 CPUs, 7 hotplug CPUs
Dec  4 19:17:42 iserv kernel: NR_CPUS: 8, nr_cpu_ids: 8, nr_node_ids 1
Dec  4 19:17:42 iserv kernel: kvm-clock: cpu 0, msr 0:180b801, primary cpu clock
Dec  4 19:17:42 iserv kernel: TSC: Unable to calibrate against PIT
Dec  4 19:17:42 iserv kernel: TSC: using PMTIMER reference calibration
Dec  4 19:17:42 iserv kernel: SMP alternatives: switching to UP code
Dec  4 19:17:42 iserv kernel: Booting paravirtualized kernel on KVM
Dec  4 19:17:42 iserv kernel: Processing INITRAMFS
Dec  4 19:17:42 iserv kernel: loading module virtio_pci
Dec  4 19:17:42 iserv kernel: EXT3 FS on vda, internal journal
Dec  4 19:26:24 iserv kernel: Clocksource tsc unstable (delta = -126732353 ns)

(Note the time difference between last and all previous lines.

At this time, there are only 5 user processes running. Like this:

    1 ?        Ss     0:04 init [2]
  791 ?        Ss     0:00 /sbin/syslogd
  800 ?        Ss     0:00 /sbin/klogd -x
  845 ?        Ss     0:00 /usr/sbin/unbound
 6423 tty1     Ss+    0:00 /sbin/getty 38400 tty1

(it's not real example but close).

Linux iserv 2.6.27-i686smp #2.6.27.7 SMP Fri Nov 21 13:24:19 MSK 2008 i686 GNU/Linux kvm-79.

Thanks!

/mjt

#13 lore raw
In reply to Marcelo Tosatti

Hi Marcelo:

I just found time to try out this patch set. I applied it to kvm-81, and it fixes the time shifts I've observed on DL380-G5s.

david

Quoted 4 lines · Marcelo Tosatti

Most Intel hosts are supposed to have their TSC's synchronized. This patchset attempts to fix the sites which overwrite the TSC making them appear unsynchronized to the guest.

#14 lore raw
In reply to Marcelo Tosatti
Quoted 4 lines · Marcelo Tosatti

Most Intel hosts are supposed to have their TSC's synchronized. This patchset attempts to fix the sites which overwrite the TSC making them appear unsynchronized to the guest.

Applied the userspace bits as well. I dropped qemu-kvm-x86.h; these files are going away, so we shouldn't add more.

Signature · 1 line
error compiling committee.c: too many arguments to function