[PATCH 0/6] TSC scaling support for KVM v3

[PATCH 0/6] TSC scaling support for KVM v3

[Joerg Roedel]

Hi,

this is the third round of my patches to support tsc-scaling in KVM. The
changes to v2 address Avi's comments from yesterday. Besides that the
whole virtual_tsc_khz thing has been moved out of the vm into the vcpu
data structure. The mult and shift parts where renamed to tsc_catchup_*
because this is their actual use (and because the handling of
virtual_tsc_khz has changed so that is made sense to seperate them).

Comments and feedback (or merging) appreciated :-)

Regards,

	Joerg

Diffstat:

 Documentation/kvm/api.txt        |   23 ++++++++
 arch/x86/include/asm/kvm_host.h  |   16 ++++-
 arch/x86/include/asm/msr-index.h |    1 +
 arch/x86/kvm/svm.c               |  117 +++++++++++++++++++++++++++++++++++++-
 arch/x86/kvm/vmx.c               |   17 ++++++
 arch/x86/kvm/x86.c               |   79 ++++++++++++++++++++------
 include/linux/kvm.h              |    5 ++
 7 files changed, 237 insertions(+), 21 deletions(-)

Shortlog:

Joerg Roedel (6):
      KVM: SVM: Implement infrastructure for TSC_RATE_MSR
      KVM: X86: Let kvm-clock report the right tsc frequency
      KVM: X86: Make tsc_delta calculation a function of guest tsc
      KVM: X86: Implement call-back to propagate virtual_tsc_khz
      KVM: X86: Delegate tsc-offset calculation to architecture code
      KVM: X86: Implement userspace interface to set virtual_tsc_khz

[PATCH 1/6] KVM: SVM: Implement infrastructure for TSC_RATE_MSR

[Joerg Roedel]

This patch enhances the kvm_amd module with functions to
support the TSC_RATE_MSR which can be used to set a given
tsc frequency for the guest vcpu.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/msr-index.h |    1 +
 arch/x86/kvm/svm.c               |   54 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/arch/x86/include/asm/msr-index.h b/arch/x86/include/asm/msr-index.h
index fd5a1f3..a7b3e40 100644
--- a/arch/x86/include/asm/msr-index.h
+++ b/arch/x86/include/asm/msr-index.h
@@ -114,6 +114,7 @@
    complete list. */
 
 #define MSR_AMD64_PATCH_LEVEL		0x0000008b
+#define MSR_AMD64_TSC_RATIO		0xc0000104
 #define MSR_AMD64_NB_CFG		0xc001001f
 #define MSR_AMD64_PATCH_LOADER		0xc0010020
 #define MSR_AMD64_OSVW_ID_LENGTH	0xc0010140
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 2a19322..2ce734c 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -63,6 +63,8 @@ MODULE_LICENSE("GPL");
 
 #define DEBUGCTL_RESERVED_BITS (~(0x3fULL))
 
+#define TSC_RATIO_RSVD          0xffffff0000000000ULL
+
 static bool erratum_383_found __read_mostly;
 
 static const u32 host_save_user_msrs[] = {
@@ -144,8 +146,13 @@ struct vcpu_svm {
 	unsigned int3_injected;
 	unsigned long int3_rip;
 	u32 apf_reason;
+
+	u64  tsc_ratio;
 };
 
+static DEFINE_PER_CPU(u64, current_tsc_ratio);
+#define TSC_RATIO_DEFAULT	0x0100000000ULL
+
 #define MSR_INVALID			0xffffffffU
 
 static struct svm_direct_access_msrs {
@@ -569,6 +576,10 @@ static int has_svm(void)
 
 static void svm_hardware_disable(void *garbage)
 {
+	/* Make sure we clean up behind us */
+	if (static_cpu_has(X86_FEATURE_TSCRATEMSR))
+		wrmsrl(MSR_AMD64_TSC_RATIO, TSC_RATIO_DEFAULT);
+
 	cpu_svm_disable();
 }
 
@@ -610,6 +621,11 @@ static int svm_hardware_enable(void *garbage)
 
 	wrmsrl(MSR_VM_HSAVE_PA, page_to_pfn(sd->save_area) << PAGE_SHIFT);
 
+	if (static_cpu_has(X86_FEATURE_TSCRATEMSR)) {
+		wrmsrl(MSR_AMD64_TSC_RATIO, TSC_RATIO_DEFAULT);
+		__get_cpu_var(current_tsc_ratio) = TSC_RATIO_DEFAULT;
+	}
+
 	svm_init_erratum_383();
 
 	return 0;
@@ -854,6 +870,32 @@ static void init_sys_seg(struct vmcb_seg *seg, uint32_t type)
 	seg->base = 0;
 }
 
+static u64 __scale_tsc(u64 ratio, u64 tsc)
+{
+	u64 mult, frac, _tsc;
+
+	mult  = ratio >> 32;
+	frac  = ratio & ((1ULL << 32) - 1);
+
+	_tsc  = tsc;
+	_tsc *= mult;
+	_tsc += (tsc >> 32) * frac;
+	_tsc += ((tsc & ((1ULL << 32) - 1)) * frac) >> 32;
+
+	return _tsc;
+}
+
+static u64 svm_scale_tsc(struct kvm_vcpu *vcpu, u64 tsc)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+	u64 _tsc = tsc;
+
+	if (svm->tsc_ratio != TSC_RATIO_DEFAULT)
+		_tsc = __scale_tsc(svm->tsc_ratio, tsc);
+
+	return _tsc;
+}
+
 static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
@@ -1048,6 +1090,8 @@ static struct kvm_vcpu *svm_create_vcpu(struct kvm *kvm, unsigned int id)
 		goto out;
 	}
 
+	svm->tsc_ratio = TSC_RATIO_DEFAULT;
+
 	err = kvm_vcpu_init(&svm->vcpu, kvm, id);
 	if (err)
 		goto free_svm;
@@ -1141,6 +1185,12 @@ static void svm_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 	for (i = 0; i < NR_HOST_SAVE_USER_MSRS; i++)
 		rdmsrl(host_save_user_msrs[i], svm->host_user_msrs[i]);
+
+	if (static_cpu_has(X86_FEATURE_TSCRATEMSR) &&
+	    svm->tsc_ratio != __get_cpu_var(current_tsc_ratio)) {
+		__get_cpu_var(current_tsc_ratio) = svm->tsc_ratio;
+		wrmsrl(MSR_AMD64_TSC_RATIO, svm->tsc_ratio);
+	}
 }
 
 static void svm_vcpu_put(struct kvm_vcpu *vcpu)
@@ -2813,7 +2863,9 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, unsigned ecx, u64 *data)
 	case MSR_IA32_TSC: {
 		struct vmcb *vmcb = get_host_vmcb(svm);
 
-		*data = vmcb->control.tsc_offset + native_read_tsc();
+		*data = vmcb->control.tsc_offset +
+			svm_scale_tsc(vcpu, native_read_tsc());
+
 		break;
 	}
 	case MSR_STAR:
-- 
1.7.1

[PATCH 2/6] KVM: X86: Let kvm-clock report the right tsc frequency

[Joerg Roedel]

This patch changes the kvm_guest_time_update function to use
TSC frequency the guest actually has for updating its clock.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_host.h |    6 +++---
 arch/x86/kvm/x86.c              |   25 +++++++++++++++----------
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 35f81b1..0344b94 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -380,7 +380,10 @@ struct kvm_vcpu_arch {
 	u64 last_kernel_ns;
 	u64 last_tsc_nsec;
 	u64 last_tsc_write;
+	u32 virtual_tsc_khz;
 	bool tsc_catchup;
+	u32  tsc_catchup_mult;
+	s8   tsc_catchup_shift;
 
 	bool nmi_pending;
 	bool nmi_injected;
@@ -450,9 +453,6 @@ struct kvm_arch {
 	u64 last_tsc_nsec;
 	u64 last_tsc_offset;
 	u64 last_tsc_write;
-	u32 virtual_tsc_khz;
-	u32 virtual_tsc_mult;
-	s8 virtual_tsc_shift;
 
 	struct kvm_xen_hvm_config xen_hvm_config;
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1b8b16a..1e7af86 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -982,6 +982,14 @@ static inline int kvm_tsc_changes_freq(void)
 	return ret;
 }
 
+static u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu)
+{
+	if (vcpu->arch.virtual_tsc_khz)
+		return vcpu->arch.virtual_tsc_khz;
+	else
+		return __this_cpu_read(cpu_tsc_khz);
+}
+
 static inline u64 nsec_to_cycles(u64 nsec)
 {
 	u64 ret;
@@ -995,20 +1003,19 @@ static inline u64 nsec_to_cycles(u64 nsec)
 	return ret;
 }
 
-static void kvm_arch_set_tsc_khz(struct kvm *kvm, u32 this_tsc_khz)
+static void kvm_init_tsc_catchup(struct kvm_vcpu *vcpu, u32 this_tsc_khz)
 {
 	/* Compute a scale to convert nanoseconds in TSC cycles */
 	kvm_get_time_scale(this_tsc_khz, NSEC_PER_SEC / 1000,
-			   &kvm->arch.virtual_tsc_shift,
-			   &kvm->arch.virtual_tsc_mult);
-	kvm->arch.virtual_tsc_khz = this_tsc_khz;
+			   &vcpu->arch.tsc_catchup_shift,
+			   &vcpu->arch.tsc_catchup_mult);
 }
 
 static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns)
 {
 	u64 tsc = pvclock_scale_delta(kernel_ns-vcpu->arch.last_tsc_nsec,
-				      vcpu->kvm->arch.virtual_tsc_mult,
-				      vcpu->kvm->arch.virtual_tsc_shift);
+				      vcpu->arch.tsc_catchup_mult,
+				      vcpu->arch.tsc_catchup_shift);
 	tsc += vcpu->arch.last_tsc_write;
 	return tsc;
 }
@@ -1075,8 +1082,7 @@ static int kvm_guest_time_update(struct kvm_vcpu *v)
 	local_irq_save(flags);
 	kvm_get_msr(v, MSR_IA32_TSC, &tsc_timestamp);
 	kernel_ns = get_kernel_ns();
-	this_tsc_khz = __this_cpu_read(cpu_tsc_khz);
-
+	this_tsc_khz = vcpu_tsc_khz(v);
 	if (unlikely(this_tsc_khz == 0)) {
 		local_irq_restore(flags);
 		kvm_make_request(KVM_REQ_CLOCK_UPDATE, v);
@@ -5955,8 +5961,7 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 	}
 	vcpu->arch.pio_data = page_address(page);
 
-	if (!kvm->arch.virtual_tsc_khz)
-		kvm_arch_set_tsc_khz(kvm, max_tsc_khz);
+	kvm_init_tsc_catchup(vcpu, max_tsc_khz);
 
 	r = kvm_mmu_create(vcpu);
 	if (r < 0)
-- 
1.7.1

[PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Joerg Roedel]

The calculation of the tsc_delta value to ensure a
forward-going tsc for the guest is a function of the
host-tsc. This works as long as the guests tsc_khz is equal
to the hosts tsc_khz. With tsc-scaling hardware support this
is not longer true and the tsc_delta needs to be calculated
using guest_tsc values.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/x86.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1e7af86..47dd6ed 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2126,8 +2126,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	kvm_x86_ops->vcpu_load(vcpu, cpu);
 	if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) {
 		/* Make sure TSC doesn't go backwards */
-		s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
-				native_read_tsc() - vcpu->arch.last_host_tsc;
+		s64 tsc_delta;
+		u64 tsc;
+
+		kvm_get_msr(vcpu, MSR_IA32_TSC, &tsc);
+		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
+			     tsc - vcpu->arch.last_guest_tsc;
+
 		if (tsc_delta < 0)
 			mark_tsc_unstable("KVM discovered backwards TSC");
 		if (check_tsc_unstable()) {
-- 
1.7.1

[PATCH 4/6] KVM: X86: Implement call-back to propagate virtual_tsc_khz

[Joerg Roedel]

This patch implements a call-back into the architecture code
to allow the propagation of changes to the virtual tsc_khz
of the vcpu.
On SVM it updates the tsc_ratio variable, on VMX it does
nothing.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/svm.c              |   33 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/vmx.c              |   11 +++++++++++
 3 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 0344b94..9958dd8 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -588,6 +588,7 @@ struct kvm_x86_ops {
 
 	bool (*has_wbinvd_exit)(void);
 
+	void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz);
 	void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
 	void (*get_exit_info)(struct kvm_vcpu *vcpu, u64 *info1, u64 *info2);
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 2ce734c..f6d66c2 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -896,6 +896,38 @@ static u64 svm_scale_tsc(struct kvm_vcpu *vcpu, u64 tsc)
 	return _tsc;
 }
 
+static void svm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+	u64 ratio;
+	u64 khz;
+
+	/* TSC scaling supported? */
+	if (!boot_cpu_has(X86_FEATURE_TSCRATEMSR))
+		return;
+
+	/* TSC-Scaling disabled or guest TSC same frequency as host TSC? */
+	if (user_tsc_khz == 0) {
+		vcpu->arch.virtual_tsc_khz = 0;
+		svm->tsc_ratio = TSC_RATIO_DEFAULT;
+		return;
+	}
+
+	khz = user_tsc_khz;
+
+	/* TSC scaling required  - calculate ratio */
+	ratio = khz << 32;
+	do_div(ratio, tsc_khz);
+
+	if (ratio == 0 || ratio & TSC_RATIO_RSVD) {
+		WARN_ONCE(1, "Invalid TSC ratio - virtual-tsc-khz=%u\n",
+				user_tsc_khz);
+		return;
+	}
+	vcpu->arch.virtual_tsc_khz = user_tsc_khz;
+	svm->tsc_ratio             = ratio;
+}
+
 static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);
@@ -4004,6 +4036,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 
 	.has_wbinvd_exit = svm_has_wbinvd_exit,
 
+	.set_tsc_khz = svm_set_tsc_khz,
 	.write_tsc_offset = svm_write_tsc_offset,
 	.adjust_tsc_offset = svm_adjust_tsc_offset,
 
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 1bdb49d..0e5dfc6 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1161,6 +1161,16 @@ static u64 guest_read_tsc(void)
 }
 
 /*
+ * Empty call-back. Needs to be implemented when VMX enables the SET_TSC_KHZ
+ * ioctl. In this case the call-back should update internal vmx state to make
+ * the changes effective.
+ */
+static void vmx_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz)
+{
+	/* Nothing to do here */
+}
+
+/*
  * writes 'offset' into guest's timestamp counter offset register
  */
 static void vmx_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
@@ -4496,6 +4506,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 
 	.has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
 
+	.set_tsc_khz = vmx_set_tsc_khz,
 	.write_tsc_offset = vmx_write_tsc_offset,
 	.adjust_tsc_offset = vmx_adjust_tsc_offset,
 
-- 
1.7.1

[PATCH 5/6] KVM: X86: Delegate tsc-offset calculation to architecture code

[Joerg Roedel]

With TSC scaling in SVM the tsc-offset needs to be
calculated differently. This patch propagates this
calculation into the architecture specific modules so that
this complexity can be handled there.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/include/asm/kvm_host.h |    2 ++
 arch/x86/kvm/svm.c              |   10 ++++++++++
 arch/x86/kvm/vmx.c              |    6 ++++++
 arch/x86/kvm/x86.c              |   10 +++++-----
 4 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 9958dd8..7f48528 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -591,6 +591,8 @@ struct kvm_x86_ops {
 	void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz);
 	void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
+	u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);
+
 	void (*get_exit_info)(struct kvm_vcpu *vcpu, u64 *info1, u64 *info2);
 	const struct trace_print_flags *exit_reasons_str;
 };
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index f6d66c2..38a4bcc 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -954,6 +954,15 @@ static void svm_adjust_tsc_offset(struct kvm_vcpu *vcpu, s64 adjustment)
 	mark_dirty(svm->vmcb, VMCB_INTERCEPTS);
 }
 
+static u64 svm_compute_tsc_offset(struct kvm_vcpu *vcpu, u64 target_tsc)
+{
+	u64 tsc;
+
+	tsc = svm_scale_tsc(vcpu, native_read_tsc());
+
+	return target_tsc - tsc;
+}
+
 static void init_vmcb(struct vcpu_svm *svm)
 {
 	struct vmcb_control_area *control = &svm->vmcb->control;
@@ -4039,6 +4048,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.set_tsc_khz = svm_set_tsc_khz,
 	.write_tsc_offset = svm_write_tsc_offset,
 	.adjust_tsc_offset = svm_adjust_tsc_offset,
+	.compute_tsc_offset = svm_compute_tsc_offset,
 
 	.set_tdp_cr3 = set_tdp_cr3,
 };
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 0e5dfc6..c4f077a 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -1184,6 +1184,11 @@ static void vmx_adjust_tsc_offset(struct kvm_vcpu *vcpu, s64 adjustment)
 	vmcs_write64(TSC_OFFSET, offset + adjustment);
 }
 
+static u64 vmx_compute_tsc_offset(struct kvm_vcpu *vcpu, u64 target_tsc)
+{
+	return target_tsc - native_read_tsc();
+}
+
 /*
  * Reads an msr value (of 'msr_index') into 'pdata'.
  * Returns 0 on success, non-0 otherwise.
@@ -4509,6 +4514,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.set_tsc_khz = vmx_set_tsc_khz,
 	.write_tsc_offset = vmx_write_tsc_offset,
 	.adjust_tsc_offset = vmx_adjust_tsc_offset,
+	.compute_tsc_offset = vmx_compute_tsc_offset,
 
 	.set_tdp_cr3 = vmx_set_cr3,
 };
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 47dd6ed..2f0b552 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -990,7 +990,7 @@ static u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu)
 		return __this_cpu_read(cpu_tsc_khz);
 }
 
-static inline u64 nsec_to_cycles(u64 nsec)
+static inline u64 nsec_to_cycles(struct kvm_vcpu *vcpu, u64 nsec)
 {
 	u64 ret;
 
@@ -998,7 +998,7 @@ static inline u64 nsec_to_cycles(u64 nsec)
 	if (kvm_tsc_changes_freq())
 		printk_once(KERN_WARNING
 		 "kvm: unreliable cycle conversion on adjustable rate TSC\n");
-	ret = nsec * __this_cpu_read(cpu_tsc_khz);
+	ret = nsec * vcpu_tsc_khz(vcpu);
 	do_div(ret, USEC_PER_SEC);
 	return ret;
 }
@@ -1028,7 +1028,7 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data)
 	s64 sdiff;
 
 	raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags);
-	offset = data - native_read_tsc();
+	offset = kvm_x86_ops->compute_tsc_offset(vcpu, data);
 	ns = get_kernel_ns();
 	elapsed = ns - kvm->arch.last_tsc_nsec;
 	sdiff = data - kvm->arch.last_tsc_write;
@@ -1044,13 +1044,13 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, u64 data)
 	 * In that case, for a reliable TSC, we can match TSC offsets,
 	 * or make a best guest using elapsed value.
 	 */
-	if (sdiff < nsec_to_cycles(5ULL * NSEC_PER_SEC) &&
+	if (sdiff < nsec_to_cycles(vcpu, 5ULL * NSEC_PER_SEC) &&
 	    elapsed < 5ULL * NSEC_PER_SEC) {
 		if (!check_tsc_unstable()) {
 			offset = kvm->arch.last_tsc_offset;
 			pr_debug("kvm: matched tsc offset for %llu\n", data);
 		} else {
-			u64 delta = nsec_to_cycles(elapsed);
+			u64 delta = nsec_to_cycles(vcpu, elapsed);
 			offset += delta;
 			pr_debug("kvm: adjusted tsc offset by %llu\n", delta);
 		}
-- 
1.7.1

[PATCH 6/6] KVM: X86: Implement userspace interface to set virtual_tsc_khz

[Joerg Roedel]

This patch implements two new vm-ioctls to get and set the
virtual_tsc_khz if the machine supports tsc-scaling. Setting
the tsc-frequency is only possible before userspace creates
any vcpu.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 Documentation/kvm/api.txt       |   23 +++++++++++++++++++++++
 arch/x86/include/asm/kvm_host.h |    7 +++++++
 arch/x86/kvm/svm.c              |   20 ++++++++++++++++++++
 arch/x86/kvm/x86.c              |   35 +++++++++++++++++++++++++++++++++++
 include/linux/kvm.h             |    5 +++++
 5 files changed, 90 insertions(+), 0 deletions(-)

diff --git a/Documentation/kvm/api.txt b/Documentation/kvm/api.txt
index 9bef4e4..1b9eaa7 100644
--- a/Documentation/kvm/api.txt
+++ b/Documentation/kvm/api.txt
@@ -1263,6 +1263,29 @@ struct kvm_assigned_msix_entry {
 	__u16 padding[3];
 };
 
+4.54 KVM_SET_TSC_KHZ
+
+Capability: KVM_CAP_TSC_CONTROL
+Architectures: x86
+Type: vcpu ioctl
+Parameters: virtual tsc_khz
+Returns: 0 on success, -1 on error
+
+Specifies the tsc frequency for the virtual machine. The unit of the
+frequency is KHz.
+
+4.55 KVM_GET_TSC_KHZ
+
+Capability: KVM_CAP_GET_TSC_KHZ
+Architectures: x86
+Type: vcpu ioctl
+Parameters: none
+Returns: virtual tsc-khz on success, negative value on error
+
+Returns the tsc frequency of the guest. The unit of the return value is
+KHz. If the host has unstable tsc this ioctl returns -EIO instead as an
+error.
+
 5. The kvm_run structure
 
 Application code obtains a pointer to the kvm_run structure by
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 7f48528..473a3be 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -632,6 +632,13 @@ u8 kvm_get_guest_memory_type(struct kvm_vcpu *vcpu, gfn_t gfn);
 
 extern bool tdp_enabled;
 
+/* control of guest tsc rate supported? */
+extern bool kvm_has_tsc_control;
+/* minimum supported tsc_khz for guests */
+extern u32  kvm_min_guest_tsc_khz;
+/* maximum supported tsc_khz for guests */
+extern u32  kvm_max_guest_tsc_khz;
+
 enum emulation_result {
 	EMULATE_DONE,       /* no further processing */
 	EMULATE_DO_MMIO,      /* kvm_run filled with mmio request */
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 38a4bcc..a5c1b5b 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -64,6 +64,8 @@ MODULE_LICENSE("GPL");
 #define DEBUGCTL_RESERVED_BITS (~(0x3fULL))
 
 #define TSC_RATIO_RSVD          0xffffff0000000000ULL
+#define TSC_RATIO_MIN		0x0000000000000001ULL
+#define TSC_RATIO_MAX		0x000000ffffffffffULL
 
 static bool erratum_383_found __read_mostly;
 
@@ -197,6 +199,7 @@ static int nested_svm_intercept(struct vcpu_svm *svm);
 static int nested_svm_vmexit(struct vcpu_svm *svm);
 static int nested_svm_check_exception(struct vcpu_svm *svm, unsigned nr,
 				      bool has_error_code, u32 error_code);
+static u64 __scale_tsc(u64 ratio, u64 tsc);
 
 enum {
 	VMCB_INTERCEPTS, /* Intercept vectors, TSC offset,
@@ -807,6 +810,23 @@ static __init int svm_hardware_setup(void)
 	if (boot_cpu_has(X86_FEATURE_FXSR_OPT))
 		kvm_enable_efer_bits(EFER_FFXSR);
 
+	if (boot_cpu_has(X86_FEATURE_TSCRATEMSR)) {
+		u64 max;
+
+		kvm_has_tsc_control = true;
+
+		/*
+		 * Make sure the user can only configure tsc_khz values that
+		 * fit into a signed integer.
+		 * A min value is not calculated needed because it will always
+		 * be 1 on all machines and a value of 0 is used to disable
+		 * tsc-scaling for the vcpu.
+		 */
+		max = min(0x7fffffffULL, __scale_tsc(tsc_khz, TSC_RATIO_MAX));
+
+		kvm_max_guest_tsc_khz = max;
+	}
+
 	if (nested) {
 		printk(KERN_INFO "kvm: Nested Virtualization enabled\n");
 		kvm_enable_efer_bits(EFER_SVME | EFER_LMSLE);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2f0b552..5cc9a44 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -100,6 +100,11 @@ EXPORT_SYMBOL_GPL(kvm_x86_ops);
 int ignore_msrs = 0;
 module_param_named(ignore_msrs, ignore_msrs, bool, S_IRUGO | S_IWUSR);
 
+bool kvm_has_tsc_control;
+EXPORT_SYMBOL_GPL(kvm_has_tsc_control);
+u32  kvm_max_guest_tsc_khz;
+EXPORT_SYMBOL_GPL(kvm_max_guest_tsc_khz);
+
 #define KVM_NR_SHARED_MSRS 16
 
 struct kvm_shared_msrs_global {
@@ -1999,6 +2004,7 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_X86_ROBUST_SINGLESTEP:
 	case KVM_CAP_XSAVE:
 	case KVM_CAP_ASYNC_PF:
+	case KVM_CAP_GET_TSC_KHZ:
 		r = 1;
 		break;
 	case KVM_CAP_COALESCED_MMIO:
@@ -2025,6 +2031,9 @@ int kvm_dev_ioctl_check_extension(long ext)
 	case KVM_CAP_XCRS:
 		r = cpu_has_xsave;
 		break;
+	case KVM_CAP_TSC_CONTROL:
+		r = kvm_has_tsc_control;
+		break;
 	default:
 		r = 0;
 		break;
@@ -3057,6 +3066,32 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = kvm_vcpu_ioctl_x86_set_xcrs(vcpu, u.xcrs);
 		break;
 	}
+	case KVM_SET_TSC_KHZ: {
+		u32 user_tsc_khz;
+
+		r = -EINVAL;
+		if (!kvm_has_tsc_control)
+			break;
+
+		user_tsc_khz = (u32)arg;
+
+		if (user_tsc_khz >= kvm_max_guest_tsc_khz)
+			goto out;
+
+		kvm_x86_ops->set_tsc_khz(vcpu, user_tsc_khz);
+
+		r = 0;
+		goto out;
+	}
+	case KVM_GET_TSC_KHZ: {
+		r = -EIO;
+		if (check_tsc_unstable())
+			goto out;
+
+		r = vcpu_tsc_khz(vcpu);
+
+		goto out;
+	}
 	default:
 		r = -EINVAL;
 	}
diff --git a/include/linux/kvm.h b/include/linux/kvm.h
index ea2dc1a..2f63ebe 100644
--- a/include/linux/kvm.h
+++ b/include/linux/kvm.h
@@ -541,6 +541,8 @@ struct kvm_ppc_pvinfo {
 #define KVM_CAP_PPC_GET_PVINFO 57
 #define KVM_CAP_PPC_IRQ_LEVEL 58
 #define KVM_CAP_ASYNC_PF 59
+#define KVM_CAP_TSC_CONTROL 60
+#define KVM_CAP_GET_TSC_KHZ 61
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -677,6 +679,9 @@ struct kvm_clock_data {
 #define KVM_SET_PIT2              _IOW(KVMIO,  0xa0, struct kvm_pit_state2)
 /* Available with KVM_CAP_PPC_GET_PVINFO */
 #define KVM_PPC_GET_PVINFO	  _IOW(KVMIO,  0xa1, struct kvm_ppc_pvinfo)
+/* Available with KVM_CAP_TSC_CONTROL */
+#define KVM_SET_TSC_KHZ           _IO(KVMIO,  0xa2)
+#define KVM_GET_TSC_KHZ           _IO(KVMIO,  0xa3)
 
 /*
  * ioctls for vcpu fds
-- 
1.7.1

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Avi Kivity]

On 03/25/2011 10:44 AM, Joerg Roedel wrote:
> The calculation of the tsc_delta value to ensure a
> forward-going tsc for the guest is a function of the
> host-tsc. This works as long as the guests tsc_khz is equal
> to the hosts tsc_khz. With tsc-scaling hardware support this
> is not longer true and the tsc_delta needs to be calculated
>
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2126,8 +2126,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>   	kvm_x86_ops->vcpu_load(vcpu, cpu);
>   	if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) {
>   		/* Make sure TSC doesn't go backwards */
> -		s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
> -				native_read_tsc() - vcpu->arch.last_host_tsc;
> +		s64 tsc_delta;
> +		u64 tsc;
> +
> +		kvm_get_msr(vcpu, MSR_IA32_TSC,&tsc);
> +		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
> +			     tsc - vcpu->arch.last_guest_tsc;
> +
>   		if (tsc_delta<  0)
>   			mark_tsc_unstable("KVM discovered backwards TSC");
>   		if (check_tsc_unstable()) {


Patch is correct, but can this really happen?  Won't all machines with 
scaled tsc also be stable?

Perhaps not multi-board machines.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 0/6] TSC scaling support for KVM v3

[Avi Kivity]

On 03/25/2011 10:44 AM, Joerg Roedel wrote:
> Hi,
>
> this is the third round of my patches to support tsc-scaling in KVM. The
> changes to v2 address Avi's comments from yesterday. Besides that the
> whole virtual_tsc_khz thing has been moved out of the vm into the vcpu
> data structure. The mult and shift parts where renamed to tsc_catchup_*
> because this is their actual use (and because the handling of
> virtual_tsc_khz has changed so that is made sense to seperate them).
>
> Comments and feedback (or merging) appreciated :-)
>

Looks good.



-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Joerg Roedel]

On Sun, Mar 27, 2011 at 12:57:03PM +0200, Avi Kivity wrote:
> On 03/25/2011 10:44 AM, Joerg Roedel wrote:
>> The calculation of the tsc_delta value to ensure a
>> forward-going tsc for the guest is a function of the
>> host-tsc. This works as long as the guests tsc_khz is equal
>> to the hosts tsc_khz. With tsc-scaling hardware support this
>> is not longer true and the tsc_delta needs to be calculated
>>
>> --- a/arch/x86/kvm/x86.c
>> +++ b/arch/x86/kvm/x86.c
>> @@ -2126,8 +2126,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>>   	kvm_x86_ops->vcpu_load(vcpu, cpu);
>>   	if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) {
>>   		/* Make sure TSC doesn't go backwards */
>> -		s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
>> -				native_read_tsc() - vcpu->arch.last_host_tsc;
>> +		s64 tsc_delta;
>> +		u64 tsc;
>> +
>> +		kvm_get_msr(vcpu, MSR_IA32_TSC,&tsc);
>> +		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
>> +			     tsc - vcpu->arch.last_guest_tsc;
>> +
>>   		if (tsc_delta<  0)
>>   			mark_tsc_unstable("KVM discovered backwards TSC");
>>   		if (check_tsc_unstable()) {
>
>
> Patch is correct, but can this really happen?  Won't all machines with  
> scaled tsc also be stable?
>
> Perhaps not multi-board machines.

Yes, all machines with tsc-scaling should also have a stable tsc, so
this patch is not strictly required.
I added this patch as a response to Zachary's thoughts about the values
that need to be passed to adjust_tsc_offset. Zachary mentioned that with
this patch-series the adjust_tsc_offset function will either take the
adjustment in guest-tsc or host-tsc units (which is not problematic if
guest and host tsc have the same frequency).
But with tsc-scaling (and Zachary's upcoming patches) this might become
a problem. So I changed the semantic of adjust_tsc_offset to always take
the adjustment in guest-tsc units. This patch-set converts the one and
only call-site of adjust_tsc_offset where the unit is calculated as a
function of the host-tsc.

	Joerg

Re: [PATCH 0/6] TSC scaling support for KVM v3

[Avi Kivity]

On 03/25/2011 10:44 AM, Joerg Roedel wrote:
> Hi,
>
> this is the third round of my patches to support tsc-scaling in KVM. The
> changes to v2 address Avi's comments from yesterday. Besides that the
> whole virtual_tsc_khz thing has been moved out of the vm into the vcpu
> data structure. The mult and shift parts where renamed to tsc_catchup_*
> because this is their actual use (and because the handling of
> virtual_tsc_khz has changed so that is made sense to seperate them).
>
> Comments and feedback (or merging) appreciated :-)

Applied, thanks.

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1852 bytes --]

On 2011-03-25 09:44, Joerg Roedel wrote:
> The calculation of the tsc_delta value to ensure a
> forward-going tsc for the guest is a function of the
> host-tsc. This works as long as the guests tsc_khz is equal
> to the hosts tsc_khz. With tsc-scaling hardware support this
> is not longer true and the tsc_delta needs to be calculated
> using guest_tsc values.
> 
> Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
> ---
>  arch/x86/kvm/x86.c |    9 +++++++--
>  1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1e7af86..47dd6ed 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2126,8 +2126,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
>  	kvm_x86_ops->vcpu_load(vcpu, cpu);
>  	if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) {
>  		/* Make sure TSC doesn't go backwards */
> -		s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
> -				native_read_tsc() - vcpu->arch.last_host_tsc;
> +		s64 tsc_delta;
> +		u64 tsc;
> +
> +		kvm_get_msr(vcpu, MSR_IA32_TSC, &tsc);
> +		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
> +			     tsc - vcpu->arch.last_guest_tsc;
> +
>  		if (tsc_delta < 0)
>  			mark_tsc_unstable("KVM discovered backwards TSC");
>  		if (check_tsc_unstable()) {

This patch appears to cause troubles to Linux guests on TSC clocksource
and APIC highres timer. The first boot after qemu start is always fine,
but after a reboot the guest timer appears to fire incorrectly or even
not at all.

Was this patch tested with a guest reboot scenario as well? Does it
account for the TSC being reset to 0 on reboot?

I'm using a 2.6.38 host + kvm-kmod, but I don't see that this could be
the reason. Going back one kernel commit while keeping kvm-kmod
unchanged resolves the issue.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Joerg Roedel]

On Sat, Apr 16, 2011 at 06:09:17PM +0200, Jan Kiszka wrote:
> On 2011-03-25 09:44, Joerg Roedel wrote:

> > +		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
> > +			     tsc - vcpu->arch.last_guest_tsc;

> This patch appears to cause troubles to Linux guests on TSC clocksource
> and APIC highres timer. The first boot after qemu start is always fine,
> but after a reboot the guest timer appears to fire incorrectly or even
> not at all.
> 
> Was this patch tested with a guest reboot scenario as well? Does it
> account for the TSC being reset to 0 on reboot?

Hmm, probably the last_guest_tsc is not updated correctly in this
scenario. I will have a look tomorrow.

	Joerg

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Roedel, Joerg]

On Sat, Apr 16, 2011 at 12:09:17PM -0400, Jan Kiszka wrote:
> This patch appears to cause troubles to Linux guests on TSC clocksource
> and APIC highres timer. The first boot after qemu start is always fine,
> but after a reboot the guest timer appears to fire incorrectly or even
> not at all.

Hi Jan,

can you please test if this patch fixes the problem?

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index bd57639..d7282ef 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -391,7 +391,6 @@ struct kvm_vcpu_arch {
 	unsigned int hw_tsc_khz;
 	unsigned int time_offset;
 	struct page *time_page;
-	u64 last_host_tsc;
 	u64 last_guest_tsc;
 	u64 last_kernel_ns;
 	u64 last_tsc_nsec;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 1d5a7f4..d8277aa 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2146,7 +2146,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 {
 	kvm_x86_ops->vcpu_put(vcpu);
 	kvm_put_guest_fpu(vcpu);
-	vcpu->arch.last_host_tsc = native_read_tsc();
+	kvm_get_msr(vcpu, MSR_IA32_TSC, &vcpu->arch.last_guest_tsc);
 }
 
 static int is_efer_nx(void)



-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Jan Kiszka]

On 2011-04-18 10:03, Roedel, Joerg wrote:
> On Sat, Apr 16, 2011 at 12:09:17PM -0400, Jan Kiszka wrote:
>> This patch appears to cause troubles to Linux guests on TSC clocksource
>> and APIC highres timer. The first boot after qemu start is always fine,
>> but after a reboot the guest timer appears to fire incorrectly or even
>> not at all.
> 
> Hi Jan,
> 
> can you please test if this patch fixes the problem?
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index bd57639..d7282ef 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -391,7 +391,6 @@ struct kvm_vcpu_arch {
>  	unsigned int hw_tsc_khz;
>  	unsigned int time_offset;
>  	struct page *time_page;
> -	u64 last_host_tsc;
>  	u64 last_guest_tsc;
>  	u64 last_kernel_ns;
>  	u64 last_tsc_nsec;
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 1d5a7f4..d8277aa 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2146,7 +2146,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
>  {
>  	kvm_x86_ops->vcpu_put(vcpu);
>  	kvm_put_guest_fpu(vcpu);
> -	vcpu->arch.last_host_tsc = native_read_tsc();
> +	kvm_get_msr(vcpu, MSR_IA32_TSC, &vcpu->arch.last_guest_tsc);
>  }
>  
>  static int is_efer_nx(void)
> 

Looks good, behaviour is now fine also after reboot.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Joerg Roedel]

On Mon, Apr 18, 2011 at 11:12:56AM +0200, Jan Kiszka wrote:

> Looks good, behaviour is now fine also after reboot.

Thanks for testing, I send it out together with another fix today.

	Joerg

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Zachary Amsden]

>
> On Sat, Apr 16, 2011 at 06:09:17PM +0200, Jan Kiszka wrote:
>    
>> On 2011-03-25 09:44, Joerg Roedel wrote:
>>      
>    
>>> +		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
>>> +			     tsc - vcpu->arch.last_guest_tsc;
>>>        
>    
>> This patch appears to cause troubles to Linux guests on TSC clocksource
>> and APIC highres timer. The first boot after qemu start is always fine,
>> but after a reboot the guest timer appears to fire incorrectly or even
>> not at all.
>>
>> Was this patch tested with a guest reboot scenario as well? Does it
>> account for the TSC being reset to 0 on reboot?
>>      
> Hmm, probably the last_guest_tsc is not updated correctly in this
> scenario. I will have a look tomorrow.
>
> 	Joerg
>
>    

To avoid this problem, when the TSC is reset, the overshoot protection 
where last_guest_tsc is used is specifically disabled:

         /* Reset of TSC must disable overshoot protection below */
         vcpu->arch.hv_clock.tsc_timestamp = 0;
         vcpu->arch.last_tsc_write = data;
         vcpu->arch.last_tsc_nsec = ns;

You can probably use the same test - last_guest_tsc is only valid if 
tsc_timestamp above != 0.

Zach

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Roedel, Joerg]

On Mon, Apr 18, 2011 at 08:02:35PM -0400, Zachary Amsden wrote:
> >
> > On Sat, Apr 16, 2011 at 06:09:17PM +0200, Jan Kiszka wrote:
> >    
> >> On 2011-03-25 09:44, Joerg Roedel wrote:
> >>      
> >    
> >>> +		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
> >>> +			     tsc - vcpu->arch.last_guest_tsc;
> >>>        
> >    
> >> This patch appears to cause troubles to Linux guests on TSC clocksource
> >> and APIC highres timer. The first boot after qemu start is always fine,
> >> but after a reboot the guest timer appears to fire incorrectly or even
> >> not at all.
> >>
> >> Was this patch tested with a guest reboot scenario as well? Does it
> >> account for the TSC being reset to 0 on reboot?
> >>      
> > Hmm, probably the last_guest_tsc is not updated correctly in this
> > scenario. I will have a look tomorrow.
> >
> > 	Joerg
> >
> >    
> 
> To avoid this problem, when the TSC is reset, the overshoot protection 
> where last_guest_tsc is used is specifically disabled:
> 
>          /* Reset of TSC must disable overshoot protection below */
>          vcpu->arch.hv_clock.tsc_timestamp = 0;
>          vcpu->arch.last_tsc_write = data;
>          vcpu->arch.last_tsc_nsec = ns;
> 
> You can probably use the same test - last_guest_tsc is only valid if 
> tsc_timestamp above != 0.

Yes, this should also work. But the other way we get rid of
last_host_tsc which is also a good thing :)

	Joerg

-- 
AMD Operating System Research Center

Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach
General Managers: Alberto Bozzo, Andrew Bowd
Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]

On 2011-04-19 08:46, Roedel, Joerg wrote:
> On Mon, Apr 18, 2011 at 08:02:35PM -0400, Zachary Amsden wrote:
>>>
>>> On Sat, Apr 16, 2011 at 06:09:17PM +0200, Jan Kiszka wrote:
>>>    
>>>> On 2011-03-25 09:44, Joerg Roedel wrote:
>>>>      
>>>    
>>>>> +		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
>>>>> +			     tsc - vcpu->arch.last_guest_tsc;
>>>>>        
>>>    
>>>> This patch appears to cause troubles to Linux guests on TSC clocksource
>>>> and APIC highres timer. The first boot after qemu start is always fine,
>>>> but after a reboot the guest timer appears to fire incorrectly or even
>>>> not at all.
>>>>
>>>> Was this patch tested with a guest reboot scenario as well? Does it
>>>> account for the TSC being reset to 0 on reboot?
>>>>      
>>> Hmm, probably the last_guest_tsc is not updated correctly in this
>>> scenario. I will have a look tomorrow.
>>>
>>> 	Joerg
>>>
>>>    
>>
>> To avoid this problem, when the TSC is reset, the overshoot protection 
>> where last_guest_tsc is used is specifically disabled:
>>
>>          /* Reset of TSC must disable overshoot protection below */
>>          vcpu->arch.hv_clock.tsc_timestamp = 0;
>>          vcpu->arch.last_tsc_write = data;
>>          vcpu->arch.last_tsc_nsec = ns;
>>
>> You can probably use the same test - last_guest_tsc is only valid if 
>> tsc_timestamp above != 0.
> 
> Yes, this should also work. But the other way we get rid of
> last_host_tsc which is also a good thing :)

That reminds me: I think we still have the bug that KVM overeagerly
declares the host's TSC unstable after resume from S3 because the TSC
appears to go backward when KVM checks. Or should this have been fixed now?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Zachary Amsden]

>
> On 2011-04-19 08:46, Roedel, Joerg wrote:
>    
>> On Mon, Apr 18, 2011 at 08:02:35PM -0400, Zachary Amsden wrote:
>>      
>>>> On Sat, Apr 16, 2011 at 06:09:17PM +0200, Jan Kiszka wrote:
>>>>
>>>>          
>>>>> On 2011-03-25 09:44, Joerg Roedel wrote:
>>>>>
>>>>>            
>>>>
>>>>          
>>>>>> +		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
>>>>>> +			     tsc - vcpu->arch.last_guest_tsc;
>>>>>>
>>>>>>              
>>>>
>>>>          
>>>>> This patch appears to cause troubles to Linux guests on TSC clocksource
>>>>> and APIC highres timer. The first boot after qemu start is always fine,
>>>>> but after a reboot the guest timer appears to fire incorrectly or even
>>>>> not at all.
>>>>>
>>>>> Was this patch tested with a guest reboot scenario as well? Does it
>>>>> account for the TSC being reset to 0 on reboot?
>>>>>
>>>>>            
>>>> Hmm, probably the last_guest_tsc is not updated correctly in this
>>>> scenario. I will have a look tomorrow.
>>>>
>>>> 	Joerg
>>>>
>>>>
>>>>          
>>> To avoid this problem, when the TSC is reset, the overshoot protection
>>> where last_guest_tsc is used is specifically disabled:
>>>
>>>           /* Reset of TSC must disable overshoot protection below */
>>>           vcpu->arch.hv_clock.tsc_timestamp = 0;
>>>           vcpu->arch.last_tsc_write = data;
>>>           vcpu->arch.last_tsc_nsec = ns;
>>>
>>> You can probably use the same test - last_guest_tsc is only valid if
>>> tsc_timestamp above != 0.
>>>        
>> Yes, this should also work. But the other way we get rid of
>> last_host_tsc which is also a good thing :)
>>      
> That reminds me: I think we still have the bug that KVM overeagerly
> declares the host's TSC unstable after resume from S3 because the TSC
> appears to go backward when KVM checks. Or should this have been fixed now?
>
>    

That should have been fixed... let me check if the patch has made it 
upstream.

Re: [PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Jan Kiszka]

On 2011-04-19 16:15, Zachary Amsden wrote:
>>
>> On 2011-04-19 08:46, Roedel, Joerg wrote:
>>   
>>> On Mon, Apr 18, 2011 at 08:02:35PM -0400, Zachary Amsden wrote:
>>>     
>>>>> On Sat, Apr 16, 2011 at 06:09:17PM +0200, Jan Kiszka wrote:
>>>>>
>>>>>         
>>>>>> On 2011-03-25 09:44, Joerg Roedel wrote:
>>>>>>
>>>>>>            
>>>>>
>>>>>         
>>>>>>> +        tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
>>>>>>> +                 tsc - vcpu->arch.last_guest_tsc;
>>>>>>>
>>>>>>>              
>>>>>
>>>>>         
>>>>>> This patch appears to cause troubles to Linux guests on TSC
>>>>>> clocksource
>>>>>> and APIC highres timer. The first boot after qemu start is always
>>>>>> fine,
>>>>>> but after a reboot the guest timer appears to fire incorrectly or
>>>>>> even
>>>>>> not at all.
>>>>>>
>>>>>> Was this patch tested with a guest reboot scenario as well? Does it
>>>>>> account for the TSC being reset to 0 on reboot?
>>>>>>
>>>>>>            
>>>>> Hmm, probably the last_guest_tsc is not updated correctly in this
>>>>> scenario. I will have a look tomorrow.
>>>>>
>>>>>     Joerg
>>>>>
>>>>>
>>>>>          
>>>> To avoid this problem, when the TSC is reset, the overshoot protection
>>>> where last_guest_tsc is used is specifically disabled:
>>>>
>>>>           /* Reset of TSC must disable overshoot protection below */
>>>>           vcpu->arch.hv_clock.tsc_timestamp = 0;
>>>>           vcpu->arch.last_tsc_write = data;
>>>>           vcpu->arch.last_tsc_nsec = ns;
>>>>
>>>> You can probably use the same test - last_guest_tsc is only valid if
>>>> tsc_timestamp above != 0.
>>>>        
>>> Yes, this should also work. But the other way we get rid of
>>> last_host_tsc which is also a good thing :)
>>>      
>> That reminds me: I think we still have the bug that KVM overeagerly
>> declares the host's TSC unstable after resume from S3 because the TSC
>> appears to go backward when KVM checks. Or should this have been fixed
>> now?
>>
>>    
> 
> That should have been fixed... let me check if the patch has made it
> upstream.

I have "Marking TSC unstable due to KVM discovered backwards TSC" in my
recent log after I forgot to kill some guest before suspending the host.
And that was with kvm-kmod/kvm.git head. So I tend to believe that it's
not yet completely fixed...

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

[PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Joerg Roedel]

The calculation of the tsc_delta value to ensure a
forward-going tsc for the guest is a function of the
host-tsc. This works as long as the guests tsc_khz is equal
to the hosts tsc_khz. With tsc-scaling hardware support this
is not longer true and the tsc_delta needs to be calculated
using guest_tsc values.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/x86.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 7979da4..4fae242 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2127,8 +2127,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	kvm_x86_ops->vcpu_load(vcpu, cpu);
 	if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) {
 		/* Make sure TSC doesn't go backwards */
-		s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
-				native_read_tsc() - vcpu->arch.last_host_tsc;
+		s64 tsc_delta;
+		u64 tsc;
+
+		kvm_get_msr(vcpu, MSR_IA32_TSC, &tsc);
+		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
+			     tsc - vcpu->arch.last_guest_tsc;
+
 		if (tsc_delta < 0)
 			mark_tsc_unstable("KVM discovered backwards TSC");
 		if (check_tsc_unstable()) {
-- 
1.7.1

[PATCH 3/6] KVM: X86: Make tsc_delta calculation a function of guest tsc

[Joerg Roedel]

The calculation of the tsc_delta value to ensure a
forward-going tsc for the guest is a function of the
host-tsc. This works as long as the guests tsc_khz is equal
to the hosts tsc_khz. With tsc-scaling hardware support this
is not longer true and the tsc_delta needs to be calculated
using guest_tsc values.

Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>
---
 arch/x86/kvm/x86.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 50daff2..fbeb5ae 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2127,8 +2127,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 	kvm_x86_ops->vcpu_load(vcpu, cpu);
 	if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) {
 		/* Make sure TSC doesn't go backwards */
-		s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
-				native_read_tsc() - vcpu->arch.last_host_tsc;
+		s64 tsc_delta;
+		u64 tsc;
+
+		kvm_get_msr(vcpu, MSR_IA32_TSC, &tsc);
+		tsc_delta = !vcpu->arch.last_guest_tsc ? 0 :
+			     tsc - vcpu->arch.last_guest_tsc;
+
 		if (tsc_delta < 0)
 			mark_tsc_unstable("KVM discovered backwards TSC");
 		if (check_tsc_unstable()) {
-- 
1.7.1