[coreboot] New patch to review for coreboot: 80b383f document Intel VMX locking behavior

[Mike Frysinger (vapier@chromium.org)]

Mike Frysinger (vapier at chromium.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/2326

-gerrit

commit 80b383f2acbf8130b8c0138318598854038da4ab
Author: Mike Frysinger <vapier at chromium.org>
Date:   Fri Feb 8 17:45:27 2013 -0500

    document Intel VMX locking behavior
    
    Add a comment explaining that the existing lock bit logic is correct
    and "as designed" even though the manual states otherwise.  This way
    people don't have to "just know" what is going on.
    
    Change-Id: I14e6763abfe339e034037b73db01d4ee634bb34d
    Signed-off-by: Mike Frysinger <vapier at chromium.org>
---
 src/cpu/intel/model_206ax/model_206ax_init.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/cpu/intel/model_206ax/model_206ax_init.c b/src/cpu/intel/model_206ax/model_206ax_init.c
index 64c32a0..71879e2 100644
--- a/src/cpu/intel/model_206ax/model_206ax_init.c
+++ b/src/cpu/intel/model_206ax/model_206ax_init.c
@@ -144,10 +144,23 @@ static void enable_vmx(void)
 
 	printk(BIOS_DEBUG, "%s VMX\n", enable ? "Enabling" : "Disabling");
 
+	/* Even though the Intel manual says you must set the lock bit in addition
+	 * to the VMX bit in order for VMX to work, it is incorrect.  Thus we leave
+	 * it unlocked for the OS to manage things itself.  This is good for a few
+	 * reasons:
+	 * - No need to reflash the bios just to toggle the lock bit.
+	 * - The VMX bits really really should match each other across cores, so
+	 *   hard locking it on one while another has the opposite setting can
+	 *   easily lead to crashes as code using VMX migrates between them.
+	 * - Vendors that want to "upsell" from a bios that disables+locks to
+	 *   one that doesn't is sleazy.
+	 * By leaving this to the OS (e.g. Linux), people can do exactly what they
+	 * want on the fly, and do it correctly (e.g. across multiple cores).
+	 */
 	if (enable) {
-			msr.lo |= (1 << 2);
-			if (regs.ecx & CPUID_SMX)
-				msr.lo |= (1 << 1);
+		msr.lo |= (1 << 2);
+		if (regs.ecx & CPUID_SMX)
+			msr.lo |= (1 << 1);
 	}
 
 	wrmsr(IA32_FEATURE_CONTROL, msr);