From d95b9fc18c9633fe4b22ead2be450438bbc2dc29 Mon Sep 17 00:00:00 2001 From: Shawn Nematbakhsh Date: Mon, 21 Nov 2016 17:04:02 -0800 Subject: [PATCH] npcx: flash: Fix SR reg reads during UMA lock If UMA is locked, read and write access to SR* regs will fail, and we'll be unable to determine or change our write protect state. Save our SR regs before enabling UMA lock, and bring down UMA lock for a brief period when writing SR regs. BUG=chrome-os-partner:60029 BRANCH=gru TEST=Manual on kevin with SW sync enabled. flashrom -p ec --wp-enable -> verify success, check flags: Flags: wp_gpio_asserted ro_at_boot ro_now Reboot host, check flags: Flags: wp_gpio_asserted ro_at_boot ro_now all_now Verify flashrom -p ec --wp-status shows protected, and flashrom -p ec --wp-disable fails. Also verify same state after EC reset. Boot into recovery mode, check flags: Flags: wp_gpio_asserted ro_at_boot ro_now Remove WP screw, reboot EC, check flags: Flags: ro_at_boot ro_now Verify flashrom WP status shows protected, and flashrom WP disable succeeds and clears all flags. Signed-off-by: Shawn Nematbakhsh Change-Id: I17e32f7305600183e7fcb87f69a3feb88978f94e Reviewed-on: https://chromium-review.googlesource.com/412977 (cherry picked from commit 6cfc0d0dd030b8b0334f99e99950a52fcf8267af) Reviewed-on: https://chromium-review.googlesource.com/415497 Commit-Ready: Shawn N Tested-by: Shawn N Reviewed-by: Shawn N --- chip/npcx/flash.c | 86 +++++++++++++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 33 deletions(-) diff --git a/chip/npcx/flash.c b/chip/npcx/flash.c index 31e4c60f1a..493899e3c0 100644 --- a/chip/npcx/flash.c +++ b/chip/npcx/flash.c @@ -17,10 +17,14 @@ #include "console.h" #include "hwtimer_chip.h" -int all_protected; /* Has all-flash protection been requested? */ -int addr_prot_start; -int addr_prot_length; -uint8_t flag_prot_inconsistent; +static int all_protected; /* Has all-flash protection been requested? */ +static int addr_prot_start; +static int addr_prot_length; +static uint8_t flag_prot_inconsistent; + +/* SR regs aren't readable when UMA lock is on, so save a copy */ +static uint8_t saved_sr1; +static uint8_t saved_sr2; #define FLASH_ABORT_TIMEOUT 10000 @@ -139,8 +143,11 @@ static void flash_set_address(uint32_t dest_addr) NPCX_UMA_AB0 = addr[0]; } -uint8_t flash_get_status1(void) +static uint8_t flash_get_status1(void) { + if (all_protected) + return saved_sr1; + /* Disable tri-state */ TRISTATE_FLASH(0); /* Read status register1 */ @@ -150,8 +157,11 @@ uint8_t flash_get_status1(void) return NPCX_UMA_DB0; } -uint8_t flash_get_status2(void) +static uint8_t flash_get_status2(void) { + if (all_protected) + return saved_sr2; + /* Disable tri-state */ TRISTATE_FLASH(0); /* Read status register2 */ @@ -347,8 +357,33 @@ static int protect_to_reg(unsigned int start, unsigned int len, return EC_SUCCESS; } +static void flash_uma_lock(int enable) +{ + if (enable && !all_protected) { + /* + * Store SR1 / SR2 for later use since we're about to lock + * out all access (including read access) to these regs. + */ + saved_sr1 = flash_get_status1(); + saved_sr2 = flash_get_status2(); + } + + all_protected = enable; + UPDATE_BIT(NPCX_UMA_ECTS, NPCX_UMA_ECTS_UMA_LOCK, enable); +} + static int flash_set_status_for_prot(int reg1, int reg2) { + /* + * Writing SR regs will fail if our UMA lock is enabled. If WP + * is deasserted then remove the lock and allow the write. + */ + if (all_protected) { + if (flash_get_protect() & EC_FLASH_PROTECT_GPIO_ASSERTED) + return EC_ERROR_ACCESS_DENIED; + flash_uma_lock(0); + } + /* Lock physical flash operations */ flash_lock_mapped_storage(1); @@ -390,7 +425,7 @@ static int flash_check_prot_reg(unsigned int offset, unsigned int bytes) { unsigned int start; unsigned int len; - uint8_t sr1 = 0, sr2 = 0; + uint8_t sr1, sr2; int rv = EC_SUCCESS; sr1 = flash_get_status1(); @@ -481,12 +516,6 @@ static int flash_program_bytes(uint32_t offset, uint32_t bytes, return rv; } -static int flash_uma_lock(int enable) -{ - UPDATE_BIT(NPCX_UMA_ECTS, NPCX_UMA_ECTS_UMA_LOCK, enable); - return EC_SUCCESS; -} - static int flash_spi_sel_lock(int enable) { /* @@ -646,9 +675,6 @@ int flash_physical_erase(int offset, int size) int flash_physical_get_protect(int bank) { uint32_t addr = bank * CONFIG_FLASH_BANK_SIZE; - /* All UMA transaction is locked means all banks are protected */ - if (IS_BIT_SET(NPCX_UMA_ECTS, NPCX_UMA_ECTS_UMA_LOCK)) - return EC_ERROR_ACCESS_DENIED; return flash_check_prot_reg(addr, CONFIG_FLASH_BANK_SIZE); } @@ -679,18 +705,14 @@ uint32_t flash_physical_get_protect_flags(void) int flash_physical_protect_now(int all) { if (all) { - all_protected = 1; /* * Set UMA_LOCK bit for locking all UMA transaction. * But we still can read directly from flash mapping address */ flash_uma_lock(1); } else { - all_protected = 0; - /* Unlocking all UMA transaction */ - flash_uma_lock(0); + /* TODO: Implement RO "now" protection */ } - /* TODO: if all, disable SPI interface */ return EC_SUCCESS; } @@ -700,26 +722,22 @@ int flash_physical_protect_at_boot(enum flash_wp_range range) { switch (range) { case FLASH_WP_NONE: - /* Unlock UMA transactions */ - if (IS_BIT_SET(NPCX_UMA_ECTS, NPCX_UMA_ECTS_UMA_LOCK)) - CLEAR_BIT(NPCX_UMA_ECTS, NPCX_UMA_ECTS_UMA_LOCK); /* Clear protection bits in status register */ return flash_set_status_for_prot(0, 0); case FLASH_WP_RO: - /* Unlock UMA transactions */ - if (IS_BIT_SET(NPCX_UMA_ECTS, NPCX_UMA_ECTS_UMA_LOCK)) - CLEAR_BIT(NPCX_UMA_ECTS, NPCX_UMA_ECTS_UMA_LOCK); /* Protect read-only */ - return flash_write_prot_reg( - WP_BANK_OFFSET*CONFIG_FLASH_BANK_SIZE, - WP_BANK_COUNT*CONFIG_FLASH_BANK_SIZE); + return flash_write_prot_reg(CONFIG_WP_STORAGE_OFF, + CONFIG_WP_STORAGE_SIZE); case FLASH_WP_ALL: - /* Protect all */ + flash_write_prot_reg(CONFIG_WP_STORAGE_OFF, + CONFIG_WP_STORAGE_SIZE); + /* * Set UMA_LOCK bit for locking all UMA transaction. * But we still can read directly from flash mapping address */ - return flash_uma_lock(1); + flash_uma_lock(1); + return EC_SUCCESS; default: return EC_ERROR_INVAL; } @@ -764,7 +782,9 @@ int flash_pre_init(void) CLEAR_BIT(NPCX_DEVCNT, NPCX_DEVCNT_F_SPI_TRIS); #endif - return EC_SUCCESS; + /* Initialize UMA to unlocked */ + flash_uma_lock(0); + return EC_SUCCESS; } void flash_lock_mapped_storage(int lock)