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@chromium.org>
Reviewed-on: http://review.coreboot.org/2326
Tested-by: build bot (Jenkins)
Reviewed-by: Peter Stuge <peter@stuge.se>

1 files 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 64c32a0bc9..71879e2b1f 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);