Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/58474 )
Change subject: writeprotect, cli_classic: delete old writeprotect code ......................................................................
writeprotect, cli_classic: delete old writeprotect code
Delete writeprotect code that was previously extracted from the cros tree.
This is the first of a series of commits adding writeprotect support. Following commits incrementally implement writeprotect operations, culminating in writeprotect support for three example chips: GD25LQ128, GD25Q32, and GD25Q256.
BUG=b:195381327,b:153800563 BRANCH=none TEST=flashrom -{r,w,E} TEST=flashrom --wp-{enable,disable,range,list,status} at end of patch series
Change-Id: I67e9b31f86465e5a8f7d3def637198671ee818a8 Signed-off-by: Nikolai Artemiev nartemiev@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/58474 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org Reviewed-by: Anastasia Klimchuk aklm@chromium.org --- M cli_classic.c M flash.h M s25f.c M writeprotect.c M writeprotect.h 5 files changed, 7 insertions(+), 571 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved
diff --git a/cli_classic.c b/cli_classic.c index 317172c..dc3a651 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -29,7 +29,6 @@ #include "flashchips.h" #include "fmap.h" #include "programmer.h" -#include "writeprotect.h" #include "libflashrom.h"
static void cli_classic_usage(const char *name) @@ -60,12 +59,6 @@ " -N | --noverify-all verify included regions only (cf. -i)\n" " -x | --extract extract regions to files\n" " -l | --layout <layoutfile> read ROM layout from <layoutfile>\n" - " --wp-disable disable write protection\n" - " --wp-enable enable write protection\n" - " --wp-list list write protect range\n" - " --wp-status show write protect status\n" - " --wp-range=<start>,<len> set write protect range\n" - " --wp-region <region> set write protect region\n" " --flash-name read out the detected flash name\n" " --flash-size read out the detected flash size\n" " --fmap read ROM layout from fmap embedded in ROM\n" @@ -128,32 +121,6 @@ return true; }
-static int parse_wp_range(unsigned int *start, unsigned int *len) -{ - char *endptr = NULL, *token = NULL; - - if (!optarg) { - msg_gerr("Error: No wp-range values provided\n"); - return -1; - } - - token = strtok(optarg, ","); - if (!token) { - msg_gerr("Error: Invalid wp-range argument format\n"); - return -1; - } - *start = strtoul(token, &endptr, 0); - - token = strtok(NULL, ","); - if (!token) { - msg_gerr("Error: Invalid wp-range argument format\n"); - return -1; - } - *len = strtoul(token, &endptr, 0); - - return 0; -} - static int do_read(struct flashctx *const flash, const char *const filename) { int ret; @@ -264,8 +231,6 @@ int list_supported_wiki = 0; #endif int flash_name = 0, flash_size = 0; - int set_wp_enable = 0, set_wp_disable = 0, wp_status = 0; - int set_wp_range = 0, set_wp_region = 0, wp_list = 0; int read_it = 0, extract_it = 0, write_it = 0, erase_it = 0, verify_it = 0; int dont_verify_it = 0, dont_verify_all = 0, list_supported = 0, operation_specified = 0; struct flashrom_layout *layout = NULL; @@ -277,15 +242,8 @@ OPTION_FLASH_CONTENTS, OPTION_FLASH_NAME, OPTION_FLASH_SIZE, - OPTION_WP_STATUS, - OPTION_WP_SET_RANGE, - OPTION_WP_SET_REGION, - OPTION_WP_ENABLE, - OPTION_WP_DISABLE, - OPTION_WP_LIST, }; int ret = 0; - unsigned int wp_start = 0, wp_len = 0;
static const char optstring[] = "r:Rw:v:nNVEfc:l:i:p:Lzho:x"; static const struct option long_options[] = { @@ -308,12 +266,6 @@ {"flash-name", 0, NULL, OPTION_FLASH_NAME}, {"flash-size", 0, NULL, OPTION_FLASH_SIZE}, {"get-size", 0, NULL, OPTION_FLASH_SIZE}, // (deprecated): back compatibility. - {"wp-status", 0, 0, OPTION_WP_STATUS}, - {"wp-range", required_argument, NULL, OPTION_WP_SET_RANGE}, - {"wp-region", 1, 0, OPTION_WP_SET_REGION}, - {"wp-enable", optional_argument, 0, OPTION_WP_ENABLE}, - {"wp-disable", 0, 0, OPTION_WP_DISABLE}, - {"wp-list", 0, 0, OPTION_WP_LIST}, {"list-supported", 0, NULL, 'L'}, {"list-supported-wiki", 0, NULL, 'z'}, {"programmer", 1, NULL, 'p'}, @@ -333,8 +285,6 @@ char *tempstr = NULL; char *pparam = NULL; struct layout_include_args *include_args = NULL; - char *wp_mode_opt = NULL; - char *wp_region = NULL;
/* * Safety-guard against a user who has (mistakenly) closed @@ -463,26 +413,6 @@ cli_classic_validate_singleop(&operation_specified); flash_size = 1; break; - case OPTION_WP_STATUS: - wp_status = 1; - break; - case OPTION_WP_LIST: - wp_list = 1; - break; - case OPTION_WP_SET_RANGE: - if (parse_wp_range(&wp_start, &wp_len) < 0) - cli_classic_abort_usage("Incorrect wp-range arguments provided.\n"); - - set_wp_range = 1; - break; - case OPTION_WP_ENABLE: - set_wp_enable = 1; - if (optarg) - wp_mode_opt = strdup(optarg); - break; - case OPTION_WP_DISABLE: - set_wp_disable = 1; - break; case 'L': cli_classic_validate_singleop(&operation_specified); list_supported = 1; @@ -565,10 +495,6 @@ } #endif /* STANDALONE */ break; - case OPTION_WP_SET_REGION: - set_wp_region = 1; - wp_region = strdup(optarg); - break; default: cli_classic_abort_usage(NULL); break; @@ -768,33 +694,15 @@ goto out_shutdown; }
- if (!(read_it | write_it | verify_it | erase_it | flash_name | flash_size - | set_wp_range | set_wp_region | set_wp_enable | - set_wp_disable | wp_status | wp_list | extract_it)) { + const bool any_op = read_it || write_it || verify_it || erase_it || + flash_name || flash_size || extract_it; + + + if (!any_op) { msg_ginfo("No operations were specified.\n"); goto out_shutdown; }
- if (set_wp_enable && set_wp_disable) { - msg_ginfo("Error: --wp-enable and --wp-disable are mutually exclusive\n"); - ret = 1; - goto out_shutdown; - } - if (set_wp_range && set_wp_region) { - msg_gerr("Error: Cannot use both --wp-range and --wp-region simultaneously.\n"); - ret = 1; - goto out_shutdown; - } - - struct wp *wp = fill_flash->chip->wp; - if (set_wp_range || set_wp_region) { - if (!wp || !wp->set_range) { - msg_gerr("Error: write protect is not supported on this flash chip.\n"); - ret = 1; - goto out_shutdown; - } - } - if (flash_name) { if (fill_flash->chip->vendor && fill_flash->chip->name) { printf("vendor="%s" name="%s"\n", @@ -849,76 +757,6 @@ goto out_shutdown; } flashrom_layout_set(fill_flash, layout); - - if (wp_status) { - if (wp && wp->wp_status) { - ret |= wp->wp_status(fill_flash); - } else { - msg_gerr("Error: write protect is not supported on this flash chip.\n"); - ret = 1; - } - goto out_release; - } - - /* Note: set_wp_disable should be done before setting the range */ - if (set_wp_disable) { - if (wp && wp->disable) { - ret |= wp->disable(fill_flash); - } else { - msg_gerr("Error: write protect is not supported on this flash chip.\n"); - ret = 1; - goto out_release; - } - } - - /* Note: set_wp_range must happen before set_wp_enable */ - if (set_wp_range) { - ret |= wp->set_range(fill_flash, wp_start, wp_len); - } - - if (set_wp_region && wp_region) { - if (flashrom_layout_get_region_range(layout, wp_region, &wp_start, &wp_len)) { - ret = 1; - goto out_release; - } - ret |= wp->set_range(fill_flash, wp_start, wp_len); - free(wp_region); - } - - if (!ret && set_wp_enable) { - enum wp_mode wp_mode; - - if (wp_mode_opt) - wp_mode = get_wp_mode(wp_mode_opt); - else - wp_mode = WP_MODE_HARDWARE; /* default */ - - if (wp_mode == WP_MODE_UNKNOWN) { - msg_gerr("Error: Invalid WP mode: "%s"\n", wp_mode_opt); - ret = 1; - goto out_release; - } - - if (wp && wp->enable) { - ret |= wp->enable(fill_flash, wp_mode); - } else { - msg_gerr("Error: write protect is not supported on this flash chip.\n"); - ret = 1; - goto out_release; - } - } - - if (wp_list) { - msg_ginfo("Valid write protection ranges:\n"); - if (wp && wp->list_ranges) { - ret |= wp->list_ranges(fill_flash); - } else { - msg_gerr("Error: write protect is not supported on this flash chip.\n"); - ret = 1; - } - goto out_release; - } - flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE, !!force); #if CONFIG_INTERNAL == 1 flashrom_flag_set(fill_flash, FLASHROM_FLAG_FORCE_BOARDMISMATCH, !!force_boardmismatch); @@ -952,7 +790,6 @@ else if (verify_it) ret = do_verify(fill_flash, filename);
-out_release: flashrom_layout_release(layout); out_shutdown: flashrom_programmer_shutdown(NULL); diff --git a/flash.h b/flash.h index 09c1c85..654cdee 100644 --- a/flash.h +++ b/flash.h @@ -245,7 +245,6 @@ /* SPI specific options (TODO: Make it a union in case other bustypes get specific options.) */ uint8_t wrea_override; /**< override opcode for write extended address register */
- struct wp *wp; };
typedef int (*chip_restore_fn_cb_t)(struct flashctx *flash, uint8_t status); diff --git a/s25f.c b/s25f.c index 2fba6bc..0bd4a7c 100644 --- a/s25f.c +++ b/s25f.c @@ -25,7 +25,6 @@
#include "chipdrivers.h" #include "spi.h" -#include "writeprotect.h"
/* * RDAR and WRAR are supported on chips which have more than one set of status diff --git a/writeprotect.c b/writeprotect.c index b26c121..cfcb13c 100644 --- a/writeprotect.c +++ b/writeprotect.c @@ -15,380 +15,11 @@ * */
+#include <stdio.h> #include <stdlib.h> #include <string.h> -#include <strings.h>
#include "flash.h" -#include "flashchips.h" +#include "libflashrom.h" #include "chipdrivers.h" -#include "spi.h" #include "writeprotect.h" - -/* - * The following procedures rely on look-up tables to match the user-specified - * range with the chip's supported ranges. This turned out to be the most - * elegant approach since diferent flash chips use different levels of - * granularity and methods to determine protected ranges. In other words, - * be stupid and simple since clever arithmetic will not work for many chips. - */ - -struct wp_range { - unsigned int start; /* starting address */ - unsigned int len; /* len */ -}; - -enum bit_state { - OFF = 0, - ON = 1, - X = -1 /* don't care. Must be bigger than max # of bp. */ -}; - -/* - * Generic write-protection schema for 25-series SPI flash chips. This assumes - * there is a status register that contains one or more consecutive bits which - * determine which address range is protected. - */ - -struct status_register_layout { - int bp0_pos; /* position of BP0 */ - int bp_bits; /* number of block protect bits */ - int srp_pos; /* position of status register protect enable bit */ -}; - -/* - * The following ranges and functions are useful for representing the - * writeprotect schema in which there are typically 5 bits of - * relevant information stored in status register 1: - * m.sec: This bit indicates the units (sectors vs. blocks) - * m.tb: The top-bottom bit indicates if the affected range is at the top of - * the flash memory's address space or at the bottom. - * bp: Bitmask representing the number of affected sectors/blocks. - */ -struct wp_range_descriptor { - struct modifier_bits m; - unsigned int bp; /* block protect bitfield */ - struct wp_range range; -}; - -struct wp_context { - struct status_register_layout sr1; /* status register 1 */ - struct wp_range_descriptor *descrs; - - /* - * Some chips store modifier bits in one or more special control - * registers instead of the status register like many older SPI NOR - * flash chips did. get_modifier_bits() and set_modifier_bits() will do - * any chip-specific operations necessary to get/set these bit values. - */ - int (*get_modifier_bits)(const struct flashctx *flash, - struct modifier_bits *m); - int (*set_modifier_bits)(const struct flashctx *flash, - struct modifier_bits *m); -}; - -/* - * Mask to extract write-protect enable and range bits - * Status register 1: - * SRP0: bit 7 - * range(BP2-BP0): bit 4-2 - * range(BP3-BP0): bit 5-2 (large chips) - * Status register 2: - * SRP1: bit 1 - */ -#define MASK_WP_AREA (0x9C) -#define MASK_WP_AREA_LARGE (0x9C) -#define MASK_WP2_AREA (0x01) - -static uint8_t do_read_status(const struct flashctx *flash) -{ - if (flash->chip->read_status) - return flash->chip->read_status(flash); - else - return spi_read_status_register(flash); -} - -static int do_write_status(const struct flashctx *flash, int status) -{ - if (flash->chip->write_status) - return flash->chip->write_status(flash, status); - else - return spi_write_status_register(flash, status); -} - -enum wp_mode get_wp_mode(const char *mode_str) -{ - enum wp_mode wp_mode = WP_MODE_UNKNOWN; - - if (!strcasecmp(mode_str, "hardware")) - wp_mode = WP_MODE_HARDWARE; - else if (!strcasecmp(mode_str, "power_cycle")) - wp_mode = WP_MODE_POWER_CYCLE; - else if (!strcasecmp(mode_str, "permanent")) - wp_mode = WP_MODE_PERMANENT; - - return wp_mode; -} - -/* Given a flash chip, this function returns its writeprotect info. */ -static int generic_range_table(const struct flashctx *flash, - struct wp_context **wp, - int *num_entries) -{ - *wp = NULL; - *num_entries = 0; - - switch (flash->chip->manufacture_id) { - default: - msg_cerr("%s: flash vendor (0x%x) not found, aborting\n", - __func__, flash->chip->manufacture_id); - return -1; - } - - return 0; -} - -static uint8_t generic_get_bp_mask(struct wp_context *wp) -{ - return ((1 << (wp->sr1.bp0_pos + wp->sr1.bp_bits)) - 1) ^ \ - ((1 << wp->sr1.bp0_pos) - 1); -} - -static uint8_t generic_get_status_check_mask(struct wp_context *wp) -{ - return generic_get_bp_mask(wp) | 1 << wp->sr1.srp_pos; -} - -/* Given a [start, len], this function finds a block protect bit combination - * (if possible) and sets the corresponding bits in "status". Remaining bits - * are preserved. */ -static int generic_range_to_status(const struct flashctx *flash, - unsigned int start, unsigned int len, - uint8_t *status, uint8_t *check_mask) -{ - struct wp_context *wp; - struct wp_range_descriptor *r; - int i, range_found = 0, num_entries; - uint8_t bp_mask; - - if (generic_range_table(flash, &wp, &num_entries)) - return -1; - - bp_mask = generic_get_bp_mask(wp); - - for (i = 0, r = &wp->descrs[0]; i < num_entries; i++, r++) { - msg_cspew("comparing range 0x%x 0x%x / 0x%x 0x%x\n", - start, len, r->range.start, r->range.len); - if ((start == r->range.start) && (len == r->range.len)) { - *status &= ~(bp_mask); - *status |= r->bp << (wp->sr1.bp0_pos); - - if (wp->set_modifier_bits) { - if (wp->set_modifier_bits(flash, &r->m) < 0) { - msg_cerr("error setting modifier bits for range.\n"); - return -1; - } - } - - range_found = 1; - break; - } - } - - if (!range_found) { - msg_cerr("%s: matching range not found\n", __func__); - return -1; - } - - *check_mask = generic_get_status_check_mask(wp); - return 0; -} - -static int generic_status_to_range(const struct flashctx *flash, - const uint8_t sr1, unsigned int *start, unsigned int *len) -{ - struct wp_context *wp; - struct wp_range_descriptor *r; - int num_entries, i, status_found = 0; - uint8_t sr1_bp; - struct modifier_bits m; - - if (generic_range_table(flash, &wp, &num_entries)) - return -1; - - /* modifier bits may be compared more than once, so get them here */ - if (wp->get_modifier_bits && wp->get_modifier_bits(flash, &m) < 0) - return -1; - - sr1_bp = (sr1 >> wp->sr1.bp0_pos) & ((1 << wp->sr1.bp_bits) - 1); - - for (i = 0, r = &wp->descrs[0]; i < num_entries; i++, r++) { - if (wp->get_modifier_bits) { - if (memcmp(&m, &r->m, sizeof(m))) - continue; - } - msg_cspew("comparing 0x%02x 0x%02x\n", sr1_bp, r->bp); - if (sr1_bp == r->bp) { - *start = r->range.start; - *len = r->range.len; - status_found = 1; - break; - } - } - - if (!status_found) { - msg_cerr("matching status not found\n"); - return -1; - } - - return 0; -} - -/* Given a [start, len], this function calls generic_range_to_status() to - * convert it to flash-chip-specific range bits, then sets into status register. - */ -static int generic_set_range(const struct flashctx *flash, - unsigned int start, unsigned int len) -{ - uint8_t status, expected, check_mask; - - status = do_read_status(flash); - msg_cdbg("%s: old status: 0x%02x\n", __func__, status); - - expected = status; /* preserve non-bp bits */ - if (generic_range_to_status(flash, start, len, &expected, &check_mask)) - return -1; - - do_write_status(flash, expected); - - status = do_read_status(flash); - msg_cdbg("%s: new status: 0x%02x\n", __func__, status); - if ((status & check_mask) != (expected & check_mask)) { - msg_cerr("expected=0x%02x, but actual=0x%02x. check mask=0x%02x\n", - expected, status, check_mask); - return 1; - } - return 0; -} - -/* Set/clear the status regsiter write protect bit in SR1. */ -static int generic_set_srp0(const struct flashctx *flash, int enable) -{ - uint8_t status, expected, check_mask; - struct wp_context *wp; - int num_entries; - - if (generic_range_table(flash, &wp, &num_entries)) - return -1; - - expected = do_read_status(flash); - msg_cdbg("%s: old status: 0x%02x\n", __func__, expected); - - if (enable) - expected |= 1 << wp->sr1.srp_pos; - else - expected &= ~(1 << wp->sr1.srp_pos); - - do_write_status(flash, expected); - - status = do_read_status(flash); - msg_cdbg("%s: new status: 0x%02x\n", __func__, status); - - check_mask = generic_get_status_check_mask(wp); - msg_cdbg("%s: check mask: 0x%02x\n", __func__, check_mask); - if ((status & check_mask) != (expected & check_mask)) { - msg_cerr("expected=0x%02x, but actual=0x%02x. check mask=0x%02x\n", - expected, status, check_mask); - return -1; - } - - return 0; -} - -static int generic_enable_writeprotect(const struct flashctx *flash, - enum wp_mode wp_mode) -{ - int ret; - - if (wp_mode != WP_MODE_HARDWARE) { - msg_cerr("%s(): unsupported write-protect mode\n", __func__); - return 1; - } - - ret = generic_set_srp0(flash, 1); - if (ret) - msg_cerr("%s(): error=%d.\n", __func__, ret); - - return ret; -} - -static int generic_disable_writeprotect(const struct flashctx *flash) -{ - int ret; - - ret = generic_set_srp0(flash, 0); - if (ret) - msg_cerr("%s(): error=%d.\n", __func__, ret); - - return ret; -} - -static int generic_list_ranges(const struct flashctx *flash) -{ - struct wp_context *wp; - struct wp_range_descriptor *r; - int i, num_entries; - - if (generic_range_table(flash, &wp, &num_entries)) - return -1; - - r = &wp->descrs[0]; - for (i = 0; i < num_entries; i++) { - msg_cinfo("start: 0x%06x, length: 0x%06x\n", - r->range.start, r->range.len); - r++; - } - - return 0; -} - -static int wp_context_status(const struct flashctx *flash) -{ - uint8_t sr1; - unsigned int start, len; - int ret = 0; - struct wp_context *wp; - int num_entries, wp_en; - - if (generic_range_table(flash, &wp, &num_entries)) - return -1; - - sr1 = do_read_status(flash); - wp_en = (sr1 >> wp->sr1.srp_pos) & 1; - - msg_cinfo("WP: status: 0x%04x\n", sr1); - msg_cinfo("WP: status.srp0: %x\n", wp_en); - /* FIXME: SRP1 is not really generic, but we probably should print - * it anyway to have consistent output. #legacycruft */ - msg_cinfo("WP: status.srp1: %x\n", 0); - msg_cinfo("WP: write protect is %s.\n", - wp_en ? "enabled" : "disabled"); - - msg_cinfo("WP: write protect range: "); - if (generic_status_to_range(flash, sr1, &start, &len)) { - msg_cinfo("(cannot resolve the range)\n"); - ret = -1; - } else { - msg_cinfo("start=0x%08x, len=0x%08x\n", start, len); - } - - return ret; -} - -struct wp wp_generic = { - .list_ranges = generic_list_ranges, - .set_range = generic_set_range, - .enable = generic_enable_writeprotect, - .disable = generic_disable_writeprotect, - .wp_status = wp_context_status, -}; diff --git a/writeprotect.h b/writeprotect.h index bded4c8..2d99897 100644 --- a/writeprotect.h +++ b/writeprotect.h @@ -18,34 +18,4 @@ #ifndef __WRITEPROTECT_H__ #define __WRITEPROTECT_H__ 1
-enum wp_mode { - WP_MODE_UNKNOWN = -1, - WP_MODE_HARDWARE, /* hardware WP pin determines status */ - WP_MODE_POWER_CYCLE, /* WP active until power off/on cycle */ - WP_MODE_PERMANENT, /* status register permanently locked, - WP permanently enabled */ -}; - -struct wp { - int (*list_ranges)(const struct flashctx *flash); - int (*set_range)(const struct flashctx *flash, - unsigned int start, unsigned int len); - int (*enable)(const struct flashctx *flash, enum wp_mode mode); - int (*disable)(const struct flashctx *flash); - int (*wp_status)(const struct flashctx *flash); -}; - -extern struct wp wp_generic; - -enum wp_mode get_wp_mode(const char *mode_str); - -/* - * Generic write-protect stuff - */ - -struct modifier_bits { - int sec; /* if 1, bp bits describe sectors */ - int tb; /* value of top/bottom select bit */ -}; - #endif /* !__WRITEPROTECT_H__ */
16 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one.
-
Anastasia Klimchuk (Code Review)