Re: PATCH: nVMX: Better MSR_IA32_FEATURE_CONTROL handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



The existing code emulated the guest's use of the IA32_FEATURE_CONTROL MSR
in a way that was enough to run nested VMX guests, but did not fully
conform to the VMX specification, and in particular did not allow a guest
BIOS to prevent the guest OS from using VMX by setting the lock bit on this
MSR.

This patch emulates this MSR better, allowing the guest to lock it, and
verifying its setting on VMXON. Also make sure that this MSR (and of course,
VMXON state) is reset on guest vcpu reset (via SIPI).

Signed-off-by: Nadav Har'El <nyh@xxxxxxxxxx>
Reported-by: Julian Stecklina <js@xxxxxxxxx>
---
 arch/x86/kvm/vmx.c |   43 +++++++++++++++++++++++--------------------
 arch/x86/kvm/x86.c |    3 ++-
 2 files changed, 25 insertions(+), 21 deletions(-)

--- .before/arch/x86/kvm/vmx.c	2012-03-19 18:34:24.000000000 +0200
+++ .after/arch/x86/kvm/vmx.c	2012-03-19 18:34:24.000000000 +0200
@@ -352,6 +352,7 @@ struct nested_vmx {
 	 * we must keep them pinned while L2 runs.
 	 */
 	struct page *apic_access_page;
+	u64 msr_ia32_feature_control;
 };
 
 struct vcpu_vmx {
@@ -1998,9 +1999,6 @@ static int vmx_get_vmx_msr(struct kvm_vc
 	}
 
 	switch (msr_index) {
-	case MSR_IA32_FEATURE_CONTROL:
-		*pdata = 0;
-		break;
 	case MSR_IA32_VMX_BASIC:
 		/*
 		 * This MSR reports some information about VMX support. We
@@ -2072,21 +2070,6 @@ static int vmx_get_vmx_msr(struct kvm_vc
 	return 1;
 }
 
-static int vmx_set_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data)
-{
-	if (!nested_vmx_allowed(vcpu))
-		return 0;
-
-	if (msr_index == MSR_IA32_FEATURE_CONTROL)
-		/* TODO: the right thing. */
-		return 1;
-	/*
-	 * No need to treat VMX capability MSRs specially: If we don't handle
-	 * them, handle_wrmsr will #GP(0), which is correct (they are readonly)
-	 */
-	return 0;
-}
-
 /*
  * Reads an msr value (of 'msr_index') into 'pdata'.
  * Returns 0 on success, non-0 otherwise.
@@ -2129,6 +2112,9 @@ static int vmx_get_msr(struct kvm_vcpu *
 	case MSR_IA32_SYSENTER_ESP:
 		data = vmcs_readl(GUEST_SYSENTER_ESP);
 		break;
+	case MSR_IA32_FEATURE_CONTROL:
+		data = to_vmx(vcpu)->nested.msr_ia32_feature_control;
+		break;
 	case MSR_TSC_AUX:
 		if (!to_vmx(vcpu)->rdtscp_enabled)
 			return 1;
@@ -2197,6 +2183,12 @@ static int vmx_set_msr(struct kvm_vcpu *
 		}
 		ret = kvm_set_msr_common(vcpu, msr_index, data);
 		break;
+	case MSR_IA32_FEATURE_CONTROL:
+		if (to_vmx(vcpu)->nested.msr_ia32_feature_control
+				& FEATURE_CONTROL_LOCKED)
+			return 1;
+		to_vmx(vcpu)->nested.msr_ia32_feature_control = data;
+		break;
 	case MSR_TSC_AUX:
 		if (!vmx->rdtscp_enabled)
 			return 1;
@@ -2205,8 +2197,6 @@ static int vmx_set_msr(struct kvm_vcpu *
 			return 1;
 		/* Otherwise falls through */
 	default:
-		if (vmx_set_vmx_msr(vcpu, msr_index, data))
-			break;
 		msr = find_msr_entry(vmx, msr_index);
 		if (msr) {
 			msr->data = data;
@@ -3807,6 +3797,8 @@ static int vmx_vcpu_setup(struct vcpu_vm
 	return 0;
 }
 
+static void free_nested(struct vcpu_vmx *vmx);
+
 static int vmx_vcpu_reset(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -3920,6 +3912,9 @@ static int vmx_vcpu_reset(struct kvm_vcp
 	/* HACK: Don't enable emulation on guest boot/reset */
 	vmx->emulation_required = 0;
 
+	/* Reset nested-VMX settings: */
+	vmx->nested.msr_ia32_feature_control = 0;
+	free_nested(vmx);
 out:
 	return ret;
 }
@@ -5031,6 +5026,14 @@ static int handle_vmon(struct kvm_vcpu *
 		return 1;
 	}
 
+#define VMXON_NEEDED_FEATURES \
+	  (FEATURE_CONTROL_LOCKED | FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX)
+	if ((vmx->nested.msr_ia32_feature_control & VMXON_NEEDED_FEATURES)
+			!= VMXON_NEEDED_FEATURES) {
+		kvm_inject_gp(vcpu, 0);
+		return 1;
+	}
+
 	INIT_LIST_HEAD(&(vmx->nested.vmcs02_pool));
 	vmx->nested.vmcs02_num = 0;
 
--- .before/arch/x86/kvm/x86.c	2012-03-19 18:34:24.000000000 +0200
+++ .after/arch/x86/kvm/x86.c	2012-03-19 18:34:24.000000000 +0200
@@ -799,7 +799,8 @@ static u32 msrs_to_save[] = {
 #ifdef CONFIG_X86_64
 	MSR_CSTAR, MSR_KERNEL_GS_BASE, MSR_SYSCALL_MASK, MSR_LSTAR,
 #endif
-	MSR_IA32_TSC, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA
+	MSR_IA32_TSC, MSR_IA32_CR_PAT, MSR_VM_HSAVE_PA,
+	MSR_IA32_FEATURE_CONTROL
 };
 
 static unsigned num_msrs_to_save;
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux