From ef4d930f88ecf150bffa5761af72e7fa68b77afd Mon Sep 17 00:00:00 2001 From: Andrey Petrov Date: Wed, 10 Jun 2015 10:33:39 -0700 Subject: [PATCH] cyan: fix issues with write protection * Fixes cyan/board.h to use correct SPI part * Adds new flash protection regions in spi_flash_reg.c * Sets SRP register in flash_physical_protect_at_boot() * Fixes a bug in COMPARE_BIT macro * Makes spi_flash_set_status() fail only when both HW pin is asserted AND SRP(s) are set * Makes sure set_flash_set_status() completes before returning BUG=chrome-os-partner:40908 BRANCH=master TEST=on Cyan: With WP pin de-asserted: flashrom -p ec --wp-enable flashrom -p ec --wp-status, make sure it is enabled flashrom -p ec --wp-disable flashrom -p ec --status, make sure it is disabled flashrom -p ec --wp-enable Assert WP pin (either with screwdriver or dut-control) flashrom -p ec --wp-disable make sure it failed Change-Id: I338cc906b73e723fdbb37f7c2fd0c4da358b6c8e Signed-off-by: Andrey Petrov Reviewed-on: https://chromium-review.googlesource.com/276671 Reviewed-by: Shawn N Tested-by: Divya Jyothi Commit-Queue: Divya Jyothi --- board/cyan/board.h | 2 +- chip/mec1322/flash.c | 82 ++++++++++-------------------------------- common/spi_flash.c | 10 ++++-- common/spi_flash_reg.c | 5 ++- include/config.h | 3 -- 5 files changed, 32 insertions(+), 70 deletions(-) diff --git a/board/cyan/board.h b/board/cyan/board.h index b0d575aad6..40ed8f95f7 100644 --- a/board/cyan/board.h +++ b/board/cyan/board.h @@ -48,7 +48,7 @@ #define CONFIG_SPI_FLASH #define CONFIG_SPI_FLASH_SIZE 524288 /* TODO: Add flash protect support for the SPI part cyan actually has */ -#define CONFIG_SPI_FLASH_W25Q64 +#define CONFIG_SPI_FLASH_W25X40 #define CONFIG_USB_PORT_POWER_SMART #define CONFIG_USB_PORT_POWER_SMART_SIMPLE diff --git a/chip/mec1322/flash.c b/chip/mec1322/flash.c index a893ba7868..f263d42755 100644 --- a/chip/mec1322/flash.c +++ b/chip/mec1322/flash.c @@ -97,22 +97,9 @@ int flash_physical_erase(int offset, int size) */ int flash_physical_get_protect(int bank) { -#ifdef CONFIG_WP_ENABLE -/* - * TODO(crosbug/p/40908): This section was causing SPI to lock up - * while installing via firmware updater. Disabled it temporarily. - * will get re-enabled after Flash protection is tested under all - * scenarios - */ - - uint32_t addr = bank * CONFIG_FLASH_BANK_SIZE; - int ret; - - ret = spi_flash_check_protect(addr, CONFIG_FLASH_BANK_SIZE); - return ret; -#else - return 0; -#endif + return spi_flash_check_protect(CONFIG_FLASH_BASE_SPI + + bank * CONFIG_FLASH_BANK_SIZE, + CONFIG_FLASH_BANK_SIZE); } /** @@ -123,29 +110,18 @@ int flash_physical_get_protect(int bank) */ int flash_physical_protect_now(int all) { -#ifdef CONFIG_WP_ENABLE -/* - * TODO(crosbug/p/40908): This section was causing SPI to lock up - * while installing via firmware updater. Disabled it temporarily. - * will get re-enabled after Flash protection is tested under all - * scenarios - */ - int offset, size, ret; if (all) { - offset = 0; + offset = CONFIG_FLASH_BASE_SPI; size = CONFIG_FLASH_PHYSICAL_SIZE; } else { - offset = CONFIG_WP_OFF; + offset = CONFIG_WP_OFF + CONFIG_FLASH_BASE_SPI; size = CONFIG_WP_SIZE; } ret = spi_flash_set_protect(offset, size); return ret; -#else - return 0; -#endif } /** @@ -159,21 +135,14 @@ uint32_t flash_physical_get_protect_flags(void) { uint32_t flags = 0; -#ifdef CONFIG_WP_ENABLE -/* - * TODO(crosbug/p/40908): This section was causing SPI to lock up - * while installing via firmware updater. Disabled it temporarily. - * will get re-enabled after Flash protection is tested under all - * scenarios - */ - - if (spi_flash_check_protect(CONFIG_RO_STORAGE_OFF, CONFIG_RO_SIZE)) { + if (spi_flash_check_protect(CONFIG_FLASH_BASE_SPI + + CONFIG_RO_STORAGE_OFF, CONFIG_RO_SIZE)) { flags |= EC_FLASH_PROTECT_RO_AT_BOOT | EC_FLASH_PROTECT_RO_NOW; - if (spi_flash_check_protect(CONFIG_RW_STORAGE_OFF, + if (spi_flash_check_protect(CONFIG_FLASH_BASE_SPI + + CONFIG_RW_STORAGE_OFF, CONFIG_RW_SIZE)) flags |= EC_FLASH_PROTECT_ALL_NOW; } -#endif return flags; } @@ -198,22 +167,14 @@ uint32_t flash_physical_get_valid_flags(void) uint32_t flash_physical_get_writable_flags(uint32_t cur_flags) { uint32_t ret = 0; + enum spi_flash_wp wp_status = SPI_WP_NONE; -#ifdef CONFIG_WP_ENABLE -/* - * TODO(crosbug/p/40908): This section was causing SPI to lock up - * while installing via firmware updater. Disabled it temporarily. - * will get re-enabled after Flash protection is tested under all - * scenarios - */ - - enum spi_flash_wp wp_status = spi_flash_check_wp(); + wp_status = spi_flash_check_wp(); if (wp_status == SPI_WP_NONE || (wp_status == SPI_WP_HARDWARE && !(cur_flags & EC_FLASH_PROTECT_GPIO_ASSERTED))) ret = EC_FLASH_PROTECT_RO_AT_BOOT | EC_FLASH_PROTECT_RO_NOW | EC_FLASH_PROTECT_ALL_NOW; -#endif return ret; } @@ -230,35 +191,30 @@ uint32_t flash_physical_get_writable_flags(uint32_t cur_flags) */ int flash_physical_protect_at_boot(enum flash_wp_range range) { -#ifdef CONFIG_WP_ENABLE -/* - * TODO(crosbug/p/40908): This section was causing SPI to lock up - * while installing via firmware updater. Disabled it temporarily. - * will get re-enabled after Flash protection is tested under all - * scenarios - */ - int offset, size, ret; + enum spi_flash_wp flashwp = SPI_WP_NONE; switch (range) { case FLASH_WP_NONE: offset = size = 0; + flashwp = SPI_WP_NONE; break; case FLASH_WP_RO: - offset = CONFIG_WP_OFF; + offset = CONFIG_FLASH_BASE_SPI + CONFIG_WP_OFF; size = CONFIG_WP_SIZE; + flashwp = SPI_WP_HARDWARE; break; case FLASH_WP_ALL: - offset = 0; + offset = CONFIG_FLASH_BASE_SPI; size = CONFIG_FLASH_PHYSICAL_SIZE; + flashwp = SPI_WP_HARDWARE; break; } ret = spi_flash_set_protect(offset, size); + if (ret == EC_SUCCESS) + ret = spi_flash_set_wp(flashwp); return ret; -#else - return 0; -#endif } /** diff --git a/common/spi_flash.c b/common/spi_flash.c index 126e38d7f7..417f97dfd1 100644 --- a/common/spi_flash.c +++ b/common/spi_flash.c @@ -15,6 +15,8 @@ #include "timer.h" #include "util.h" #include "watchdog.h" +#include "ec_commands.h" +#include "flash.h" /* * Time to sleep when chip is busy @@ -109,8 +111,9 @@ int spi_flash_set_status(int reg1, int reg2) uint8_t cmd[3] = {SPI_FLASH_WRITE_SR, reg1, reg2}; int rv = EC_SUCCESS; - /* Register has protection */ - if (spi_flash_check_wp() != SPI_WP_NONE) + /* fail if both HW pin is asserted and SRP(s) is 1 */ + if (spi_flash_check_wp() != SPI_WP_NONE && + (flash_get_protect() & EC_FLASH_PROTECT_GPIO_ASSERTED) != 0) return EC_ERROR_ACCESS_DENIED; /* Enable writing to SPI flash */ @@ -130,6 +133,9 @@ int spi_flash_set_status(int reg1, int reg2) if (rv) return rv; + /* SRP update takes up to 10 ms, so wait for transaction to finish */ + spi_flash_wait(); + return rv; } diff --git a/common/spi_flash_reg.c b/common/spi_flash_reg.c index f1261ad823..6eab99f1cc 100644 --- a/common/spi_flash_reg.c +++ b/common/spi_flash_reg.c @@ -27,7 +27,7 @@ struct protect_range { }; /* Compare macro for (x =? b) for 'X' comparison */ -#define COMPARE_BIT(a, b) ((a) != X && (a) != (b)) +#define COMPARE_BIT(a, b) ((a) != X && (a) != !!(b)) /* Assignment macro where 'X' = 0 */ #define GET_BIT(a) ((a) == X ? 0 : (a)) @@ -44,6 +44,9 @@ static const struct protect_range spi_flash_protect_ranges[] = { { X, X, 1, { 0, 0, 1 }, 0, 0x10000 }, /* Lower 1/8 */ { X, X, 1, { 0, 1, 0 }, 0, 0x20000 }, /* Lower 1/4 */ { X, X, X, { 1, X, X }, 0, 0x80000 }, /* All protected */ + { X, X, 0, { 0, 0, 1 }, 0x70000, 0x10000 }, /* Upper 1/8*/ + { X, X, 0, { 0, 1, 0 }, 0x60000, 0x20000 }, /* Upper 1/4*/ + { X, X, 0, { 0, 1, 1 }, 0x40000, 0x40000 }, /* Upper 1/2*/ }; #elif defined(CONFIG_SPI_FLASH_W25Q64) diff --git a/include/config.h b/include/config.h index 2606fd2787..999083d776 100644 --- a/include/config.h +++ b/include/config.h @@ -723,9 +723,6 @@ #undef CONFIG_RW_STORAGE_OFF #undef CONFIG_RW_SIZE -/* Enable SPI Flash write protect. */ -#undef CONFIG_WP_ENABLE - /* * Write protect region offset / size. This region normally encompasses the * RO image, but may also contain additional images or data.