Change in coreboot[master]: security/intel/txt: Fix GETSEC checks in romstage

[Michał Żygowski (Code Review)]

Michał Żygowski has submitted this change. ( https://review.coreboot.org/c/coreboot/+/59520 )

Change subject: security/intel/txt: Fix GETSEC checks in romstage
......................................................................

security/intel/txt: Fix GETSEC checks in romstage

IA32_FEATURE_CONTROL does not need to be checked by BIOS, in fact these
bits are needed only by SENTER and SINIT ACM. ACM ENTERACCS does not
check these bits according to Intel SDM. Also noticed that the lock bit
of IA32_FEATURE_CONTROL cannot be cleared by issuing neither global
reset nor full reset on Sandybridge/Ivybridge platforms which results
in a reset loop. However, check the IA32_FEATURE_CONTROL SENTER bits in
ramstage where the register is properly set on all cores already.

TEST=Run ACM SCLEAN on Dell OptiPlex 9010 with i7-3770/Q77

Signed-off-by: Michał Żygowski 
Change-Id: Ie9103041498f557b85019a56e1252090a4fcd0c9
Reviewed-on: https://review.coreboot.org/c/coreboot/+/59520
Tested-by: build bot (Jenkins) 
Reviewed-by: Patrick Rudolph 
---
M src/security/intel/txt/getsec.c
M src/security/intel/txt/romstage.c
2 files changed, 31 insertions(+), 11 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Rudolph: Looks good to me, approved

diff --git a/src/security/intel/txt/getsec.c b/src/security/intel/txt/getsec.c
index af9b7bb..cd22927 100644
--- a/src/security/intel/txt/getsec.c
+++ b/src/security/intel/txt/getsec.c
@@ -9,6 +9,7 @@
 #include <cpu/x86/mp.h>
 #include <cpu/x86/msr.h>
 #include <types.h>
+#include <rules.h>

#include "txt_register.h"
 #include "txt_getsec.h"
@@ -24,16 +25,26 @@
    /*
     * Check if SMX and VMX is supported by CPU.
     */
-	if (!(ecx & CPUID_SMX) || !(ecx & CPUID_VMX))
+	if (!(ecx & CPUID_SMX) || !(ecx & CPUID_VMX)) {
+		printk(BIOS_ERR, "SMX/VMX not supported by CPU\n");
    	return false;
-
+	}
    /*
-	 * Check if SMX, VMX and GetSec instructions haven't been disabled.
+	 * This requirement is not needed for ENTERACCS, but for SENTER (see SDM).
+	 * Skip check in romstage because IA32_FEATURE_CONTROL cannot be unlocked
+	 * even after a global reset e.g. on Sandy/IvyBridge. However the register
+	 * gets set properly in ramstage where all CPUs are already initialized.
     */
-	msr_t msr = rdmsr(IA32_FEATURE_CONTROL);
-	if ((msr.lo & 0xff06) != 0xff06)
-		return false;
-
+	if (!ENV_ROMSTAGE_OR_BEFORE) {
+		/*
+		* Check if SMX, VMX and GetSec instructions haven't been disabled.
+		*/
+		msr_t msr = rdmsr(IA32_FEATURE_CONTROL);
+		if ((msr.lo & 0xff06) != 0xff06) {
+			printk(BIOS_ERR, "GETSEC not enabled in IA32_FEATURE_CONTROL MSR\n");
+			return false;
+		}
+	}
    /*
     * Enable SMX. Required to execute GetSec instruction.
     * Chapter 2.2.4.3
diff --git a/src/security/intel/txt/romstage.c b/src/security/intel/txt/romstage.c
index 63db10f..98308b7 100644
--- a/src/security/intel/txt/romstage.c
+++ b/src/security/intel/txt/romstage.c
@@ -5,6 +5,7 @@
 #include <cf9_reset.h>
 #include <console/console.h>
 #include <cpu/intel/common/common.h>
+#include <cpu/x86/cr.h>
 #include <cpu/x86/msr.h>
 #include <southbridge/intel/common/pmbase.h>
 #include <timer.h>
@@ -83,14 +84,22 @@
 void intel_txt_romstage_init(void)
 {
    /* Bail early if the CPU doesn't support TXT */
-	if (!is_txt_cpu())
+	if (!is_txt_cpu()) {
+		printk(BIOS_ERR, "TEE-TXT: CPU not TXT capable.\n");
    	return;
+	}

-	/* We need to use GETSEC here, so enable it */
-	enable_getsec_or_reset();
+	/*
+	 * We need to use GETSEC here, so enable it.
+	 * CR4_SMXE is all we need to be able to call GETSEC[CAPABILITIES]
+	 * or GETSEC[ENTERACCS] for SCLEAN.
+	 */
+	write_cr4(read_cr4() | CR4_SMXE);

-	if (!is_txt_chipset())
+	if (!is_txt_chipset()) {
+		printk(BIOS_ERR, "TEE-TXT: Chipset not TXT capable.\n");
    	return;
+	}

const uint8_t txt_ests = read8((void *)TXT_ESTS);

2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.