Enabling IA32_TSC_ADJUST for KVM guest VM support

PATCH Superseded Will Auld 4 participants 8 messages Nov 27, 2012 – Nov 30, 2012

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Will Auld

author

2012-11-27 19:09 UTC #0 lore raw

PATCH v5 2/2 Enabling IA32_TSC_ADJUST for KVM guest VM support

CPUID.7.0.EBX[1]=1 indicates IA32_TSC_ADJUST MSR 0x3b is supported

Basic design is to emulate the MSR by allowing reads and writes to a guest vcpu specific location to store the value of the emulated MSR while adding the value to the vmcs tsc_offset. In this way the IA32_TSC_ADJUST value will be included in all reads to the TSC MSR whether through rdmsr or rdtsc. This is of course as long as the "use TSC counter offsetting" VM-execution control is enabled as well as the IA32_TSC_ADJUST control.

However, because hardware will only return the TSC + IA32_TSC_ADJUST + vmsc tsc_offset for a guest process when it does and rdtsc (with the correct settings) the value of our virtualized IA32_TSC_ADJUST must be stored in one of these three locations. The argument against storing it in the actual MSR is performance. This is likely to be seldom used while the save/restore is required on every transition. IA32_TSC_ADJUST was created as a way to solve some issues with writing TSC itself so that is not an option either. The remaining option, defined above as our solution has the problem of returning incorrect vmcs tsc_offset values (unless we intercept and fix, not done here) as mentioned above. However, more problematic is that storing the data in vmcs tsc_offset will have a different semantic effect on the system than does using the actual MSR. This is illustrated in the following example:
The hypervisor set the IA32_TSC_ADJUST, then the guest sets it and a guest process performs a rdtsc. In this case the guest process will get TSC + IA32_TSC_ADJUST_hyperviser + vmsc tsc_offset including IA32_TSC_ADJUST_guest. While the total system semantics changed the semantics as seen by the guest do not and hence this will not cause a problem.

Signed-off-by Will Auld <[email protected]>

---
 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/include/asm/kvm_host.h   |  3 +++
 arch/x86/include/asm/msr-index.h  |  1 +
 arch/x86/kvm/cpuid.c              |  2 ++
 arch/x86/kvm/cpuid.h              |  8 ++++++++
 arch/x86/kvm/svm.c                |  7 +++++++
 arch/x86/kvm/vmx.c                |  9 +++++++++
 arch/x86/kvm/x86.c                | 22 ++++++++++++++++++++++
 8 files changed, 53 insertions(+)

arch/x86/include/asm/cpufeature.h +1 -0

index 6b7ee5f..e574d81 100644
@@ -199,6 +199,7 @@
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
 #define X86_FEATURE_FSGSBASE	(9*32+ 0) /* {RD/WR}{FS/GS}BASE instructions*/
+#define X86_FEATURE_TSC_ADJUST  (9*32+ 1) /* TSC adjustment MSR 0x3b */
 #define X86_FEATURE_BMI1	(9*32+ 3) /* 1st group bit manipulation extensions */
 #define X86_FEATURE_HLE		(9*32+ 4) /* Hardware Lock Elision */
 #define X86_FEATURE_AVX2	(9*32+ 5) /* AVX2 instructions */

arch/x86/include/asm/kvm_host.h +3 -0

index da34027..cf8c7e0 100644
@@ -442,6 +442,8 @@ struct kvm_vcpu_arch {
 	u32 virtual_tsc_mult;
 	u32 virtual_tsc_khz;
 
+	s64 ia32_tsc_adjust_msr;
+
 	atomic_t nmi_queued;  /* unprocessed asynchronous NMIs */
 	unsigned nmi_pending; /* NMI queued after currently running handler */
 	bool nmi_injected;    /* Trying to inject an NMI this entry */

@@ -690,6 +692,7 @@ struct kvm_x86_ops {
 	bool (*has_wbinvd_exit)(void);
 
 	void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale);
+	u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
 	void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
 	u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);

arch/x86/include/asm/msr-index.h +1 -0

index 957ec87..6486569 100644
@@ -231,6 +231,7 @@
 #define MSR_IA32_EBL_CR_POWERON		0x0000002a
 #define MSR_EBC_FREQUENCY_ID		0x0000002c
 #define MSR_IA32_FEATURE_CONTROL        0x0000003a
+#define MSR_IA32_TSC_ADJUST             0x0000003b
 
 #define FEATURE_CONTROL_LOCKED				(1<<0)
 #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)

arch/x86/kvm/cpuid.c +2 -0

index 0595f13..e817bac 100644
@@ -320,6 +320,8 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		if (index == 0) {
 			entry->ebx &= kvm_supported_word9_x86_features;
 			cpuid_mask(&entry->ebx, 9);
+			// TSC_ADJUST is emulated 
+			entry->ebx |= F(TSC_ADJUST);
 		} else
 			entry->ebx = 0;
 		entry->eax = 0;

arch/x86/kvm/cpuid.h +8 -0

index a10e460..3a8b504 100644
@@ -28,6 +28,14 @@ static inline bool guest_cpuid_has_xsave(struct kvm_vcpu *vcpu)
 	return best && (best->ecx & bit(X86_FEATURE_XSAVE));
 }
 
+static inline bool guest_cpuid_has_tsc_adjust(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpuid_entry2 *best;
+
+	best = kvm_find_cpuid_entry(vcpu, 7, 0);
+	return best && (best->ebx & bit(X86_FEATURE_TSC_ADJUST));
+}
+
 static inline bool guest_cpuid_has_smep(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpuid_entry2 *best;

arch/x86/kvm/svm.c +7 -0

index 5ac11f0..7f5e6eb 100644
@@ -1012,6 +1012,12 @@ static void svm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
 	svm->tsc_ratio             = ratio;
 }
 
+static u64 svm_read_tsc_offset(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+	return svm->vmcb->control.tsc_offset;
+}
+
 static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);

@@ -4333,6 +4339,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.has_wbinvd_exit = svm_has_wbinvd_exit,
 
 	.set_tsc_khz = svm_set_tsc_khz,
+	.read_tsc_offset = svm_read_tsc_offset,
 	.write_tsc_offset = svm_write_tsc_offset,
 	.adjust_tsc_offset = svm_adjust_tsc_offset,
 	.compute_tsc_offset = svm_compute_tsc_offset,

arch/x86/kvm/vmx.c +9 -0

index 819970f..0a73e72 100644
@@ -1861,6 +1861,11 @@ static void vmx_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
 		WARN(1, "user requested TSC rate below hardware speed\n");
 }
 
+static u64 vmx_read_tsc_offset(struct kvm_vcpu *vcpu)
+{
+	return vmcs_read64(TSC_OFFSET);
+}
+
 /*
  * writes 'offset' into guest's timestamp counter offset register
  */

@@ -2243,6 +2248,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		}
 		ret = kvm_set_msr_common(vcpu, msr_info);
 		break;
+	case MSR_IA32_TSC_ADJUST:
+		ret = kvm_set_msr_common(vcpu, msr_info);
+		break;
 	case MSR_TSC_AUX:
 		if (!vmx->rdtscp_enabled)
 			return 1;

@@ -7338,6 +7346,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
 
 	.set_tsc_khz = vmx_set_tsc_khz,
+	.read_tsc_offset = vmx_read_tsc_offset,
 	.write_tsc_offset = vmx_write_tsc_offset,
 	.adjust_tsc_offset = vmx_adjust_tsc_offset,
 	.compute_tsc_offset = vmx_compute_tsc_offset,

arch/x86/kvm/x86.c +22 -0

index edafa29..7c2990b 100644
@@ -824,6 +824,7 @@ static u32 msrs_to_save[] = {
 static unsigned num_msrs_to_save;
 
 static u32 emulated_msrs[] = {
+	MSR_IA32_TSC_ADJUST,
 	MSR_IA32_TSCDEADLINE,
 	MSR_IA32_MISC_ENABLE,
 	MSR_IA32_MCG_STATUS,

@@ -1047,6 +1048,12 @@ static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns)
 	return tsc;
 }
 
+static void update_ia32_tsc_adjust_msr(struct kvm_vcpu *vcpu, s64 offset)
+{
+	u64 curr_offset =  kvm_x86_ops->read_tsc_offset(vcpu);
+	vcpu->arch.ia32_tsc_adjust_msr += offset - curr_offset;
+}
+
 void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr)
 {
 	struct kvm *kvm = vcpu->kvm;

@@ -1131,6 +1138,8 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr)
 	vcpu->arch.this_tsc_nsec = kvm->arch.cur_tsc_nsec;
 	vcpu->arch.this_tsc_write = kvm->arch.cur_tsc_write;
 
+	if (guest_cpuid_has_tsc_adjust(vcpu) && !msr->host_initiated) 
+		update_ia32_tsc_adjust_msr(vcpu, offset);
 	kvm_x86_ops->write_tsc_offset(vcpu, offset);
 	raw_spin_unlock_irqrestore(&kvm->arch.tsc_write_lock, flags);
 }

@@ -1621,6 +1630,15 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_IA32_TSCDEADLINE:
 		kvm_set_lapic_tscdeadline_msr(vcpu, data);
 		break;
+        case MSR_IA32_TSC_ADJUST:
+		if (guest_cpuid_has_tsc_adjust(vcpu)) {
+			if (!msr_info->host_initiated) { 
+				u64 adj = data - vcpu->arch.ia32_tsc_adjust_msr;
+				kvm_x86_ops->adjust_tsc_offset(vcpu, adj, true);
+			}
+			vcpu->arch.ia32_tsc_adjust_msr = data;
+		}
+		break;
 	case MSR_IA32_MISC_ENABLE:
 		vcpu->arch.ia32_misc_enable_msr = data;
 		break;

@@ -1981,6 +1999,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	case MSR_IA32_TSCDEADLINE:
 		data = kvm_get_lapic_tscdeadline_msr(vcpu);
 		break;
+	case MSR_IA32_TSC_ADJUST:
+		data = (u64)vcpu->arch.ia32_tsc_adjust_msr;
+		break;
 	case MSR_IA32_MISC_ENABLE:
 		data = vcpu->arch.ia32_misc_enable_msr;
 		break;

@@ -6226,6 +6247,7 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 	if (!zalloc_cpumask_var(&vcpu->arch.wbinvd_dirty_mask, GFP_KERNEL))
 		goto fail_free_mce_banks;
 
+	vcpu->arch.ia32_tsc_adjust_msr = 0x0;
 	kvm_async_pf_hash_reset(vcpu);
 	kvm_pmu_init(vcpu);

Signature · 1 line

1.8.0.rc0

Marcelo Tosatti

2012-11-28 01:03 UTC #1 lore raw

Hi Will,

Quoted 187 lines · Will Auld

CPUID.7.0.EBX[1]=1 indicates IA32_TSC_ADJUST MSR 0x3b is supported

Signed-off-by Will Auld <[email protected]>

---
 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/include/asm/kvm_host.h   |  3 +++
 arch/x86/include/asm/msr-index.h  |  1 +
 arch/x86/kvm/cpuid.c              |  2 ++
 arch/x86/kvm/cpuid.h              |  8 ++++++++
 arch/x86/kvm/svm.c                |  7 +++++++
 arch/x86/kvm/vmx.c                |  9 +++++++++
 arch/x86/kvm/x86.c                | 22 ++++++++++++++++++++++
 8 files changed, 53 insertions(+)

arch/x86/include/asm/cpufeature.h +1 -0

index 6b7ee5f..e574d81 100644
@@ -199,6 +199,7 @@
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
 #define X86_FEATURE_FSGSBASE	(9*32+ 0) /* {RD/WR}{FS/GS}BASE instructions*/
+#define X86_FEATURE_TSC_ADJUST  (9*32+ 1) /* TSC adjustment MSR 0x3b */
 #define X86_FEATURE_BMI1	(9*32+ 3) /* 1st group bit manipulation extensions */
 #define X86_FEATURE_HLE		(9*32+ 4) /* Hardware Lock Elision */
 #define X86_FEATURE_AVX2	(9*32+ 5) /* AVX2 instructions */

arch/x86/include/asm/kvm_host.h +3 -0

index da34027..cf8c7e0 100644
@@ -442,6 +442,8 @@ struct kvm_vcpu_arch {
 	u32 virtual_tsc_mult;
 	u32 virtual_tsc_khz;
 
+	s64 ia32_tsc_adjust_msr;
+
 	atomic_t nmi_queued;  /* unprocessed asynchronous NMIs */
 	unsigned nmi_pending; /* NMI queued after currently running handler */
 	bool nmi_injected;    /* Trying to inject an NMI this entry */

@@ -690,6 +692,7 @@ struct kvm_x86_ops {
 	bool (*has_wbinvd_exit)(void);
 
 	void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale);
+	u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
 	void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
 	u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);

arch/x86/include/asm/msr-index.h +1 -0

index 957ec87..6486569 100644
@@ -231,6 +231,7 @@
 #define MSR_IA32_EBL_CR_POWERON		0x0000002a
 #define MSR_EBC_FREQUENCY_ID		0x0000002c
 #define MSR_IA32_FEATURE_CONTROL        0x0000003a
+#define MSR_IA32_TSC_ADJUST             0x0000003b
 
 #define FEATURE_CONTROL_LOCKED				(1<<0)
 #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)

arch/x86/kvm/cpuid.c +2 -0

index 0595f13..e817bac 100644
@@ -320,6 +320,8 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		if (index == 0) {
 			entry->ebx &= kvm_supported_word9_x86_features;
 			cpuid_mask(&entry->ebx, 9);
+			// TSC_ADJUST is emulated 
+			entry->ebx |= F(TSC_ADJUST);
 		} else
 			entry->ebx = 0;
 		entry->eax = 0;

arch/x86/kvm/cpuid.h +8 -0

index a10e460..3a8b504 100644
@@ -28,6 +28,14 @@ static inline bool guest_cpuid_has_xsave(struct kvm_vcpu *vcpu)
 	return best && (best->ecx & bit(X86_FEATURE_XSAVE));
 }
 
+static inline bool guest_cpuid_has_tsc_adjust(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpuid_entry2 *best;
+
+	best = kvm_find_cpuid_entry(vcpu, 7, 0);
+	return best && (best->ebx & bit(X86_FEATURE_TSC_ADJUST));
+}
+
 static inline bool guest_cpuid_has_smep(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpuid_entry2 *best;

arch/x86/kvm/svm.c +7 -0

index 5ac11f0..7f5e6eb 100644
@@ -1012,6 +1012,12 @@ static void svm_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
 	svm->tsc_ratio             = ratio;
 }
 
+static u64 svm_read_tsc_offset(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_svm *svm = to_svm(vcpu);
+	return svm->vmcb->control.tsc_offset;
+}
+
 static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 {
 	struct vcpu_svm *svm = to_svm(vcpu);

@@ -4333,6 +4339,7 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.has_wbinvd_exit = svm_has_wbinvd_exit,
 
 	.set_tsc_khz = svm_set_tsc_khz,
+	.read_tsc_offset = svm_read_tsc_offset,
 	.write_tsc_offset = svm_write_tsc_offset,
 	.adjust_tsc_offset = svm_adjust_tsc_offset,
 	.compute_tsc_offset = svm_compute_tsc_offset,

arch/x86/kvm/vmx.c +9 -0

index 819970f..0a73e72 100644
@@ -1861,6 +1861,11 @@ static void vmx_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale)
 		WARN(1, "user requested TSC rate below hardware speed\n");
 }
 
+static u64 vmx_read_tsc_offset(struct kvm_vcpu *vcpu)
+{
+	return vmcs_read64(TSC_OFFSET);
+}
+
 /*
  * writes 'offset' into guest's timestamp counter offset register
  */

@@ -2243,6 +2248,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 		}
 		ret = kvm_set_msr_common(vcpu, msr_info);
+	case MSR_IA32_TSC_ADJUST:
+		ret = kvm_set_msr_common(vcpu, msr_info);
+		break;
 	case MSR_TSC_AUX:
 		if (!vmx->rdtscp_enabled)
 			return 1;

@@ -7338,6 +7346,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
 
 	.set_tsc_khz = vmx_set_tsc_khz,
+	.read_tsc_offset = vmx_read_tsc_offset,
 	.write_tsc_offset = vmx_write_tsc_offset,
 	.adjust_tsc_offset = vmx_adjust_tsc_offset,
 	.compute_tsc_offset = vmx_compute_tsc_offset,

arch/x86/kvm/x86.c +1 -0

index edafa29..7c2990b 100644
@@ -824,6 +824,7 @@ static u32 msrs_to_save[] = {
 static unsigned num_msrs_to_save;
 
 static u32 emulated_msrs[] = {
+	MSR_IA32_TSC_ADJUST,
 	MSR_IA32_TSCDEADLINE,
 	MSR_IA32_MISC_ENABLE,
 	MSR_IA32_MCG_STATUS,

arch/x86/kvm/x86.c static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns) · line 1053

Quoted 6 lines

arch/x86/kvm/x86.c +3 -0

@@ -1047,6 +1048,12 @@ static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns)
 	return tsc;
 }
 
+static void update_ia32_tsc_adjust_msr(struct kvm_vcpu *vcpu, s64 offset)
+{
+	u64 curr_offset =  kvm_x86_ops->read_tsc_offset(vcpu);

Extra space.

Other than that, looks fine. Remaining comments:

- Basic functional test must be improved (testing
that wrmsr(tsc, val) reflects on tsc_adjust and
wrmsr(tsc_adjust_msr, val) reflects on tsc is
enough.

- Behaviour on reset: what is the behaviour on RESET?

- Please rebase against queue branch on kvm.git.

Thanks

Auld, Will

2012-11-28 02:30 UTC #2 lore raw

Thanks Marcelo,

I'll address these items.

Quoted 1 line

- Please rebase against queue branch on kvm.git.

I am not sure how to do this. The repository I have been working against is:

git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git

I assume that I need to change this in some way but I am not sure what I need to do. Will you explain or give me the needed path?

Thanks,

Will

Quoted 237 lines

-----Original Message-----
From: Marcelo Tosatti [mailto:[email protected]]
Sent: Tuesday, November 27, 2012 5:00 PM
To: Auld, Will
Cc: [email protected]; Dugger, Donald D; Liu, Jinsong; Zhang, Xiantao; [email protected]; qemu-devel; Gleb
Subject: Re: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support
Hi Will,
Quoted 8 lines · Will Auld
CPUID.7.0.EBX[1]=1 indicates IA32_TSC_ADJUST MSR 0x3b is supported
Basic design is to emulate the MSR by allowing reads and writes to a guest vcpu specific location to store the value of the emulated MSR while adding the value to the vmcs tsc_offset. In this way the IA32_TSC_ADJUST value will be included in all reads to the TSC MSR whether through rdmsr or rdtsc. This is of course as long as the "use TSC counter offsetting" VM-execution control is enabled as well as
the IA32_TSC_ADJUST control.
Quoted 2 lines
However, because hardware will only return the TSC + IA32_TSC_ADJUST
+
Quoted 7 lines
vmsc tsc_offset for a guest process when it does and rdtsc (with the correct
settings) the value of our virtualized IA32_TSC_ADJUST must be stored in one of these three locations. The argument against storing it in the actual MSR is performance. This is likely to be seldom used while the save/restore is required on every transition. IA32_TSC_ADJUST was created as a way to solve some issues with writing TSC itself so that
is not an option either.
Quoted 1 line
The remaining option, defined above as our solution has the problem
of
Quoted 4 lines
returning incorrect vmcs tsc_offset values (unless we intercept and fix, not done here) as mentioned above. However, more problematic is that storing the data in vmcs tsc_offset will have a different semantic effect on the system than does using the actual MSR. This is
illustrated in the following example:
Quoted 3 lines
The hypervisor set the IA32_TSC_ADJUST, then the guest sets it and a guest process performs a rdtsc. In this case the guest process will get TSC + IA32_TSC_ADJUST_hyperviser + vmsc tsc_offset including
IA32_TSC_ADJUST_guest.
Quoted 21 lines
While the total system semantics changed the semantics as seen by the guest do not and hence this will not cause a problem.
Signed-off-by Will Auld <[email protected]>
---
 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/include/asm/kvm_host.h   |  3 +++
 arch/x86/include/asm/msr-index.h  |  1 +
 arch/x86/kvm/cpuid.c              |  2 ++
 arch/x86/kvm/cpuid.h              |  8 ++++++++
 arch/x86/kvm/svm.c                |  7 +++++++
 arch/x86/kvm/vmx.c                |  9 +++++++++
 arch/x86/kvm/x86.c                | 22 ++++++++++++++++++++++
 8 files changed, 53 insertions(+)
arch/x86/include/asm/cpufeature.h +0 -0
index 6b7ee5f..e574d81 100644
arch/x86/include/asm/cpufeature.h line 200
Quoted 2 lines
arch/x86/include/asm/cpufeature.h +0 -0
@@ -199,6 +199,7 @@
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word
9 */
arch/x86/include/asm/cpufeature.h line 201
Quoted 1 line
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_FSGSBASE	(9*32+ 0) /* {RD/WR}{FS/GS}BASE
instructions*/
arch/x86/include/asm/cpufeature.h line 204
Quoted 3 lines
arch/x86/include/asm/cpufeature.h +2 -0
+#define X86_FEATURE_TSC_ADJUST  (9*32+ 1) /* TSC adjustment MSR 0x3b
+*/
 #define X86_FEATURE_BMI1	(9*32+ 3) /* 1st group bit manipulation
extensions */
arch/x86/include/asm/cpufeature.h line 205
Quoted 1 line
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_HLE		(9*32+ 4) /* Hardware Lock Elision
*/
arch/x86/include/asm/kvm_host.h struct kvm_vcpu_arch { · line 448
Quoted 13 lines
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_AVX2	(9*32+ 5) /* AVX2 instructions */
(quoted diff) +0 -0
b/arch/x86/include/asm/kvm_host.h index da34027..cf8c7e0 100644
arch/x86/include/asm/kvm_host.h +2 -0
@@ -442,6 +442,8 @@ struct kvm_vcpu_arch {
 	u32 virtual_tsc_mult;
 	u32 virtual_tsc_khz;
 
+	s64 ia32_tsc_adjust_msr;
+
 	atomic_t nmi_queued;  /* unprocessed asynchronous NMIs */
 	unsigned nmi_pending; /* NMI queued after currently running
handler */
arch/x86/kvm/cpuid.c static int do_cpuid_ent(struct kvm_cpuid_entry2
Quoted 28 lines
arch/x86/include/asm/kvm_host.h +1 -0
 	bool nmi_injected;    /* Trying to inject an NMI this entry */
@@ -690,6 +692,7 @@ struct kvm_x86_ops {
 	bool (*has_wbinvd_exit)(void);
 
 	void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool
 scale);
+	u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
 	void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
arch/x86/include/asm/kvm_host.h +0 -0
 	u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);
arch/x86/include/asm/msr-index.h +1 -0
index 957ec87..6486569 100644
@@ -231,6 +231,7 @@
 #define MSR_IA32_EBL_CR_POWERON		0x0000002a
 #define MSR_EBC_FREQUENCY_ID		0x0000002c
 #define MSR_IA32_FEATURE_CONTROL        0x0000003a
+#define MSR_IA32_TSC_ADJUST             0x0000003b
 
 #define FEATURE_CONTROL_LOCKED				(1<<0)
 #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
arch/x86/kvm/cpuid.c index renamed from arch/x86/kvm/cpuid.c +0 -0
0595f13..e817bac 100644
arch/x86/kvm/cpuid.c +0 -0
@@ -320,6 +320,8 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2
*entry, u32 function,
arch/x86/kvm/cpuid.h static inline bool guest_cpuid_has_xsave(struct
Quoted 13 lines
arch/x86/kvm/cpuid.c +2 -0
 		if (index == 0) {
 			entry->ebx &= kvm_supported_word9_x86_features;
 			cpuid_mask(&entry->ebx, 9);
+			// TSC_ADJUST is emulated
+			entry->ebx |= F(TSC_ADJUST);
 		} else
 			entry->ebx = 0;
 		entry->eax = 0;
arch/x86/kvm/cpuid.h index renamed from arch/x86/kvm/cpuid.h +0 -0
a10e460..3a8b504 100644
arch/x86/kvm/cpuid.h +0 -0
@@ -28,6 +28,14 @@ static inline bool guest_cpuid_has_xsave(struct
kvm_vcpu *vcpu)
arch/x86/kvm/svm.c static void svm_set_tsc_khz(struct kvm_vcpu
Quoted 16 lines
arch/x86/kvm/cpuid.h +7 -0
 	return best && (best->ecx & bit(X86_FEATURE_XSAVE));  }
 
+static inline bool guest_cpuid_has_tsc_adjust(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpuid_entry2 *best;
+
+	best = kvm_find_cpuid_entry(vcpu, 7, 0);
+	return best && (best->ebx & bit(X86_FEATURE_TSC_ADJUST)); }
+
 static inline bool guest_cpuid_has_smep(struct kvm_vcpu *vcpu)  {
 	struct kvm_cpuid_entry2 *best;
arch/x86/kvm/svm.c index renamed from arch/x86/kvm/svm.c +0 -0
5ac11f0..7f5e6eb 100644
arch/x86/kvm/svm.c +0 -0
@@ -1012,6 +1012,12 @@ static void svm_set_tsc_khz(struct kvm_vcpu
*vcpu, u32 user_tsc_khz, bool scale)
arch/x86/kvm/vmx.c static void vmx_set_tsc_khz(struct kvm_vcpu
Quoted 23 lines
arch/x86/kvm/svm.c +4 -0
 	svm->tsc_ratio             = ratio;
 }
 
+static u64 svm_read_tsc_offset(struct kvm_vcpu *vcpu) {
+	struct vcpu_svm *svm = to_svm(vcpu);
+	return svm->vmcb->control.tsc_offset; }
+
 static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 {
 	struct vcpu_svm *svm = to_svm(vcpu); @@ -4333,6 +4339,7 @@ static
struct kvm_x86_ops svm_x86_ops = {
arch/x86/kvm/svm.c +1 -0
 	.has_wbinvd_exit = svm_has_wbinvd_exit,
 
 	.set_tsc_khz = svm_set_tsc_khz,
+	.read_tsc_offset = svm_read_tsc_offset,
 	.write_tsc_offset = svm_write_tsc_offset,
 	.adjust_tsc_offset = svm_adjust_tsc_offset,
 	.compute_tsc_offset = svm_compute_tsc_offset, diff --git
a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 819970f..0a73e72 100644
arch/x86/kvm/vmx.c +0 -0
@@ -1861,6 +1861,11 @@ static void vmx_set_tsc_khz(struct kvm_vcpu
*vcpu, u32 user_tsc_khz, bool scale)
arch/x86/kvm/vmx.c static void vmx_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale) · line 1861
Quoted 1 line
arch/x86/kvm/vmx.c +0 -0
 		WARN(1, "user requested TSC rate below hardware speed\n");
}
arch/x86/kvm/vmx.c static int vmx_set_msr(struct kvm_vcpu *vcpu,
Quoted 9 lines
arch/x86/kvm/vmx.c +4 -0
+static u64 vmx_read_tsc_offset(struct kvm_vcpu *vcpu) {
+	return vmcs_read64(TSC_OFFSET);
+}
+
 /*
  * writes 'offset' into guest's timestamp counter offset register
  */
@@ -2243,6 +2248,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu,
struct msr_data *msr_info)
arch/x86/kvm/x86.c static u64 compute_guest_tsc(struct kvm_vcpu
Quoted 29 lines
arch/x86/kvm/vmx.c +4 -0
 		}
 		ret = kvm_set_msr_common(vcpu, msr_info);
+	case MSR_IA32_TSC_ADJUST:
+		ret = kvm_set_msr_common(vcpu, msr_info);
+		break;
 	case MSR_TSC_AUX:
 		if (!vmx->rdtscp_enabled)
 			return 1;
@@ -7338,6 +7346,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
 
 	.set_tsc_khz = vmx_set_tsc_khz,
+	.read_tsc_offset = vmx_read_tsc_offset,
 	.write_tsc_offset = vmx_write_tsc_offset,
 	.adjust_tsc_offset = vmx_adjust_tsc_offset,
 	.compute_tsc_offset = vmx_compute_tsc_offset, diff --git
a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index edafa29..7c2990b 100644
arch/x86/kvm/x86.c +1 -0
@@ -824,6 +824,7 @@ static u32 msrs_to_save[] = {  static unsigned
 num_msrs_to_save;
 
 static u32 emulated_msrs[] = {
+	MSR_IA32_TSC_ADJUST,
 	MSR_IA32_TSCDEADLINE,
 	MSR_IA32_MISC_ENABLE,
 	MSR_IA32_MCG_STATUS,
@@ -1047,6 +1048,12 @@ static u64 compute_guest_tsc(struct kvm_vcpu
*vcpu, s64 kernel_ns)
arch/x86/kvm/x86.c static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns) · line 1053
Quoted 6 lines
arch/x86/kvm/x86.c +3 -0
 	return tsc;
 }
 
+static void update_ia32_tsc_adjust_msr(struct kvm_vcpu *vcpu, s64
+offset) {
+	u64 curr_offset =  kvm_x86_ops->read_tsc_offset(vcpu);
Extra space.
Other than that, looks fine. Remaining comments:
- Basic functional test must be improved (testing that wrmsr(tsc, val) reflects on tsc_adjust and wrmsr(tsc_adjust_msr, val) reflects on tsc is enough.
- Behaviour on reset: what is the behaviour on RESET?
- Please rebase against queue branch on kvm.git.
Thanks

Amos Kong

2012-11-28 02:39 UTC #3 lore raw

Quoted 6 lines · Will

Thanks Marcelo,
I'll address these items.
Quoted 1 line
- Please rebase against queue branch on kvm.git.

Quoted 8 lines

I am not sure how to do this. The repository I have been working against is:
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
I assume that I need to change this in some way but I am not sure what I need to do. Will you explain or give me the needed path?

http://www.linux-kvm.org/page/Code

You should use this tree: git://git.kernel.org/pub/scm/virt/kvm/kvm.git

Quoted 248 lines

Thanks,
Will
Quoted 231 lines
-----Original Message-----
From: Marcelo Tosatti [mailto:[email protected]]
Sent: Tuesday, November 27, 2012 5:00 PM
To: Auld, Will
Cc: [email protected]; Dugger, Donald D; Liu, Jinsong; Zhang, Xiantao; [email protected]; qemu-devel; Gleb
Subject: Re: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support
Hi Will,
Quoted 8 lines · Will Auld
CPUID.7.0.EBX[1]=1 indicates IA32_TSC_ADJUST MSR 0x3b is supported
Basic design is to emulate the MSR by allowing reads and writes to a guest vcpu specific location to store the value of the emulated MSR while adding the value to the vmcs tsc_offset. In this way the IA32_TSC_ADJUST value will be included in all reads to the TSC MSR whether through rdmsr or rdtsc. This is of course as long as the "use TSC counter offsetting" VM-execution control is enabled as well as
the IA32_TSC_ADJUST control.
Quoted 2 lines
However, because hardware will only return the TSC + IA32_TSC_ADJUST
+
Quoted 7 lines
vmsc tsc_offset for a guest process when it does and rdtsc (with the correct
settings) the value of our virtualized IA32_TSC_ADJUST must be stored in one of these three locations. The argument against storing it in the actual MSR is performance. This is likely to be seldom used while the save/restore is required on every transition. IA32_TSC_ADJUST was created as a way to solve some issues with writing TSC itself so that
is not an option either.
Quoted 1 line
The remaining option, defined above as our solution has the problem
of
Quoted 4 lines
returning incorrect vmcs tsc_offset values (unless we intercept and fix, not done here) as mentioned above. However, more problematic is that storing the data in vmcs tsc_offset will have a different semantic effect on the system than does using the actual MSR. This is
illustrated in the following example:
Quoted 3 lines
The hypervisor set the IA32_TSC_ADJUST, then the guest sets it and a guest process performs a rdtsc. In this case the guest process will get TSC + IA32_TSC_ADJUST_hyperviser + vmsc tsc_offset including
IA32_TSC_ADJUST_guest.
Quoted 21 lines
While the total system semantics changed the semantics as seen by the guest do not and hence this will not cause a problem.
Signed-off-by Will Auld <[email protected]>
---
 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/include/asm/kvm_host.h   |  3 +++
 arch/x86/include/asm/msr-index.h  |  1 +
 arch/x86/kvm/cpuid.c              |  2 ++
 arch/x86/kvm/cpuid.h              |  8 ++++++++
 arch/x86/kvm/svm.c                |  7 +++++++
 arch/x86/kvm/vmx.c                |  9 +++++++++
 arch/x86/kvm/x86.c                | 22 ++++++++++++++++++++++
 8 files changed, 53 insertions(+)
arch/x86/include/asm/cpufeature.h +0 -0
index 6b7ee5f..e574d81 100644
arch/x86/include/asm/cpufeature.h line 200
Quoted 2 lines
arch/x86/include/asm/cpufeature.h +0 -0
@@ -199,6 +199,7 @@
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word
9 */
arch/x86/include/asm/cpufeature.h line 201
Quoted 1 line
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_FSGSBASE       (9*32+ 0) /* {RD/WR}{FS/GS}BASE
instructions*/
arch/x86/include/asm/cpufeature.h line 204
Quoted 3 lines
arch/x86/include/asm/cpufeature.h +2 -0
+#define X86_FEATURE_TSC_ADJUST  (9*32+ 1) /* TSC adjustment MSR 0x3b
+*/
 #define X86_FEATURE_BMI1   (9*32+ 3) /* 1st group bit manipulation
extensions */
arch/x86/include/asm/cpufeature.h line 205
Quoted 1 line
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_HLE            (9*32+ 4) /* Hardware Lock Elision
*/
arch/x86/include/asm/kvm_host.h struct kvm_vcpu_arch { · line 448
Quoted 13 lines
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_AVX2   (9*32+ 5) /* AVX2 instructions */
(quoted diff) +0 -0
b/arch/x86/include/asm/kvm_host.h index da34027..cf8c7e0 100644
arch/x86/include/asm/kvm_host.h +2 -0
@@ -442,6 +442,8 @@ struct kvm_vcpu_arch {
    u32 virtual_tsc_mult;
    u32 virtual_tsc_khz;
 
+   s64 ia32_tsc_adjust_msr;
+
    atomic_t nmi_queued;  /* unprocessed asynchronous NMIs */
    unsigned nmi_pending; /* NMI queued after currently running
handler */
arch/x86/kvm/cpuid.c static int do_cpuid_ent(struct kvm_cpuid_entry2
Quoted 28 lines
arch/x86/include/asm/kvm_host.h +1 -0
    bool nmi_injected;    /* Trying to inject an NMI this entry */
@@ -690,6 +692,7 @@ struct kvm_x86_ops {
    bool (*has_wbinvd_exit)(void);
 
    void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool
 scale);
+   u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
    void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
    u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);
arch/x86/include/asm/msr-index.h +1 -0
index 957ec87..6486569 100644
@@ -231,6 +231,7 @@
 #define MSR_IA32_EBL_CR_POWERON            0x0000002a
 #define MSR_EBC_FREQUENCY_ID               0x0000002c
 #define MSR_IA32_FEATURE_CONTROL        0x0000003a
+#define MSR_IA32_TSC_ADJUST             0x0000003b
 
 #define FEATURE_CONTROL_LOCKED                             (1<<0)
 #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX   (1<<1)
arch/x86/kvm/cpuid.c index renamed from arch/x86/kvm/cpuid.c +0 -0
0595f13..e817bac 100644
arch/x86/kvm/cpuid.c +0 -0
@@ -320,6 +320,8 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2
*entry, u32 function,
arch/x86/kvm/cpuid.h static inline bool guest_cpuid_has_xsave(struct
Quoted 13 lines
arch/x86/kvm/cpuid.c +2 -0
            if (index == 0) {
                    entry->ebx &= kvm_supported_word9_x86_features;
                    cpuid_mask(&entry->ebx, 9);
+                   // TSC_ADJUST is emulated
+                   entry->ebx |= F(TSC_ADJUST);
            } else
                    entry->ebx = 0;
            entry->eax = 0;
arch/x86/kvm/cpuid.h index renamed from arch/x86/kvm/cpuid.h +0 -0
a10e460..3a8b504 100644
arch/x86/kvm/cpuid.h +0 -0
@@ -28,6 +28,14 @@ static inline bool guest_cpuid_has_xsave(struct
kvm_vcpu *vcpu)
arch/x86/kvm/svm.c static void svm_set_tsc_khz(struct kvm_vcpu
Quoted 16 lines
arch/x86/kvm/cpuid.h +7 -0
    return best && (best->ecx & bit(X86_FEATURE_XSAVE));  }
 
+static inline bool guest_cpuid_has_tsc_adjust(struct kvm_vcpu *vcpu)
+{
+   struct kvm_cpuid_entry2 *best;
+
+   best = kvm_find_cpuid_entry(vcpu, 7, 0);
+   return best && (best->ebx & bit(X86_FEATURE_TSC_ADJUST)); }
+
 static inline bool guest_cpuid_has_smep(struct kvm_vcpu *vcpu)  {
    struct kvm_cpuid_entry2 *best;
arch/x86/kvm/svm.c index renamed from arch/x86/kvm/svm.c +0 -0
5ac11f0..7f5e6eb 100644
arch/x86/kvm/svm.c +0 -0
@@ -1012,6 +1012,12 @@ static void svm_set_tsc_khz(struct kvm_vcpu
*vcpu, u32 user_tsc_khz, bool scale)
arch/x86/kvm/vmx.c static void vmx_set_tsc_khz(struct kvm_vcpu
Quoted 23 lines
arch/x86/kvm/svm.c +4 -0
    svm->tsc_ratio             = ratio;
 }
 
+static u64 svm_read_tsc_offset(struct kvm_vcpu *vcpu) {
+   struct vcpu_svm *svm = to_svm(vcpu);
+   return svm->vmcb->control.tsc_offset; }
+
 static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 {
    struct vcpu_svm *svm = to_svm(vcpu); @@ -4333,6 +4339,7 @@ static
struct kvm_x86_ops svm_x86_ops = {
.has_wbinvd_exit = svm_has_wbinvd_exit,
.set_tsc_khz = svm_set_tsc_khz,
+ .read_tsc_offset = svm_read_tsc_offset,
.write_tsc_offset = svm_write_tsc_offset, .adjust_tsc_offset = svm_adjust_tsc_offset, .compute_tsc_offset = svm_compute_tsc_offset, diff --git
a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 819970f..0a73e72 100644
arch/x86/kvm/vmx.c +0 -0
@@ -1861,6 +1861,11 @@ static void vmx_set_tsc_khz(struct kvm_vcpu
*vcpu, u32 user_tsc_khz, bool scale)
arch/x86/kvm/vmx.c static void vmx_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale) · line 1861
Quoted 1 line
arch/x86/kvm/vmx.c +0 -0
            WARN(1, "user requested TSC rate below hardware speed\n");
}
arch/x86/kvm/vmx.c static int vmx_set_msr(struct kvm_vcpu *vcpu,
Quoted 9 lines
arch/x86/kvm/vmx.c +4 -0
+static u64 vmx_read_tsc_offset(struct kvm_vcpu *vcpu) {
+   return vmcs_read64(TSC_OFFSET);
+}
+
 /*
  * writes 'offset' into guest's timestamp counter offset register
  */
@@ -2243,6 +2248,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu,
struct msr_data *msr_info)
arch/x86/kvm/x86.c static u64 compute_guest_tsc(struct kvm_vcpu
Quoted 29 lines
arch/x86/kvm/vmx.c +4 -0
            }
            ret = kvm_set_msr_common(vcpu, msr_info);
+   case MSR_IA32_TSC_ADJUST:
+           ret = kvm_set_msr_common(vcpu, msr_info);
+           break;
    case MSR_TSC_AUX:
            if (!vmx->rdtscp_enabled)
                    return 1;
@@ -7338,6 +7346,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
    .has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
 
    .set_tsc_khz = vmx_set_tsc_khz,
+   .read_tsc_offset = vmx_read_tsc_offset,
    .write_tsc_offset = vmx_write_tsc_offset,
    .adjust_tsc_offset = vmx_adjust_tsc_offset,
    .compute_tsc_offset = vmx_compute_tsc_offset, diff --git
a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index edafa29..7c2990b 100644
arch/x86/kvm/x86.c +1 -0
@@ -824,6 +824,7 @@ static u32 msrs_to_save[] = {  static unsigned
 num_msrs_to_save;
 
 static u32 emulated_msrs[] = {
+   MSR_IA32_TSC_ADJUST,
    MSR_IA32_TSCDEADLINE,
    MSR_IA32_MISC_ENABLE,
    MSR_IA32_MCG_STATUS,
@@ -1047,6 +1048,12 @@ static u64 compute_guest_tsc(struct kvm_vcpu
*vcpu, s64 kernel_ns)
arch/x86/kvm/x86.c static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns) · line 1053
Quoted 6 lines
arch/x86/kvm/x86.c +3 -0
    return tsc;
 }
 
+static void update_ia32_tsc_adjust_msr(struct kvm_vcpu *vcpu, s64
+offset) {
+   u64 curr_offset =  kvm_x86_ops->read_tsc_offset(vcpu);
Extra space.
Other than that, looks fine. Remaining comments:
- Basic functional test must be improved (testing that wrmsr(tsc, val) reflects on tsc_adjust and wrmsr(tsc_adjust_msr, val) reflects on tsc is enough.
Quoted 6 lines
- Behaviour on reset: what is the behaviour on RESET?
- Please rebase against queue branch on kvm.git.
Thanks
--
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

Auld, Will

2012-11-28 21:41 UTC #4 lore raw

Thanks Amos.

From: Amos Kong [mailto:[email protected]]
Sent: Tuesday, November 27, 2012 6:39 PM
To: Auld, Will
Cc: Marcelo Tosatti; [email protected]; Dugger, Donald D; Liu, Jinsong; Zhang, Xiantao; [email protected]; qemu-devel; Gleb
Subject: Re: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support

On Wed, Nov 28, 2012 at 10:29 AM, Auld, Will <[email protected]<mailto:[email protected]>> wrote:
Thanks Marcelo,

I'll address these items.

Quoted 1 line

- Please rebase against queue branch on kvm.git.

I am not sure how to do this. The repository I have been working against is:

git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git<http://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git>

I assume that I need to change this in some way but I am not sure what I need to do. Will you explain or give me the needed path?

http://www.linux-kvm.org/page/Code

You should use this tree: git://git.kernel.org/pub/scm/virt/kvm/kvm.git<http://git.kernel.org/pub/scm/virt/kvm/kvm.git>

Thanks,

Will

Quoted 231 lines

-----Original Message-----
From: Marcelo Tosatti [mailto:[email protected]<mailto:[email protected]>]
Sent: Tuesday, November 27, 2012 5:00 PM
To: Auld, Will
Cc: [email protected]<mailto:[email protected]>; Dugger, Donald D; Liu, Jinsong; Zhang, Xiantao; [email protected]<mailto:[email protected]>; qemu-devel; Gleb
Subject: Re: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support
Hi Will,
Quoted 8 lines · Will Auld
CPUID.7.0.EBX[1]=1 indicates IA32_TSC_ADJUST MSR 0x3b is supported
Basic design is to emulate the MSR by allowing reads and writes to a guest vcpu specific location to store the value of the emulated MSR while adding the value to the vmcs tsc_offset. In this way the IA32_TSC_ADJUST value will be included in all reads to the TSC MSR whether through rdmsr or rdtsc. This is of course as long as the "use TSC counter offsetting" VM-execution control is enabled as well as
the IA32_TSC_ADJUST control.
Quoted 2 lines
However, because hardware will only return the TSC + IA32_TSC_ADJUST
+
Quoted 7 lines
vmsc tsc_offset for a guest process when it does and rdtsc (with the correct
settings) the value of our virtualized IA32_TSC_ADJUST must be stored in one of these three locations. The argument against storing it in the actual MSR is performance. This is likely to be seldom used while the save/restore is required on every transition. IA32_TSC_ADJUST was created as a way to solve some issues with writing TSC itself so that
is not an option either.
Quoted 1 line
The remaining option, defined above as our solution has the problem
of
Quoted 4 lines
returning incorrect vmcs tsc_offset values (unless we intercept and fix, not done here) as mentioned above. However, more problematic is that storing the data in vmcs tsc_offset will have a different semantic effect on the system than does using the actual MSR. This is
illustrated in the following example:
Quoted 3 lines
The hypervisor set the IA32_TSC_ADJUST, then the guest sets it and a guest process performs a rdtsc. In this case the guest process will get TSC + IA32_TSC_ADJUST_hyperviser + vmsc tsc_offset including
IA32_TSC_ADJUST_guest.
Quoted 21 lines
While the total system semantics changed the semantics as seen by the guest do not and hence this will not cause a problem.
Signed-off-by Will Auld <[email protected]<mailto:[email protected]>>
---
 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/include/asm/kvm_host.h   |  3 +++
 arch/x86/include/asm/msr-index.h  |  1 +
 arch/x86/kvm/cpuid.c              |  2 ++
 arch/x86/kvm/cpuid.h              |  8 ++++++++
 arch/x86/kvm/svm.c                |  7 +++++++
 arch/x86/kvm/vmx.c                |  9 +++++++++
 arch/x86/kvm/x86.c                | 22 ++++++++++++++++++++++
 8 files changed, 53 insertions(+)
arch/x86/include/asm/cpufeature.h +0 -0
index 6b7ee5f..e574d81 100644
arch/x86/include/asm/cpufeature.h line 200
Quoted 2 lines
arch/x86/include/asm/cpufeature.h +0 -0
@@ -199,6 +199,7 @@
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word
9 */
arch/x86/include/asm/cpufeature.h line 201
Quoted 1 line
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_FSGSBASE       (9*32+ 0) /* {RD/WR}{FS/GS}BASE
instructions*/
arch/x86/include/asm/cpufeature.h line 204
Quoted 3 lines
arch/x86/include/asm/cpufeature.h +2 -0
+#define X86_FEATURE_TSC_ADJUST  (9*32+ 1) /* TSC adjustment MSR 0x3b
+*/
 #define X86_FEATURE_BMI1   (9*32+ 3) /* 1st group bit manipulation
extensions */
arch/x86/include/asm/cpufeature.h line 205
Quoted 1 line
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_HLE            (9*32+ 4) /* Hardware Lock Elision
*/
arch/x86/include/asm/kvm_host.h struct kvm_vcpu_arch { · line 448
Quoted 13 lines
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_AVX2   (9*32+ 5) /* AVX2 instructions */
(quoted diff) +0 -0
b/arch/x86/include/asm/kvm_host.h index da34027..cf8c7e0 100644
arch/x86/include/asm/kvm_host.h +2 -0
@@ -442,6 +442,8 @@ struct kvm_vcpu_arch {
    u32 virtual_tsc_mult;
    u32 virtual_tsc_khz;
 
+   s64 ia32_tsc_adjust_msr;
+
    atomic_t nmi_queued;  /* unprocessed asynchronous NMIs */
    unsigned nmi_pending; /* NMI queued after currently running
handler */
arch/x86/kvm/cpuid.c static int do_cpuid_ent(struct kvm_cpuid_entry2
Quoted 28 lines
arch/x86/include/asm/kvm_host.h +1 -0
    bool nmi_injected;    /* Trying to inject an NMI this entry */
@@ -690,6 +692,7 @@ struct kvm_x86_ops {
    bool (*has_wbinvd_exit)(void);
 
    void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool
 scale);
+   u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
    void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
    u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);
arch/x86/include/asm/msr-index.h +1 -0
index 957ec87..6486569 100644
@@ -231,6 +231,7 @@
 #define MSR_IA32_EBL_CR_POWERON            0x0000002a
 #define MSR_EBC_FREQUENCY_ID               0x0000002c
 #define MSR_IA32_FEATURE_CONTROL        0x0000003a
+#define MSR_IA32_TSC_ADJUST             0x0000003b
 
 #define FEATURE_CONTROL_LOCKED                             (1<<0)
 #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX   (1<<1)
arch/x86/kvm/cpuid.c index renamed from arch/x86/kvm/cpuid.c +0 -0
0595f13..e817bac 100644
arch/x86/kvm/cpuid.c +0 -0
@@ -320,6 +320,8 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2
*entry, u32 function,
arch/x86/kvm/cpuid.h static inline bool guest_cpuid_has_xsave(struct
Quoted 13 lines
arch/x86/kvm/cpuid.c +2 -0
            if (index == 0) {
                    entry->ebx &= kvm_supported_word9_x86_features;
                    cpuid_mask(&entry->ebx, 9);
+                   // TSC_ADJUST is emulated
+                   entry->ebx |= F(TSC_ADJUST);
            } else
                    entry->ebx = 0;
            entry->eax = 0;
arch/x86/kvm/cpuid.h index renamed from arch/x86/kvm/cpuid.h +0 -0
a10e460..3a8b504 100644
arch/x86/kvm/cpuid.h +0 -0
@@ -28,6 +28,14 @@ static inline bool guest_cpuid_has_xsave(struct
kvm_vcpu *vcpu)
arch/x86/kvm/svm.c static void svm_set_tsc_khz(struct kvm_vcpu
Quoted 16 lines
arch/x86/kvm/cpuid.h +7 -0
    return best && (best->ecx & bit(X86_FEATURE_XSAVE));  }
 
+static inline bool guest_cpuid_has_tsc_adjust(struct kvm_vcpu *vcpu)
+{
+   struct kvm_cpuid_entry2 *best;
+
+   best = kvm_find_cpuid_entry(vcpu, 7, 0);
+   return best && (best->ebx & bit(X86_FEATURE_TSC_ADJUST)); }
+
 static inline bool guest_cpuid_has_smep(struct kvm_vcpu *vcpu)  {
    struct kvm_cpuid_entry2 *best;
arch/x86/kvm/svm.c index renamed from arch/x86/kvm/svm.c +0 -0
5ac11f0..7f5e6eb 100644
arch/x86/kvm/svm.c +0 -0
@@ -1012,6 +1012,12 @@ static void svm_set_tsc_khz(struct kvm_vcpu
*vcpu, u32 user_tsc_khz, bool scale)
arch/x86/kvm/vmx.c static void vmx_set_tsc_khz(struct kvm_vcpu
Quoted 23 lines
arch/x86/kvm/svm.c +4 -0
    svm->tsc_ratio             = ratio;
 }
 
+static u64 svm_read_tsc_offset(struct kvm_vcpu *vcpu) {
+   struct vcpu_svm *svm = to_svm(vcpu);
+   return svm->vmcb->control.tsc_offset; }
+
 static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 {
    struct vcpu_svm *svm = to_svm(vcpu); @@ -4333,6 +4339,7 @@ static
struct kvm_x86_ops svm_x86_ops = {
.has_wbinvd_exit = svm_has_wbinvd_exit,
.set_tsc_khz = svm_set_tsc_khz,
+ .read_tsc_offset = svm_read_tsc_offset,
.write_tsc_offset = svm_write_tsc_offset, .adjust_tsc_offset = svm_adjust_tsc_offset, .compute_tsc_offset = svm_compute_tsc_offset, diff --git
a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 819970f..0a73e72 100644
arch/x86/kvm/vmx.c +0 -0
@@ -1861,6 +1861,11 @@ static void vmx_set_tsc_khz(struct kvm_vcpu
*vcpu, u32 user_tsc_khz, bool scale)
arch/x86/kvm/vmx.c static void vmx_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale) · line 1861
Quoted 1 line
arch/x86/kvm/vmx.c +0 -0
            WARN(1, "user requested TSC rate below hardware speed\n");
}
arch/x86/kvm/vmx.c static int vmx_set_msr(struct kvm_vcpu *vcpu,
Quoted 9 lines
arch/x86/kvm/vmx.c +4 -0
+static u64 vmx_read_tsc_offset(struct kvm_vcpu *vcpu) {
+   return vmcs_read64(TSC_OFFSET);
+}
+
 /*
  * writes 'offset' into guest's timestamp counter offset register
  */
@@ -2243,6 +2248,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu,
struct msr_data *msr_info)
arch/x86/kvm/x86.c static u64 compute_guest_tsc(struct kvm_vcpu
Quoted 29 lines
arch/x86/kvm/vmx.c +4 -0
            }
            ret = kvm_set_msr_common(vcpu, msr_info);
+   case MSR_IA32_TSC_ADJUST:
+           ret = kvm_set_msr_common(vcpu, msr_info);
+           break;
    case MSR_TSC_AUX:
            if (!vmx->rdtscp_enabled)
                    return 1;
@@ -7338,6 +7346,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
    .has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
 
    .set_tsc_khz = vmx_set_tsc_khz,
+   .read_tsc_offset = vmx_read_tsc_offset,
    .write_tsc_offset = vmx_write_tsc_offset,
    .adjust_tsc_offset = vmx_adjust_tsc_offset,
    .compute_tsc_offset = vmx_compute_tsc_offset, diff --git
a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index edafa29..7c2990b 100644
arch/x86/kvm/x86.c +1 -0
@@ -824,6 +824,7 @@ static u32 msrs_to_save[] = {  static unsigned
 num_msrs_to_save;
 
 static u32 emulated_msrs[] = {
+   MSR_IA32_TSC_ADJUST,
    MSR_IA32_TSCDEADLINE,
    MSR_IA32_MISC_ENABLE,
    MSR_IA32_MCG_STATUS,
@@ -1047,6 +1048,12 @@ static u64 compute_guest_tsc(struct kvm_vcpu
*vcpu, s64 kernel_ns)
arch/x86/kvm/x86.c static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns) · line 1053
Quoted 6 lines
arch/x86/kvm/x86.c +3 -0
    return tsc;
 }
 
+static void update_ia32_tsc_adjust_msr(struct kvm_vcpu *vcpu, s64
+offset) {
+   u64 curr_offset =  kvm_x86_ops->read_tsc_offset(vcpu);
Extra space.
Other than that, looks fine. Remaining comments:
- Basic functional test must be improved (testing that wrmsr(tsc, val) reflects on tsc_adjust and wrmsr(tsc_adjust_msr, val) reflects on tsc is enough.

Quoted 6 lines

- Behaviour on reset: what is the behaviour on RESET?
- Please rebase against queue branch on kvm.git.
Thanks

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

Auld, Will

2012-11-29 19:21 UTC #5 lore raw

In reply to Marcelo Tosatti

Marcelo,

The behavior on reset is to return the TSC_AJUST msr value to 0x0. I am currently initializing this emulated msr in kvm_arch_vcpu_init().

Quoted 1 line

- Behaviour on reset: what is the behaviour on RESET?

I am testing the rebase now. I would like to get any needed changes for this initialization into my next patch set version (v6) if possible.

Thanks,

Will

Quoted 237 lines

-----Original Message-----
From: Marcelo Tosatti [mailto:[email protected]]
Sent: Tuesday, November 27, 2012 5:00 PM
To: Auld, Will
Cc: [email protected]; Dugger, Donald D; Liu, Jinsong; Zhang, Xiantao; [email protected]; qemu-devel; Gleb
Subject: Re: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support
Hi Will,
Quoted 8 lines · Will Auld
CPUID.7.0.EBX[1]=1 indicates IA32_TSC_ADJUST MSR 0x3b is supported
Basic design is to emulate the MSR by allowing reads and writes to a guest vcpu specific location to store the value of the emulated MSR while adding the value to the vmcs tsc_offset. In this way the IA32_TSC_ADJUST value will be included in all reads to the TSC MSR whether through rdmsr or rdtsc. This is of course as long as the "use TSC counter offsetting" VM-execution control is enabled as well as
the IA32_TSC_ADJUST control.
Quoted 2 lines
However, because hardware will only return the TSC + IA32_TSC_ADJUST
+
Quoted 7 lines
vmsc tsc_offset for a guest process when it does and rdtsc (with the correct
settings) the value of our virtualized IA32_TSC_ADJUST must be stored in one of these three locations. The argument against storing it in the actual MSR is performance. This is likely to be seldom used while the save/restore is required on every transition. IA32_TSC_ADJUST was created as a way to solve some issues with writing TSC itself so that
is not an option either.
Quoted 1 line
The remaining option, defined above as our solution has the problem
of
Quoted 4 lines
returning incorrect vmcs tsc_offset values (unless we intercept and fix, not done here) as mentioned above. However, more problematic is that storing the data in vmcs tsc_offset will have a different semantic effect on the system than does using the actual MSR. This is
illustrated in the following example:
Quoted 3 lines
The hypervisor set the IA32_TSC_ADJUST, then the guest sets it and a guest process performs a rdtsc. In this case the guest process will get TSC + IA32_TSC_ADJUST_hyperviser + vmsc tsc_offset including
IA32_TSC_ADJUST_guest.
Quoted 21 lines
While the total system semantics changed the semantics as seen by the guest do not and hence this will not cause a problem.
Signed-off-by Will Auld <[email protected]>
---
 arch/x86/include/asm/cpufeature.h |  1 +
 arch/x86/include/asm/kvm_host.h   |  3 +++
 arch/x86/include/asm/msr-index.h  |  1 +
 arch/x86/kvm/cpuid.c              |  2 ++
 arch/x86/kvm/cpuid.h              |  8 ++++++++
 arch/x86/kvm/svm.c                |  7 +++++++
 arch/x86/kvm/vmx.c                |  9 +++++++++
 arch/x86/kvm/x86.c                | 22 ++++++++++++++++++++++
 8 files changed, 53 insertions(+)
arch/x86/include/asm/cpufeature.h +0 -0
index 6b7ee5f..e574d81 100644
arch/x86/include/asm/cpufeature.h line 200
Quoted 2 lines
arch/x86/include/asm/cpufeature.h +0 -0
@@ -199,6 +199,7 @@
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word
9 */
arch/x86/include/asm/cpufeature.h line 201
Quoted 1 line
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_FSGSBASE	(9*32+ 0) /* {RD/WR}{FS/GS}BASE
instructions*/
arch/x86/include/asm/cpufeature.h line 204
Quoted 3 lines
arch/x86/include/asm/cpufeature.h +2 -0
+#define X86_FEATURE_TSC_ADJUST  (9*32+ 1) /* TSC adjustment MSR 0x3b
+*/
 #define X86_FEATURE_BMI1	(9*32+ 3) /* 1st group bit manipulation
extensions */
arch/x86/include/asm/cpufeature.h line 205
Quoted 1 line
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_HLE		(9*32+ 4) /* Hardware Lock Elision
*/
arch/x86/include/asm/kvm_host.h struct kvm_vcpu_arch { · line 448
Quoted 13 lines
arch/x86/include/asm/cpufeature.h +0 -0
 #define X86_FEATURE_AVX2	(9*32+ 5) /* AVX2 instructions */
(quoted diff) +0 -0
b/arch/x86/include/asm/kvm_host.h index da34027..cf8c7e0 100644
arch/x86/include/asm/kvm_host.h +2 -0
@@ -442,6 +442,8 @@ struct kvm_vcpu_arch {
 	u32 virtual_tsc_mult;
 	u32 virtual_tsc_khz;
 
+	s64 ia32_tsc_adjust_msr;
+
 	atomic_t nmi_queued;  /* unprocessed asynchronous NMIs */
 	unsigned nmi_pending; /* NMI queued after currently running
handler */
arch/x86/kvm/cpuid.c static int do_cpuid_ent(struct kvm_cpuid_entry2
Quoted 28 lines
arch/x86/include/asm/kvm_host.h +1 -0
 	bool nmi_injected;    /* Trying to inject an NMI this entry */
@@ -690,6 +692,7 @@ struct kvm_x86_ops {
 	bool (*has_wbinvd_exit)(void);
 
 	void (*set_tsc_khz)(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool
 scale);
+	u64 (*read_tsc_offset)(struct kvm_vcpu *vcpu);
 	void (*write_tsc_offset)(struct kvm_vcpu *vcpu, u64 offset);
 
arch/x86/include/asm/kvm_host.h +0 -0
 	u64 (*compute_tsc_offset)(struct kvm_vcpu *vcpu, u64 target_tsc);
arch/x86/include/asm/msr-index.h +1 -0
index 957ec87..6486569 100644
@@ -231,6 +231,7 @@
 #define MSR_IA32_EBL_CR_POWERON		0x0000002a
 #define MSR_EBC_FREQUENCY_ID		0x0000002c
 #define MSR_IA32_FEATURE_CONTROL        0x0000003a
+#define MSR_IA32_TSC_ADJUST             0x0000003b
 
 #define FEATURE_CONTROL_LOCKED				(1<<0)
 #define FEATURE_CONTROL_VMXON_ENABLED_INSIDE_SMX	(1<<1)
arch/x86/kvm/cpuid.c index renamed from arch/x86/kvm/cpuid.c +0 -0
0595f13..e817bac 100644
arch/x86/kvm/cpuid.c +0 -0
@@ -320,6 +320,8 @@ static int do_cpuid_ent(struct kvm_cpuid_entry2
*entry, u32 function,
arch/x86/kvm/cpuid.h static inline bool guest_cpuid_has_xsave(struct
Quoted 13 lines
arch/x86/kvm/cpuid.c +2 -0
 		if (index == 0) {
 			entry->ebx &= kvm_supported_word9_x86_features;
 			cpuid_mask(&entry->ebx, 9);
+			// TSC_ADJUST is emulated
+			entry->ebx |= F(TSC_ADJUST);
 		} else
 			entry->ebx = 0;
 		entry->eax = 0;
arch/x86/kvm/cpuid.h index renamed from arch/x86/kvm/cpuid.h +0 -0
a10e460..3a8b504 100644
arch/x86/kvm/cpuid.h +0 -0
@@ -28,6 +28,14 @@ static inline bool guest_cpuid_has_xsave(struct
kvm_vcpu *vcpu)
arch/x86/kvm/svm.c static void svm_set_tsc_khz(struct kvm_vcpu
Quoted 16 lines
arch/x86/kvm/cpuid.h +7 -0
 	return best && (best->ecx & bit(X86_FEATURE_XSAVE));  }
 
+static inline bool guest_cpuid_has_tsc_adjust(struct kvm_vcpu *vcpu)
+{
+	struct kvm_cpuid_entry2 *best;
+
+	best = kvm_find_cpuid_entry(vcpu, 7, 0);
+	return best && (best->ebx & bit(X86_FEATURE_TSC_ADJUST)); }
+
 static inline bool guest_cpuid_has_smep(struct kvm_vcpu *vcpu)  {
 	struct kvm_cpuid_entry2 *best;
arch/x86/kvm/svm.c index renamed from arch/x86/kvm/svm.c +0 -0
5ac11f0..7f5e6eb 100644
arch/x86/kvm/svm.c +0 -0
@@ -1012,6 +1012,12 @@ static void svm_set_tsc_khz(struct kvm_vcpu
*vcpu, u32 user_tsc_khz, bool scale)
arch/x86/kvm/vmx.c static void vmx_set_tsc_khz(struct kvm_vcpu
Quoted 23 lines
arch/x86/kvm/svm.c +4 -0
 	svm->tsc_ratio             = ratio;
 }
 
+static u64 svm_read_tsc_offset(struct kvm_vcpu *vcpu) {
+	struct vcpu_svm *svm = to_svm(vcpu);
+	return svm->vmcb->control.tsc_offset; }
+
 static void svm_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 {
 	struct vcpu_svm *svm = to_svm(vcpu); @@ -4333,6 +4339,7 @@ static
struct kvm_x86_ops svm_x86_ops = {
arch/x86/kvm/svm.c +1 -0
 	.has_wbinvd_exit = svm_has_wbinvd_exit,
 
 	.set_tsc_khz = svm_set_tsc_khz,
+	.read_tsc_offset = svm_read_tsc_offset,
 	.write_tsc_offset = svm_write_tsc_offset,
 	.adjust_tsc_offset = svm_adjust_tsc_offset,
 	.compute_tsc_offset = svm_compute_tsc_offset, diff --git
a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 819970f..0a73e72 100644
arch/x86/kvm/vmx.c +0 -0
@@ -1861,6 +1861,11 @@ static void vmx_set_tsc_khz(struct kvm_vcpu
*vcpu, u32 user_tsc_khz, bool scale)
arch/x86/kvm/vmx.c static void vmx_set_tsc_khz(struct kvm_vcpu *vcpu, u32 user_tsc_khz, bool scale) · line 1861
Quoted 1 line
arch/x86/kvm/vmx.c +0 -0
 		WARN(1, "user requested TSC rate below hardware speed\n");
}
arch/x86/kvm/vmx.c static int vmx_set_msr(struct kvm_vcpu *vcpu,
Quoted 9 lines
arch/x86/kvm/vmx.c +4 -0
+static u64 vmx_read_tsc_offset(struct kvm_vcpu *vcpu) {
+	return vmcs_read64(TSC_OFFSET);
+}
+
 /*
  * writes 'offset' into guest's timestamp counter offset register
  */
@@ -2243,6 +2248,9 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu,
struct msr_data *msr_info)
arch/x86/kvm/x86.c static u64 compute_guest_tsc(struct kvm_vcpu
Quoted 29 lines
arch/x86/kvm/vmx.c +4 -0
 		}
 		ret = kvm_set_msr_common(vcpu, msr_info);
+	case MSR_IA32_TSC_ADJUST:
+		ret = kvm_set_msr_common(vcpu, msr_info);
+		break;
 	case MSR_TSC_AUX:
 		if (!vmx->rdtscp_enabled)
 			return 1;
@@ -7338,6 +7346,7 @@ static struct kvm_x86_ops vmx_x86_ops = {
 	.has_wbinvd_exit = cpu_has_vmx_wbinvd_exit,
 
 	.set_tsc_khz = vmx_set_tsc_khz,
+	.read_tsc_offset = vmx_read_tsc_offset,
 	.write_tsc_offset = vmx_write_tsc_offset,
 	.adjust_tsc_offset = vmx_adjust_tsc_offset,
 	.compute_tsc_offset = vmx_compute_tsc_offset, diff --git
a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index edafa29..7c2990b 100644
arch/x86/kvm/x86.c +1 -0
@@ -824,6 +824,7 @@ static u32 msrs_to_save[] = {  static unsigned
 num_msrs_to_save;
 
 static u32 emulated_msrs[] = {
+	MSR_IA32_TSC_ADJUST,
 	MSR_IA32_TSCDEADLINE,
 	MSR_IA32_MISC_ENABLE,
 	MSR_IA32_MCG_STATUS,
@@ -1047,6 +1048,12 @@ static u64 compute_guest_tsc(struct kvm_vcpu
*vcpu, s64 kernel_ns)
arch/x86/kvm/x86.c static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns) · line 1053
Quoted 6 lines
arch/x86/kvm/x86.c +3 -0
 	return tsc;
 }
 
+static void update_ia32_tsc_adjust_msr(struct kvm_vcpu *vcpu, s64
+offset) {
+	u64 curr_offset =  kvm_x86_ops->read_tsc_offset(vcpu);
Extra space.
Other than that, looks fine. Remaining comments:
- Basic functional test must be improved (testing that wrmsr(tsc, val) reflects on tsc_adjust and wrmsr(tsc_adjust_msr, val) reflects on tsc is enough.
- Behaviour on reset: what is the behaviour on RESET?
- Please rebase against queue branch on kvm.git.
Thanks

Marcelo Tosatti

2012-11-29 20:43 UTC #6 lore raw

Quoted 3 lines · Will

Marcelo,
The behavior on reset is to return the TSC_AJUST msr value to 0x0. I am currently initializing this emulated msr in kvm_arch_vcpu_init().

Will,

Reset is handled by QEMU. kvm_arch_vcpu_init is only called during vcpu allocation (ie vm creation). See x86_cpu_reset in QEMU's target-i386/cpu.c file.

Quoted 4 lines

Quoted 2 lines
- Behaviour on reset: what is the behaviour on RESET?
I am testing the rebase now. I would like to get any needed changes for this initialization into my next patch set version (v6) if possible.

For the kernel patch, everything is fine. Apparently for the QEMU patch it is necessary to set value to zero on reset.

Auld, Will

2012-11-30 00:21 UTC #7 lore raw

Marcelo,

My latest QEMU patch ([PATCH V5] target-i386: Enabling IA32_TSC_ADJUST for QEMU KVM guest VMs) seems to be OK with regards to reset. The tsc_adjust variable is being zeroed by the memset() in x86_cpu_reset(). Later code seems to write these values through kvm_put_msr() by way of cpu_synchronize_all_post_reset().

Andreas said: I'm expecting this to go through Marcello's queue unless I'm told otherwise.

Does this meet your expectations as well?

Thanks,

Will

Quoted 30 lines

-----Original Message-----
From: Marcelo Tosatti [mailto:[email protected]]
Sent: Thursday, November 29, 2012 12:44 PM
To: Auld, Will
Cc: [email protected]; Dugger, Donald D; Liu, Jinsong; Zhang, Xiantao; [email protected]; qemu-devel; Gleb
Subject: Re: [PATCH V5 2/2] Enabling IA32_TSC_ADJUST for KVM guest VM support
Quoted 3 lines · Will
Marcelo,
The behavior on reset is to return the TSC_AJUST msr value to 0x0. I
am currently initializing this emulated msr in kvm_arch_vcpu_init().
Will,
Reset is handled by QEMU. kvm_arch_vcpu_init is only called during vcpu allocation (ie vm creation). See x86_cpu_reset in QEMU's target- i386/cpu.c file.
Quoted 4 lines
Quoted 2 lines
- Behaviour on reset: what is the behaviour on RESET?
I am testing the rebase now. I would like to get any needed changes
for this initialization into my next patch set version (v6) if possible.
For the kernel patch, everything is fine. Apparently for the QEMU patch it is necessary to set value to zero on reset.