Add support for RDTSCP in VMX

PATCH Applied Sheng Yang 3 participants 12 messages Dec 18, 2009 – Dec 23, 2009

Subsystems (3)

x86 KVM KVM x86

lore

Share & tools

lore

Sheng Yang

author

2009-12-18 08:48 UTC #0 lore raw

v2 cover Add support for RDTSCP in VMX

--
regards
Yang, Sheng

 arch/x86/include/asm/kvm_host.h  |    2 +
 arch/x86/include/asm/msr-index.h |    1 +
 arch/x86/include/asm/msr.h       |    4 +-
 arch/x86/include/asm/vmx.h       |    1 +
 arch/x86/kernel/vsyscall_64.c    |    3 +-
 arch/x86/kvm/svm.c               |   13 +++++++
 arch/x86/kvm/vmx.c               |   68 +++++++++++++++++++++++++++++++++++---
 arch/x86/kvm/x86.c               |   61 +++++++++++++++++++++-------------
 8 files changed, 122 insertions(+), 31 deletions(-)

Sheng Yang

author

2009-12-18 08:48 UTC #1 lore raw

PATCH v2 1/6 KVM: VMX: Remove redundant variable

It's no longer necessary.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/kvm/vmx.c |    2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

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

index 9a0a2cf..5c464ed 100644
@@ -2383,14 +2383,12 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx)
 	for (i = 0; i < NR_VMX_MSR; ++i) {
 		u32 index = vmx_msr_index[i];
 		u32 data_low, data_high;
-		u64 data;
 		int j = vmx->nmsrs;
 
 		if (rdmsr_safe(index, &data_low, &data_high) < 0)
 			continue;
 		if (wrmsr_safe(index, data_low, data_high) < 0)
 			continue;
-		data = data_low | ((u64)data_high << 32);
 		vmx->guest_msrs[j].index = i;
 		vmx->guest_msrs[j].data = 0;
 		vmx->guest_msrs[j].mask = -1ull;

Signature · 1 line

1.5.4.5

Sheng Yang

author

2009-12-18 08:48 UTC #2 lore raw

PATCH v2 2/6 x86: Add IA32_TSC_AUX MSR

In reply to Sheng Yang

Also replaced the hardcode value in write_tsc() and write_tscp_aux().

Signed-off-by Sheng Yang <[email protected]>

---

(Already applied by Ingo)

 arch/x86/include/asm/msr-index.h |    1 +
 arch/x86/include/asm/msr.h       |    4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

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

index 4ffe09b..ac98d29 100644
@@ -12,6 +12,7 @@
 #define MSR_FS_BASE		0xc0000100 /* 64bit FS base */
 #define MSR_GS_BASE		0xc0000101 /* 64bit GS base */
 #define MSR_KERNEL_GS_BASE	0xc0000102 /* SwapGS GS shadow */
+#define MSR_TSC_AUX		0xc0000103 /* Auxiliary TSC */
 
 /* EFER bits: */
 #define _EFER_SCE		0  /* SYSCALL/SYSRET */

arch/x86/include/asm/msr.h +2 -2

index 7e2b6ba..e61fb87 100644
@@ -240,9 +240,9 @@ do {                                                            \
 #define checking_wrmsrl(msr, val) wrmsr_safe((msr), (u32)(val),		\
 					     (u32)((val) >> 32))
 
-#define write_tsc(val1, val2) wrmsr(0x10, (val1), (val2))
+#define write_tsc(val1, val2) wrmsr(MSR_IA32_TSC, (val1), (val2))
 
-#define write_rdtscp_aux(val) wrmsr(0xc0000103, (val), 0)
+#define write_rdtscp_aux(val) wrmsr(MSR_TSC_AUX, (val), 0)
 
 #ifdef CONFIG_SMP
 int rdmsr_on_cpu(unsigned int cpu, u32 msr_no, u32 *l, u32 *h);

Signature · 1 line

1.5.4.5

Sheng Yang

author

2009-12-18 08:48 UTC #3 lore raw

PATCH v2 3/6 KVM: Extended shared_msr_global to per CPU

In reply to Sheng Yang

shared_msr_global saved host value of relevant MSRs, but it have an assumption that all MSRs it tracked shared the value across the different CPUs. It's not true with some MSRs, e.g. MSR_TSC_AUX.

Extend it to per CPU to provide the support of MSR_TSC_AUX, and more alike MSRs.

Notice now the shared_msr_global still have one assumption: it can only deal with the MSRs that won't change in host after KVM module loaded.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/kvm/x86.c |   55 +++++++++++++++++++++++++++++++--------------------
 1 files changed, 33 insertions(+), 22 deletions(-)

arch/x86/kvm/x86.c +33 ~~-22~~

index dd15d7a..9ce7917 100644
@@ -92,16 +92,16 @@ module_param_named(ignore_msrs, ignore_msrs, bool, S_IRUGO | S_IWUSR);
 
 struct kvm_shared_msrs_global {
 	int nr;
-	struct kvm_shared_msr {
-		u32 msr;
-		u64 value;
-	} msrs[KVM_NR_SHARED_MSRS];
+	u32 msrs[KVM_NR_SHARED_MSRS];
 };
 
 struct kvm_shared_msrs {
 	struct user_return_notifier urn;
 	bool registered;
-	u64 current_value[KVM_NR_SHARED_MSRS];
+	struct kvm_shared_msr_values {
+		u64 host;
+		u64 curr;
+	} values[KVM_NR_SHARED_MSRS];
 };
 
 static struct kvm_shared_msrs_global __read_mostly shared_msrs_global;

@@ -146,53 +146,64 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {
 static void kvm_on_user_return(struct user_return_notifier *urn)
 {
 	unsigned slot;
-	struct kvm_shared_msr *global;
 	struct kvm_shared_msrs *locals
 		= container_of(urn, struct kvm_shared_msrs, urn);
+	struct kvm_shared_msr_values *values;
 
 	for (slot = 0; slot < shared_msrs_global.nr; ++slot) {
-		global = &shared_msrs_global.msrs[slot];
-		if (global->value != locals->current_value[slot]) {
-			wrmsrl(global->msr, global->value);
-			locals->current_value[slot] = global->value;
+		values = &locals->values[slot];
+		if (values->host != values->curr) {
+			wrmsrl(shared_msrs_global.msrs[slot], values->host);
+			values->curr = values->host;
 		}
 	}
 	locals->registered = false;
 	user_return_notifier_unregister(urn);
 }
 
-void kvm_define_shared_msr(unsigned slot, u32 msr)
+static void shared_msr_update(unsigned slot, u32 msr)
 {
-	int cpu;
+	struct kvm_shared_msrs *smsr;
 	u64 value;
 
+	smsr = &__get_cpu_var(shared_msrs);
+	/* only read, and nobody should modify it at this time,
+	 * so don't need lock */
+	if (slot >= shared_msrs_global.nr) {
+		printk(KERN_ERR "kvm: invalid MSR slot!");
+		return;
+	}
+	rdmsrl_safe(msr, &value);
+	smsr->values[slot].host = value;
+	smsr->values[slot].curr = value;
+}
+
+void kvm_define_shared_msr(unsigned slot, u32 msr)
+{
 	if (slot >= shared_msrs_global.nr)
 		shared_msrs_global.nr = slot + 1;
-	shared_msrs_global.msrs[slot].msr = msr;
-	rdmsrl_safe(msr, &value);
-	shared_msrs_global.msrs[slot].value = value;
-	for_each_online_cpu(cpu)
-		per_cpu(shared_msrs, cpu).current_value[slot] = value;
+	shared_msrs_global.msrs[slot] = msr;
+	/* we need ensured the shared_msr_global have been updated */
+	smp_wmb();
 }
 EXPORT_SYMBOL_GPL(kvm_define_shared_msr);
 
 static void kvm_shared_msr_cpu_online(void)
 {
 	unsigned i;
-	struct kvm_shared_msrs *locals = &__get_cpu_var(shared_msrs);
 
 	for (i = 0; i < shared_msrs_global.nr; ++i)
-		locals->current_value[i] = shared_msrs_global.msrs[i].value;
+		shared_msr_update(i, shared_msrs_global.msrs[i]);
 }
 
 void kvm_set_shared_msr(unsigned slot, u64 value, u64 mask)
 {
 	struct kvm_shared_msrs *smsr = &__get_cpu_var(shared_msrs);
 
-	if (((value ^ smsr->current_value[slot]) & mask) == 0)
+	if (((value ^ smsr->values[slot].curr) & mask) == 0)
 		return;
-	smsr->current_value[slot] = value;
-	wrmsrl(shared_msrs_global.msrs[slot].msr, value);
+	smsr->values[slot].curr = value;
+	wrmsrl(shared_msrs_global.msrs[slot], value);
 	if (!smsr->registered) {
 		smsr->urn.on_user_return = kvm_on_user_return;
 		user_return_notifier_register(&smsr->urn);

Signature · 1 line

1.5.4.5

Sheng Yang

author

2009-12-18 08:48 UTC #4 lore raw

PATCH v2 4/6 x86: Raise vsyscall priority on hotplug notifier chain

In reply to Sheng Yang

KVM need vsyscall_init() to initialize MSR_TSC_AUX before it read the value. Per Avi's suggestion, this patch raised vsyscall priority on hotplug notifier chain, to 30.

CC Ingo Molnar <[email protected]>

CC [email protected]

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/kernel/vsyscall_64.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

arch/x86/kernel/vsyscall_64.c +2 -1

index 8cb4974..971bbc9 100644
@@ -300,7 +300,8 @@ static int __init vsyscall_init(void)
 	register_sysctl_table(kernel_root_table2);
 #endif
 	on_each_cpu(cpu_vsyscall_init, NULL, 1);
-	hotcpu_notifier(cpu_vsyscall_notifier, 0);
+	/* notifier priority > KVM */
+	hotcpu_notifier(cpu_vsyscall_notifier, 30);
 	return 0;
 }

Signature · 1 line

1.5.4.5

Sheng Yang

author

2009-12-18 08:48 UTC #5 lore raw

PATCH v2 5/6 KVM: Add cpuid_update() callback to kvm_x86_ops

In reply to Sheng Yang

Sometime, we need to adjust some state in order to reflect guest CPUID setting, e.g. if we don't expose rdtscp to guest, we won't want to enable it on hardware. cpuid_update() is introduced for this purpose.

Also export kvm_find_cpuid_entry() for later use.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/kvm/svm.c              |    6 ++++++
 arch/x86/kvm/vmx.c              |    6 ++++++
 arch/x86/kvm/x86.c              |    3 +++
 4 files changed, 16 insertions(+), 0 deletions(-)

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

index 4f865e8..d2a91ae 100644
@@ -471,6 +471,7 @@ struct kvm_x86_ops {
 	int (*hardware_setup)(void);               /* __init */
 	void (*hardware_unsetup)(void);            /* __exit */
 	bool (*cpu_has_accelerated_tpr)(void);
+	void (*cpuid_update)(struct kvm_vcpu *vcpu);
 
 	/* Create, but do not attach this VCPU */
 	struct kvm_vcpu *(*vcpu_create)(struct kvm *kvm, unsigned id);

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

index 3de0b37..d89b0a0 100644
@@ -2854,6 +2854,10 @@ static u64 svm_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 	return 0;
 }
 
+static void svm_cpuid_update(struct kvm_vcpu *vcpu)
+{
+}
+
 static const struct trace_print_flags svm_exit_reasons_str[] = {
 	{ SVM_EXIT_READ_CR0,           		"read_cr0" },
 	{ SVM_EXIT_READ_CR3,	      		"read_cr3" },

@@ -2978,6 +2982,8 @@ static struct kvm_x86_ops svm_x86_ops = {
 
 	.exit_reasons_str = svm_exit_reasons_str,
 	.gb_page_enable = svm_gb_page_enable,
+
+	.cpuid_update = svm_cpuid_update,
 };
 
 static int __init svm_init(void)

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

index 5c464ed..91dff23 100644
@@ -3961,6 +3961,10 @@ static bool vmx_gb_page_enable(void)
 	return false;
 }
 
+static void vmx_cpuid_update(struct kvm_vcpu *vcpu)
+{
+}
+
 static struct kvm_x86_ops vmx_x86_ops = {
 	.cpu_has_kvm_support = cpu_has_kvm_support,
 	.disabled_by_bios = vmx_disabled_by_bios,

@@ -4025,6 +4029,8 @@ static struct kvm_x86_ops vmx_x86_ops = {
 
 	.exit_reasons_str = vmx_exit_reasons_str,
 	.gb_page_enable = vmx_gb_page_enable,
+
+	.cpuid_update = vmx_cpuid_update,
 };
 
 static int __init vmx_init(void)

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

index 9ce7917..e16dede 100644
@@ -1574,6 +1574,7 @@ static int kvm_vcpu_ioctl_set_cpuid(struct kvm_vcpu *vcpu,
 	cpuid_fix_nx_cap(vcpu);
 	r = 0;
 	kvm_apic_set_version(vcpu);
+	kvm_x86_ops->cpuid_update(vcpu);
 
 out_free:
 	vfree(cpuid_entries);

@@ -1596,6 +1597,7 @@ static int kvm_vcpu_ioctl_set_cpuid2(struct kvm_vcpu *vcpu,
 		goto out;
 	vcpu->arch.cpuid_nent = cpuid->nent;
 	kvm_apic_set_version(vcpu);
+	kvm_x86_ops->cpuid_update(vcpu);
 	return 0;
 
 out:

@@ -3730,6 +3732,7 @@ struct kvm_cpuid_entry2 *kvm_find_cpuid_entry(struct kvm_vcpu *vcpu,
 	}
 	return best;
 }
+EXPORT_SYMBOL_GPL(kvm_find_cpuid_entry);
 
 int cpuid_maxphyaddr(struct kvm_vcpu *vcpu)
 {

Signature · 1 line

1.5.4.5

Sheng Yang

author

2009-12-18 08:48 UTC #6 lore raw

PATCH v2 6/6 KVM: VMX: Add instruction rdtscp support for guest

In reply to Sheng Yang

Before enabling, execution of "rdtscp" in guest would result in #UD.

Signed-off-by Sheng Yang <[email protected]>

---
 arch/x86/include/asm/kvm_host.h |    1 +
 arch/x86/include/asm/vmx.h      |    1 +
 arch/x86/kvm/svm.c              |    7 ++++
 arch/x86/kvm/vmx.c              |   60 +++++++++++++++++++++++++++++++++++++--
 arch/x86/kvm/x86.c              |    3 +-
 5 files changed, 68 insertions(+), 4 deletions(-)

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

index d2a91ae..07e6636 100644
@@ -533,6 +533,7 @@ struct kvm_x86_ops {
 	int (*get_tdp_level)(void);
 	u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio);
 	bool (*gb_page_enable)(void);
+	bool (*rdtscp_supported)(void);
 
 	const struct trace_print_flags *exit_reasons_str;
 };

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

index 2b49454..8c39320 100644
@@ -53,6 +53,7 @@
  */
 #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001
 #define SECONDARY_EXEC_ENABLE_EPT               0x00000002
+#define SECONDARY_EXEC_RDTSCP			0x00000008
 #define SECONDARY_EXEC_ENABLE_VPID              0x00000020
 #define SECONDARY_EXEC_WBINVD_EXITING		0x00000040
 #define SECONDARY_EXEC_UNRESTRICTED_GUEST	0x00000080

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

index d89b0a0..3a880ac 100644
@@ -2916,6 +2916,11 @@ static bool svm_gb_page_enable(void)
 	return true;
 }
 
+static bool svm_rdtscp_supported(void)
+{
+	return false;
+}
+
 static struct kvm_x86_ops svm_x86_ops = {
 	.cpu_has_kvm_support = has_svm,
 	.disabled_by_bios = is_disabled,

@@ -2984,6 +2989,8 @@ static struct kvm_x86_ops svm_x86_ops = {
 	.gb_page_enable = svm_gb_page_enable,
 
 	.cpuid_update = svm_cpuid_update,
+
+	.rdtscp_supported = svm_rdtscp_supported,
 };
 
 static int __init svm_init(void)

arch/x86/kvm/vmx.c +57 -3

122 lines not on this page · raw message

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

index e16dede..91e4234 100644
@@ -1646,6 +1646,7 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 #else
 	unsigned f_lm = 0;
 #endif
+	unsigned f_rdtscp = kvm_x86_ops->rdtscp_supported() ? F(RDTSCP) : 0;
 
 	/* cpuid 1.edx */
 	const u32 kvm_supported_word0_x86_features =

@@ -1665,7 +1666,7 @@ static void do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 		F(MTRR) | F(PGE) | F(MCA) | F(CMOV) |
 		F(PAT) | F(PSE36) | 0 /* Reserved */ |
 		f_nx | 0 /* Reserved */ | F(MMXEXT) | F(MMX) |
-		F(FXSR) | F(FXSR_OPT) | f_gbpages | 0 /* RDTSCP */ |
+		F(FXSR) | F(FXSR_OPT) | f_gbpages | f_rdtscp |
 		0 /* Reserved */ | f_lm | F(3DNOWEXT) | F(3DNOW);
 	/* cpuid 1.ecx */
 	const u32 kvm_supported_word4_x86_features =

Signature · 1 line

1.5.4.5

Avi Kivity

Applied

2009-12-20 09:30 UTC #7 lore raw

In reply to Sheng Yang

On 12/18/2009 10:48 AM, Sheng Yang wrote:

Applied all, thanks.

Signature · 1 line

error compiling committee.c: too many arguments to function

Marcelo Tosatti

2009-12-22 17:26 UTC #8 lore raw

Quoted 3 lines · Avi Kivity

On 12/18/2009 10:48 AM, Sheng Yang wrote:
Applied all, thanks.

Need to save/restore MSR_TSC_AUX in qemu-kvm.

Avi Kivity

2009-12-22 18:26 UTC #9 lore raw

Quoted 8 lines · Marcelo Tosatti

Quoted 4 lines · Avi Kivity
On 12/18/2009 10:48 AM, Sheng Yang wrote:
Applied all, thanks.
Need to save/restore MSR_TSC_AUX in qemu-kvm.

Should be automatic. After all, we expose all the MSR list, so qemu can read it and save everything there.

Signature · 2 lines

I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Marcelo Tosatti

2009-12-23 13:37 UTC #10 lore raw

Quoted 5 lines · Avi Kivity

Quoted 4 lines · Marcelo Tosatti
Quoted 2 lines · Avi Kivity
On 12/18/2009 10:48 AM, Sheng Yang wrote:

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
Applied all, thanks.
Need to save/restore MSR_TSC_AUX in qemu-kvm.
Should be automatic. After all, we expose all the MSR list, so qemu can read it and save everything there.

Hum, thinking a bit more about this (and reading Glauber's previous attempt) its not entirely clear that using the list provided by KVM_GET_MSR_INDEX_LIST to automatically save/restore is a good thing:

- The MSR must be saved in QEMU's CPUState representation, and its likely that it won't exist prior to the addition there.

- What about savevm/loadvm versioning? Currently the QEMU migration protocol is unable to deal with a new kernel that supports, for example, MSR_TSC_AUX vs a kernel that does not (in case MSRs were automatically saved), with the same qemu-kvm version.

Avi Kivity

2009-12-23 13:44 UTC #11 lore raw

Quoted 8 lines · Marcelo Tosatti

Quoted 4 lines
Should be automatic. After all, we expose all the MSR list, so qemu can read it and save everything there.
Hum, thinking a bit more about this (and reading Glauber's previous attempt) its not entirely clear that using the list provided by KVM_GET_MSR_INDEX_LIST to automatically save/restore is a good thing:

Quoted 8 lines

- The MSR must be saved in QEMU's CPUState representation, and its likely that it won't exist prior to the addition there.
- What about savevm/loadvm versioning? Currently the QEMU migration protocol is unable to deal with a new kernel that supports, for example, MSR_TSC_AUX vs a kernel that does not (in case MSRs were automatically saved), with the same qemu-kvm version.

You're right.

Well, qemu knows about cpuid.rdtscp, so we can add it to vmstate if it is enabled. We should probably whitelist flags in qemu in the same way we whitelist them in kvm.

Signature · 1 line

error compiling committee.c: too many arguments to function