security/intel: Add option to enable SMM flash access only

On platforms where the boot media can be updated externally, e.g.
using a BMC, add the possibility to enable writes in SMM only. This
allows to protect the BIOS region even without the use of vboot, but
keeps SMMSTORE working for use in payloads. Note that this breaks
flashconsole, since the flash becomes read-only.

Tested on Asrock B85M Pro4 and HP 280 G2, SMM BIOS write protection
works as expected, and SMMSTORE can still be used.

Change-Id: I157db885b5f1d0f74009ede6fb2342b20d9429fa
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Signed-off-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/40830
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Patrick Rudolph <siro@das-labor.org>

7 files changed, 95 insertions, 35 deletions

diff --git a/src/security/lockdown/Kconfig b/src/security/lockdown/Kconfig
index 9d83a45b1d..8d48beb766 100644
--- a/src/security/lockdown/Kconfig
+++ b/src/security/lockdown/Kconfig
@@ -84,6 +84,17 @@ config BOOTMEDIA_LOCK_IN_VERSTAGE
 	  ramstage, like the MRC cache for example.
 	  Use this option if you don't trust code running after verstage.
 
+config BOOTMEDIA_SMM_BWP
+	bool "Boot media only writable in SMM"
+	depends on !CONSOLE_SPI_FLASH
+	depends on BOOT_DEVICE_SPI_FLASH && HAVE_SMI_HANDLER
+	depends on SOUTHBRIDGE_INTEL_COMMON_SPI || SOC_INTEL_COMMON_BLOCK_SPI
+	select SOC_INTEL_COMMON_BLOCK_SMM_TCO_ENABLE if SOC_INTEL_COMMON_BLOCK_SPI
+	help
+	  Only allow flash writes in SMM. Select this if you want to use SMMSTORE
+	  while also preventing unauthorized writes through the internal controller.
+	  Note that this breaks flashconsole, since the flash becomes read-only.
+
 choice
 	prompt "SPI Flash write protection duration"
 	default BOOTMEDIA_SPI_LOCK_REBOOT
diff --git a/src/soc/intel/common/block/fast_spi/fast_spi.c b/src/soc/intel/common/block/fast_spi/fast_spi.c
index 15cd26e2e2..843071e2a8 100644
--- a/src/soc/intel/common/block/fast_spi/fast_spi.c
+++ b/src/soc/intel/common/block/fast_spi/fast_spi.c
@@ -418,3 +418,14 @@ void fast_spi_enable_wp(void)
 	bios_cntl &= ~SPI_BIOS_CONTROL_WPD;
 	pci_write_config8(dev, SPI_BIOS_CONTROL, bios_cntl);
 }
+
+/* Disable SPI Write Protect. */
+void fast_spi_disable_wp(void)
+{
+	const pci_devfn_t dev = PCH_DEV_SPI;
+	uint8_t bios_cntl;
+
+	bios_cntl = pci_read_config8(dev, SPI_BIOS_CONTROL);
+	bios_cntl |= SPI_BIOS_CONTROL_WPD;
+	pci_write_config8(dev, SPI_BIOS_CONTROL, bios_cntl);
+}
diff --git a/src/soc/intel/common/block/include/intelblocks/fast_spi.h b/src/soc/intel/common/block/include/intelblocks/fast_spi.h
index d0442566b3..d5d3cd560c 100644
--- a/src/soc/intel/common/block/include/intelblocks/fast_spi.h
+++ b/src/soc/intel/common/block/include/intelblocks/fast_spi.h
@@ -77,6 +77,10 @@ bool fast_spi_wpd_status(void);
  */
 void fast_spi_enable_wp(void);
 /*
+ * Disable SPI Write protect.
+ */
+void fast_spi_disable_wp(void);
+/*
  * Get base and size of extended BIOS decode window used at runtime in host address space. If
  * the BIOS region is not greater than 16MiB, then this function returns 0 for both base and
  * size.
diff --git a/src/soc/intel/common/block/smm/smihandler.c b/src/soc/intel/common/block/smm/smihandler.c
index 91142a51ce..5789b4843e 100644
--- a/src/soc/intel/common/block/smm/smihandler.c
+++ b/src/soc/intel/common/block/smm/smihandler.c
@@ -44,31 +44,6 @@ __weak int smihandler_soc_disable_busmaster(pci_devfn_t dev)
 	return 1;
 }
 
-/*
- * Needs to implement the mechanism to know if an illegal attempt
- * has been made to write to the BIOS area.
- */
-static void smihandler_soc_check_illegal_access(
-	uint32_t tco_sts)
-{
-	if (!((tco_sts & (1 << 8)) && CONFIG(SPI_FLASH_SMM)
-			&& fast_spi_wpd_status()))
-		return;
-
-	/*
-	 * BWE is RW, so the SMI was caused by a
-	 * write to BWE, not by a write to the BIOS
-	 *
-	 * This is the place where we notice someone
-	 * is trying to tinker with the BIOS. We are
-	 * trying to be nice and just ignore it. A more
-	 * resolute answer would be to power down the
-	 * box.
-	 */
-	printk(BIOS_DEBUG, "Switching back to RO\n");
-	fast_spi_enable_wp();
-}
-
 /* Mainboard overrides. */
 
 __weak void mainboard_smi_gpi_handler(
@@ -301,9 +276,19 @@ static void southbridge_smi_store(
 	/* Parameter buffer in EBX */
 	reg_ebx = save_state_ops->get_reg(io_smi, RBX);
 
+	const bool wp_enabled = !fast_spi_wpd_status();
+	if (wp_enabled) {
+		fast_spi_disable_wp();
+		/* Not clearing SPI sync SMI status here results in hangs */
+		fast_spi_clear_sync_smi_status();
+	}
+
 	/* drivers/smmstore/smi.c */
 	ret = smmstore_exec(sub_command, (void *)(uintptr_t)reg_ebx);
 	save_state_ops->set_reg(io_smi, RAX, ret);
+
+	if (wp_enabled)
+		fast_spi_enable_wp();
 }
 
 static void finalize(void)
@@ -320,6 +305,9 @@ static void finalize(void)
 		/* Re-init SPI driver to handle locked BAR */
 		fast_spi_init();
 
+	if (CONFIG(BOOTMEDIA_SMM_BWP))
+		fast_spi_enable_wp();
+
 	/*
 	 * HECI is disabled in smihandler_soc_at_finalize() which also locks down the side band
 	 * interface.  Some boards may require this interface in mainboard_smi_finalize(),
@@ -401,12 +389,26 @@ void smihandler_southbridge_tco(
 	 */
 	fast_spi_clear_sync_smi_status();
 
+	/* If enabled, enforce SMM BIOS write protection */
+	if (CONFIG(BOOTMEDIA_SMM_BWP) && fast_spi_wpd_status()) {
+		/*
+		 * BWE is RW, so the SMI was caused by a
+		 * write to BWE, not by a write to the BIOS
+		 *
+		 * This is the place where we notice someone
+		 * is trying to tinker with the BIOS. We are
+		 * trying to be nice and just ignore it. A more
+		 * resolute answer would be to power down the
+		 * box.
+		 */
+		printk(BIOS_DEBUG, "Switching SPI back to RO\n");
+		fast_spi_enable_wp();
+	}
+
 	/* Any TCO event? */
 	if (!tco_sts)
 		return;
 
-	smihandler_soc_check_illegal_access(tco_sts);
-
 	if (tco_sts & TCO_TIMEOUT) { /* TIMEOUT */
 		/* Handle TCO timeout */
 		printk(BIOS_DEBUG, "TCO Timeout.\n");
diff --git a/src/soc/intel/common/pch/lockdown/lockdown.c b/src/soc/intel/common/pch/lockdown/lockdown.c
index d9495a432b..87f36fc91c 100644
--- a/src/soc/intel/common/pch/lockdown/lockdown.c
+++ b/src/soc/intel/common/pch/lockdown/lockdown.c
@@ -65,6 +65,12 @@ static void fast_spi_lockdown_cfg(int chipset_lockdown)
 		/* BIOS Interface Lock */
 		fast_spi_set_bios_interface_lock_down();
 
+		/* Only allow writes in SMM */
+		if (CONFIG(BOOTMEDIA_SMM_BWP)) {
+			//fast_spi_set_eiss();	/* TODO */
+			fast_spi_enable_wp();
+		}
+
 		/* BIOS Lock */
 		fast_spi_set_lock_enable();
 
diff --git a/src/southbridge/intel/common/finalize.c b/src/southbridge/intel/common/finalize.c
index 975d839e67..6fb27bbb4b 100644
--- a/src/southbridge/intel/common/finalize.c
+++ b/src/southbridge/intel/common/finalize.c
@@ -45,6 +45,9 @@ void intel_pch_finalize_smm(void)
 	pci_write_config32(PCI_DEV(0, 27, 0), 0x74,
 		    pci_read_config32(PCI_DEV(0, 27, 0), 0x74));
 
+	if (CONFIG(BOOTMEDIA_SMM_BWP))
+		write_pmbase16(SMI_EN, read_pmbase16(SMI_EN) | TCO_EN);
+
 	write_pmbase16(TCO1_CNT, read_pmbase16(TCO1_CNT) | TCO_LOCK);
 
 	post_code(POST_OS_BOOT);
diff --git a/src/southbridge/intel/common/spi.c b/src/southbridge/intel/common/spi.c
index 828f3d3e3d..30f7657948 100644
--- a/src/southbridge/intel/common/spi.c
+++ b/src/southbridge/intel/common/spi.c
@@ -27,6 +27,8 @@
 
 static int spi_is_multichip(void);
 
+static void spi_set_smm_only_flashing(bool enable);
+
 struct ich7_spi_regs {
 	uint16_t spis;
 	uint16_t spic;
@@ -282,7 +284,6 @@ static void *get_spi_bar(pci_devfn_t dev)
 
 void spi_init(void)
 {
-	uint8_t bios_cntl;
 	struct ich9_spi_regs *ich9_spi;
 	struct ich7_spi_regs *ich7_spi;
 	uint16_t hsfs;
@@ -332,13 +333,8 @@ void spi_init(void)
 
 	ich_set_bbar(0);
 
-	if (CONFIG(SOUTHBRIDGE_INTEL_I82801GX) || CONFIG(SOUTHBRIDGE_INTEL_COMMON_SPI_ICH9)) {
-		/* Disable the BIOS write protect so write commands are allowed. */
-		bios_cntl = pci_read_config8(dev, 0xdc);
-		/* Deassert SMM BIOS Write Protect Disable. */
-		bios_cntl &= ~(1 << 5);
-		pci_write_config8(dev, 0xdc, bios_cntl | 0x1);
-	}
+	/* Disable the BIOS write protect so write commands are allowed. */
+	spi_set_smm_only_flashing(false);
 }
 
 static int spi_locked(void)
@@ -1096,12 +1092,39 @@ void spi_finalize_ops(void)
 		writeb_(spi_config->ops[i].op, &cntlr.opmenu[i]);
 	}
 	writew_(optype, cntlr.optype);
+
+	spi_set_smm_only_flashing(CONFIG(BOOTMEDIA_SMM_BWP));
 }
 
 __weak void intel_southbridge_override_spi(struct intel_swseq_spi_config *spi_config)
 {
 }
 
+#define BIOS_CNTL		0xdc
+#define  BIOS_CNTL_BIOSWE	(1 << 0)
+#define  BIOS_CNTL_BLE		(1 << 1)
+#define  BIOS_CNTL_SMM_BWP	(1 << 5)
+
+static void spi_set_smm_only_flashing(bool enable)
+{
+	if (!(CONFIG(SOUTHBRIDGE_INTEL_I82801GX) || CONFIG(SOUTHBRIDGE_INTEL_COMMON_SPI_ICH9)))
+		return;
+
+	const pci_devfn_t dev = PCI_DEV(0, 31, 0);
+
+	uint8_t bios_cntl = pci_read_config8(dev, BIOS_CNTL);
+
+	if (enable) {
+		bios_cntl &= ~BIOS_CNTL_BIOSWE;
+		bios_cntl |= BIOS_CNTL_BLE | BIOS_CNTL_SMM_BWP;
+	} else {
+		bios_cntl &= ~(BIOS_CNTL_BLE | BIOS_CNTL_SMM_BWP);
+		bios_cntl |= BIOS_CNTL_BIOSWE;
+	}
+
+	pci_write_config8(dev, BIOS_CNTL, bios_cntl);
+}
+
 static const struct spi_ctrlr spi_ctrlr = {
 	.xfer_vector = xfer_vectors,
 	.max_xfer_size = member_size(struct ich9_spi_regs, fdata),