[coreboot-gerrit] Change in coreboot[master]: security/flash: Refactor flash protection logic

[Philipp Deppenwiese (Code Review)]

Philipp Deppenwiese has uploaded this change for review. ( https://review.coreboot.org/25283

Change subject: security/flash: Refactor flash protection logic
......................................................................

security/flash: Refactor flash protection logic

Change-Id: Ic0a859e6ce9aba32278f666a38a952ec8b4c1b4d
Signed-off-by: zaolin 
---
M src/drivers/mrc_cache/mrc_cache.c
M src/drivers/spi/spi_flash.c
M src/security/flash/Makefile.inc
M src/security/flash/flash.c
M src/security/flash/flash.h
A src/security/flash/wp.c
M src/security/vboot/vboot_handoff.c
7 files changed, 94 insertions(+), 33 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/83/25283/1

diff --git a/src/drivers/mrc_cache/mrc_cache.c b/src/drivers/mrc_cache/mrc_cache.c
index 1850034..2a1c48c 100644
--- a/src/drivers/mrc_cache/mrc_cache.c
+++ b/src/drivers/mrc_cache/mrc_cache.c
@@ -446,13 +446,21 @@
 /* Read flash status register to determine if write protect is active */
 static int nvm_is_write_protected(void)
 {
+	int result;
+
    if (!IS_ENABLED(CONFIG_CHROMEOS))
    	return 0;

if (!IS_ENABLED(CONFIG_BOOT_DEVICE_SPI_FLASH))
    	return 0;

-	return get_write_protect_state();
+	result = get_write_protect_state(boot_device_spi_flash());
+	if (result < 0)
+		printk(BIOS_ERR, "SPI flash protection failed!\n");
+	else
+		printk(BIOS_DEBUG, "SPI flash protection: WP=%d\n", result);
+
+	return result;
 }

/* Apply protection to a range of flash */
diff --git a/src/drivers/spi/spi_flash.c b/src/drivers/spi/spi_flash.c
index d20773f..0e269dd 100644
--- a/src/drivers/spi/spi_flash.c
+++ b/src/drivers/spi/spi_flash.c
@@ -419,7 +419,7 @@
    if (!region_is_subregion(&flash_region, region))
    	return -1;

-	if (flash->ops->get_write_protection) {
+	if (!flash->ops->get_write_protection) {
    	printk(BIOS_WARNING, "SPI: Write-protection gathering not "
    	       "implemented for this vendor.\n");
    	return 0;
@@ -442,7 +442,7 @@
    if (!region_is_subregion(&flash_region, region))
    	return -1;

-	if (flash->ops->set_write_protection) {
+	if (!flash->ops->set_write_protection) {
    	printk(BIOS_WARNING, "SPI: Setting write-protection is not "
    			     "implemented for this vendor.\n");
    	wp_result = -1;
@@ -450,7 +450,7 @@
    	wp_result = flash->ops->set_write_protection(flash, region);
    }

-	if (flash->ops->lock_status_register) {
+	if (!flash->ops->lock_status_register) {
    	printk(BIOS_WARNING, "SPI: Locking status register is not "
    			     "implemented for this vendor.\n");
    	st_result = 0;
diff --git a/src/security/flash/Makefile.inc b/src/security/flash/Makefile.inc
index 50d4a7a..f4bb413 100644
--- a/src/security/flash/Makefile.inc
+++ b/src/security/flash/Makefile.inc
@@ -1,5 +1,7 @@
 ## flash

-verstage-y += flash.c
-romstage-y += flash.c
 ramstage-y += flash.c
+
+ramstage-y += wp.c
+romstage-y += wp.c
+verstage-y += wp.c
diff --git a/src/security/flash/flash.c b/src/security/flash/flash.c
index 49a2980..608d6d2 100644
--- a/src/security/flash/flash.c
+++ b/src/security/flash/flash.c
@@ -13,44 +13,37 @@
  * GNU General Public License for more details.
  */

+#include <bootstate.h>
 #include <commonlib/region.h>
+#include <console/console.h>
 #include <fmap.h>
 #include <security/flash/flash.h>
-#include <spi_flash.h>
 #include <string.h>

#define FMAP_VBOOT_RO_REGION "WP_RO"
 #define FMAP_BIOS_REGION "BIOS"
 #define FMAP_FLASH_REGION "FLASH"

-__attribute__((weak)) int get_spi_wp_pin_state(void) { return 0; }
-
-int set_write_protect_enabled(void)
+int set_write_protect_enabled(const struct spi_flash *flash)
 {
    int result = -1;
    struct region region;

if (IS_ENABLED(CONFIG_FLASH_SPI_PROTECTIONS)) {
-		struct spi_flash flash;
-
-		spi_init();
-		if (spi_flash_probe(0, 0, &flash))
-			return result;
-
    	if (IS_ENABLED(CONFIG_FLASH_MODE_VBOOT)) {
    		if (fmap_locate_area(FMAP_VBOOT_RO_REGION, &region) ==
    		    0) {
-				result = spi_flash_set_write_protected(&flash,
+				result = spi_flash_set_write_protected(flash,
    							       &region);
    		}
    	} else if (IS_ENABLED(CONFIG_FLASH_MODE_BIOS)) {
    		if (fmap_locate_area(FMAP_BIOS_REGION, &region) == 0) {
-				result = spi_flash_set_write_protected(&flash,
+				result = spi_flash_set_write_protected(flash,
    							       &region);
    		}
    	} else if (IS_ENABLED(CONFIG_FLASH_MODE_EVERYTHING)) {
    		if (fmap_locate_area(FMAP_FLASH_REGION, &region) == 0) {
-				result = spi_flash_set_write_protected(&flash,
+				result = spi_flash_set_write_protected(flash,
    							       &region);
    		}
    	}
@@ -60,33 +53,27 @@
    return result;
 }

-int get_write_protect_state(void)
+int get_write_protect_state(const struct spi_flash *flash)
 {
    int result = -1;
    struct region region;

if (IS_ENABLED(CONFIG_FLASH_SPI_PROTECTIONS)) {
-		struct spi_flash flash;
-
-		spi_init();
-		if (spi_flash_probe(0, 0, &flash))
-			return result;
-
    	if (IS_ENABLED(CONFIG_FLASH_MODE_VBOOT)) {
    		if (fmap_locate_area(FMAP_VBOOT_RO_REGION, &region) ==
    		    0) {
-				result = spi_flash_is_write_protected(&flash,
+				result = spi_flash_is_write_protected(flash,
    							      &region);
-				result &= get_spi_wp_pin_state();
+				result = result && get_spi_wp_pin_state();
    		}
    	} else if (IS_ENABLED(CONFIG_FLASH_MODE_BIOS)) {
    		if (fmap_locate_area(FMAP_BIOS_REGION, &region) == 0) {
-				result = spi_flash_is_write_protected(&flash,
+				result = spi_flash_is_write_protected(flash,
    							      &region);
    		}
    	} else if (IS_ENABLED(CONFIG_FLASH_MODE_EVERYTHING)) {
    		if (fmap_locate_area(FMAP_FLASH_REGION, &region) == 0) {
-				result = spi_flash_is_write_protected(&flash,
+				result = spi_flash_is_write_protected(flash,
    							      &region);
    		}
    	}
@@ -95,3 +82,47 @@

return result;
 }
+
+static void init_flash_write_protection(void *unused)
+{
+	if (IS_ENABLED(CONFIG_BOOT_DEVICE_SPI_FLASH)) {
+		const struct spi_flash *flash = boot_device_spi_flash();
+
+		int result = get_write_protect_state(flash);
+		if (result == 0) {
+			printk(
+			    BIOS_INFO,
+			    "FLASH: Write protection disable, activating..\n");
+			set_write_protect_enabled(flash);
+		} else if (result < 0) {
+			printk(BIOS_ERR, "FLASH: Error getting WP state.\n");
+		} else {
+			printk(BIOS_INFO,
+			       "FLASH: Write protection already enabled.\n");
+		}
+	} else {
+		struct spi_flash flash;
+
+		spi_init();
+		if (spi_flash_probe(0, 0, &flash))
+			return;
+
+		int result = get_write_protect_state(flash);
+		if (result == 0) {
+			printk(
+			    BIOS_INFO,
+			    "FLASH: Write protection disable, activating..\n");
+			set_write_protect_enabled(&flash);
+		} else if (result < 0) {
+			printk(BIOS_ERR, "FLASH: Error getting WP state.\n")
+		} else {
+			printk(BIOS_INFO,
+			       "FLASH: Write protection already enabled.\n");
+		}
+	}
+}
+
+BOOT_STATE_INIT_ENTRY(BS_POST_DEVICE, BS_ON_ENTRY, init_flash_write_protection,
+		      NULL);
+BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, init_flash_write_protection,
+		      NULL);
diff --git a/src/security/flash/flash.h b/src/security/flash/flash.h
index 0d113c8..4c83495 100644
--- a/src/security/flash/flash.h
+++ b/src/security/flash/flash.h
@@ -16,9 +16,11 @@
 #ifndef FLASH_H_
 #define FLASH_H_

+#include <spi_flash.h>
+
 int get_spi_wp_pin_state(void);

-int set_write_protect_enabled(void);
-int get_write_protect_state(void);
+int set_write_protect_enabled(const struct spi_flash *flash);
+int get_write_protect_state(const struct spi_flash *flash);

#endif /* FLASH_H_ */
diff --git a/src/security/flash/wp.c b/src/security/flash/wp.c
new file mode 100644
index 0000000..87d84f7
--- /dev/null
+++ b/src/security/flash/wp.c
@@ -0,0 +1,18 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright 2018 Facebook Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <security/flash/flash.h>
+
+__attribute__((weak)) int get_spi_wp_pin_state(void) { return 0; }
diff --git a/src/security/vboot/vboot_handoff.c b/src/security/vboot/vboot_handoff.c
index 3190b7d..b0bf7f8 100644
--- a/src/security/vboot/vboot_handoff.c
+++ b/src/security/vboot/vboot_handoff.c
@@ -61,7 +61,7 @@
    vb_sd->data_used = sizeof(VbSharedDataHeader);
    vb_sd->fw_version_tpm = vb2_sd->fw_version_secdata;

-	if (get_write_protect_state())
+	if (get_spi_wp_pin_state())
    	vb_sd->flags |= VBSD_BOOT_FIRMWARE_WP_ENABLED;

if (vb2_sd->recovery_reason) {