Patches + RFC: Deal with locked ICH flash descriptor regions transparently
The following patches (attached and in-lined below) add support for "graceful" handling of certain errors and in particular the read/write errors that occur on newer Intel chipsets. With a little work, it could be extended to handle SPI hardware write protection schema, too. Note: I think we should probably add a "I_want_a_brick" programmer parameter for those daring enough to try this. I suspect certain OEM tools use unknown Mebx commands<http://support.dell.com/support/edocs/systems/latd630/en/amt/MEBX.htm>to disable ME read/write protection when they do updates. Perhaps that stuff can be added later as a board_enable thing. The impetus for this is to overcome difficulties in dealing with new Intel platforms which use their flash descriptor layout, like this (using Pontus' H57 verbose output): 0x54: 0x00000000 (FREG0: Flash Descriptor) 0x00000000-0x00000fff is read-only 0x58: 0x07ff0600 (FREG1: BIOS) 0x00600000-0x007fffff is read-write 0x5C: 0x05ff0001 (FREG2: Management Engine) 0x00001000-0x005fffff is locked 0x60: 0x00000fff (FREG3: Gigabit Ethernet) Gigabit Ethernet region is unused. 0x64: 0x00000fff (FREG4: Platform Data) Platform Data region is unused. In this case, the flashrom will abort due to a transaction error when it attempts to read or write offsets 0x001000-0x05fffff, or when it attempt to write new content to 0x000000-0x000fff. With the patches applied, the low-level ICH code will handle all the details of checking the address associated with an opcode against the permissions set in the flash descriptor. If an operation cannot be performed on a given region, an error code is propagated up the stack so that high-level read/write logic can decide to skip that region. As is, the patch will make the high-level logic fill in unreadable regions with 0xff bytes (for reads) and will quietly skip over unwriteable regions (for writes). The biggest downside is, of course, that read/write operations no longer do exactly what one might expect. A full read operation (flashrom -r) will give you a ROM-sized image with 0xff bytes wherever read is forbidden (e.g. where the management engine firmware lives). A full write operation will skip forbidden regions (e.g. flash descriptor and ME) which may leave stuff in an inconsistent and potentially unusable state (thanks, Intel!). Signed-off-by: David Hendricks <dhendrix@google.com> 1-3_error_handling_helper_function.diff: Index: flashrom-me/flash.h =================================================================== --- flashrom-me.orig/flash.h +++ flashrom-me/flash.h @@ -226,12 +226,25 @@ int write_buf_to_file(unsigned char *buf #define OK 0 #define NT 1 /* Not tested */ +/* what to do in case of an error */ +enum error_action { + error_fail, /* fail immediately */ + error_ignore, /* non-fatal error; continue */ +}; + /* Something happened that shouldn't happen, but we can go on. */ #define ERROR_NONFATAL 0x100 /* Something happened that shouldn't happen, we'll abort. */ #define ERROR_FATAL -0xee +/* Operation failed due to access restriction set in programmer or flash chip */ +#define ACCESS_DENIED -7 +extern enum error_action access_denied_action; + +/* convenience function for checking return codes */ +extern int ignore_error(int x); + /* cli_output.c */ /* Let gcc and clang check for correct printf-style format strings. */ int print(int type, const char *fmt, ...) __attribute__((format(printf, 2, 3))); Index: flashrom-me/flashrom.c =================================================================== --- flashrom-me.orig/flashrom.c +++ flashrom-me/flashrom.c @@ -42,6 +42,25 @@ const char flashrom_version[] = FLASHROM char *chip_to_probe = NULL; int verbose = 0; +/* error handling stuff */ +enum error_action access_denied_action = error_ignore; + +/* returns boolean (1 if caller should ignore error code, 0 if not) */ +int ignore_error(int err) { + int ret = 0; + + switch(err) { + case ACCESS_DENIED: + if (access_denied_action == error_ignore) + ret = 1; + break; + default: + break; + } + + return ret; +} + static enum programmer programmer = PROGRAMMER_INVALID; static char *programmer_param = NULL; 2-3_update_spi_error_handling.diff: Index: flashrom-me/spi.c =================================================================== --- flashrom-me.orig/spi.c +++ flashrom-me/spi.c @@ -100,25 +100,35 @@ int default_spi_send_multicommand(struct int default_spi_read(struct flashchip *flash, uint8_t *buf, unsigned int start, unsigned int len) { unsigned int max_data = spi_programmer->max_data_read; + int ret; if (max_data == MAX_DATA_UNSPECIFIED) { msg_perr("%s called, but SPI read chunk size not defined " "on this hardware. Please report a bug at " "flashrom@flashrom.org\n", __func__); return 1; } - return spi_read_chunked(flash, buf, start, len, max_data); + rc = spi_read_chunked(flash, buf, start, len, max_data); + /* translate SPI-specific access denied error to generic error */ + if (ret == SPI_ACCESS_DENIED) + rc = ACCESS_DENIED; + return ret; } int default_spi_write_256(struct flashchip *flash, uint8_t *buf, unsigned int start, unsigned int len) { unsigned int max_data = spi_programmer->max_data_write; + int ret; if (max_data == MAX_DATA_UNSPECIFIED) { msg_perr("%s called, but SPI write chunk size not defined " "on this hardware. Please report a bug at " "flashrom@flashrom.org\n", __func__); return 1; } - return spi_write_chunked(flash, buf, start, len, max_data); + ret = spi_write_chunked(flash, buf, start, len, max_data); + /* translate SPI-specific access denied error to generic error */ + if (ret == SPI_ACCESS_DENIED) + ret = ACCESS_DENIED; + return ret; } int spi_chip_read(struct flashchip *flash, uint8_t *buf, unsigned int start, unsigned int len) Index: flashrom-me/spi.h =================================================================== --- flashrom-me.orig/spi.h +++ flashrom-me/spi.h @@ -125,5 +125,6 @@ #define SPI_INVALID_LENGTH -4 #define SPI_FLASHROM_BUG -5 #define SPI_PROGRAMMER_ERROR -6 +#define SPI_ACCESS_DENIED -7 #endif /* !__SPI_H__ */ Index: flashrom-me/spi25.c =================================================================== --- flashrom-me.orig/spi25.c +++ flashrom-me/spi25.c @@ -916,8 +916,10 @@ int spi_nbyte_program(unsigned int addr, result = spi_send_multicommand(cmds); if (result) { - msg_cerr("%s failed during command execution at address 0x%x\n", - __func__, addr); + if (result != SPI_ACCESS_DENIED) { + msg_cerr("%s failed during command execution at address 0x%x\n", + __func__, addr); + } } return result; } @@ -970,7 +972,7 @@ int spi_nbyte_read(unsigned int address, */ int spi_read_chunked(struct flashchip *flash, uint8_t *buf, unsigned int start, unsigned int len, unsigned int chunksize) { - int rc = 0; + int rc = 0, chunk_status = 0; unsigned int i, j, starthere, lenhere, toread; unsigned int page_size = flash->page_size; @@ -990,12 +992,26 @@ int spi_read_chunked(struct flashchip *f /* Length of bytes in the range in this page. */ lenhere = min(start + len, (i + 1) * page_size) - starthere; for (j = 0; j < lenhere; j += chunksize) { + uint8_t *bytes = buf + starthere - start + j; + toread = min(chunksize, lenhere - j); - rc = spi_nbyte_read(starthere + j, buf + starthere - start + j, toread); - if (rc) - break; + chunk_status = spi_nbyte_read(starthere + j, bytes, toread); + + if (chunk_status) { + if (ignore_error(chunk_status)) { + /* fill this chunk with 0xff bytes and + let caller know about the error */ + memset(bytes, 0xff, toread); + rc = chunk_status; + chunk_status = 0; + continue; + } else { + rc = chunk_status; + break; + } + } } - if (rc) + if (chunk_status) break; } Index: flashrom-me/flashrom.c =================================================================== --- flashrom-me.orig/flashrom.c +++ flashrom-me/flashrom.c @@ -583,6 +583,7 @@ int verify_range(struct flashchip *flash unsigned int i; uint8_t *readbuf = malloc(len); int ret = 0, failcount = 0; + unsigned int chunksize; if (!len) goto out_free; @@ -606,23 +607,35 @@ int verify_range(struct flashchip *flash if (!message) message = "VERIFY"; - ret = flash->read(flash, readbuf, start, len); - if (ret) { - msg_gerr("Verification impossible because read failed " - "at 0x%x (len 0x%x)\n", start, len); - return ret; - } + chunksize = min(flash->page_size, len); + for (i = 0; i < len; i += chunksize) { + int tmp, j; + + tmp = flash->read(flash, readbuf + i, start + i, chunksize); + + if (tmp) { + ret = tmp; + if (ignore_error(tmp)) { + chunksize = min(flash->page_size, len - i); + continue; + } else { + goto out_free; + } + } - for (i = 0; i < len; i++) { - if (cmpbuf[i] != readbuf[i]) { - /* Only print the first failure. */ - if (!failcount++) - msg_cerr("%s FAILED at 0x%08x! " - "Expected=0x%02x, Read=0x%02x,", - message, start + i, cmpbuf[i], - readbuf[i]); + for (j = 0; j < chunksize; j++) { + if (cmpbuf[i + j] != readbuf[i + j]) { + /* Only print the first failure. */ + if (!failcount++) + msg_cerr("%s FAILED at 0x%08x! " + "Expected=0x%02x, Read=0x%02x,", + message, start + i + j, cmpbuf[i + j], + readbuf[j]); + } } + chunksize = min(flash->page_size, len - i); } + if (failcount) { msg_cerr(" failed byte count from 0x%08x-0x%08x: 0x%x\n", start, start + len - 1, failcount); @@ -1057,6 +1070,16 @@ int verify_flash(struct flashchip *flash ret = verify_range(flash, buf, 0, total_size, NULL); + if (ret == ACCESS_DENIED) { + msg_gdbg("Could not fully verify due to access error, "); + if (access_denied_action == error_ignore) { + msg_gdbg("ignoring\n"); + ret = 0; + } else { + msg_gdbg("aborting\n"); + } + } + if (!ret) msg_cinfo("VERIFIED. \n"); @@ -1139,10 +1162,15 @@ int read_flash_to_file(struct flashchip ret = 1; goto out_free; } - if (flash->read(flash, buf, 0, size)) { - msg_cerr("Read operation failed!\n"); - ret = 1; - goto out_free; + + ret = flash->read(flash, buf, 0, size); + if (ret) { + if (ignore_error(ret)) { + ret = 0; + } else { + msg_cerr("Read operation failed!\n"); + goto out_free; + } } ret = write_buf_to_file(buf, size, filename); @@ -1243,8 +1271,14 @@ static int erase_and_write_block_helper( if (need_erase(curcontents, newcontents, len, gran)) { msg_cdbg("E"); ret = erasefn(flash, start, len); - if (ret) + if (ret) { + if (ret == ACCESS_DENIED) + msg_cdbg("D"); + else + msg_cerr("ERASE FAILED!\n"); return ret; + } + if (check_erased_range(flash, start, len)) { msg_cerr("ERASE FAILED!\n"); return -1; @@ -1262,8 +1296,11 @@ static int erase_and_write_block_helper( /* Needs the partial write function signature. */ ret = flash->write(flash, newcontents + starthere, start + starthere, lenhere); - if (ret) + if (ret) { + if (ret == ACCESS_DENIED) + msg_cdbg("D"); return ret; + } starthere += lenhere; skip = 0; } @@ -1284,7 +1321,7 @@ static int walk_eraseregions(struct flas unsigned int len)), void *param1, void *param2) { - int i, j; + int i, j, ret = 0; unsigned int start = 0; unsigned int len; struct block_eraser eraser = flash->block_erasers[erasefunction]; @@ -1300,15 +1337,19 @@ static int walk_eraseregions(struct flas msg_cdbg(", "); msg_cdbg("0x%06x-0x%06x", start, start + len - 1); - if (do_something(flash, start, len, param1, param2, - eraser.block_erase)) { - return 1; + ret = do_something(flash, start, len, param1, param2, + eraser.block_erase); + if (ret) { + if (ignore_error(ret)) + ret = 0; + else + return ret; } start += len; } } msg_cdbg("\n"); - return 0; + return ret; } static int check_block_eraser(const struct flashchip *flash, int k, int log) @@ -1695,7 +1736,7 @@ int doit(struct flashchip *flash, int fo { uint8_t *oldcontents; uint8_t *newcontents; - int ret = 0; + int ret = 0, tmp; unsigned long size = flash->total_size * 1024; if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) { @@ -1768,10 +1809,15 @@ int doit(struct flashchip *flash, int fo * takes time as well. */ msg_cinfo("Reading old flash chip contents... "); - if (flash->read(flash, oldcontents, 0, size)) { - ret = 1; - msg_cinfo("FAILED.\n"); - goto out; + tmp = flash->read(flash, oldcontents, 0, size); + if (tmp) { + if (ignore_error(tmp)) { + msg_gdbg("ignoring error\n"); + } else { + ret = 1; + msg_cinfo("FAILED.\n"); + goto out; + } } msg_cinfo("done.\n"); 3-3_ichspi_deal_with_locked_regions.diff: Index: flashrom-me/ichspi.c =================================================================== --- flashrom-me.orig/ichspi.c +++ flashrom-me/ichspi.c @@ -983,6 +983,95 @@ static int run_opcode(OPCODE op, uint32_ } } +#define DEFAULT_NUM_FD_REGIONS 5 +static int num_fd_regions; + +const char *const region_names[] = { + "Flash Descriptor", "BIOS", "Management Engine", + "Gigabit Ethernet", "Platform Data" +}; + +enum fd_access_level { + FD_REGION_LOCKED, + FD_REGION_READ_ONLY, + FD_REGION_WRITE_ONLY, + FD_REGION_READ_WRITE, +}; + +struct fd_region_permission { + enum fd_access_level level; + const char *name; +} fd_region_permissions[] = { + /* order corresponds to FRAP bitfield */ + { FD_REGION_LOCKED, "locked" }, + { FD_REGION_READ_ONLY, "read-only" }, + { FD_REGION_WRITE_ONLY, "write-only" }, + { FD_REGION_READ_WRITE, "read-write" }, +}; + +/* FIXME: Replace usage of access_names with the region_access struct */ +const char *const access_names[4] = { + "locked", "read-only", "write-only", "read-write" +}; + +struct fd_region { + const char *name; + struct fd_region_permission *permission; + uint32_t base; + uint32_t limit; +} fd_regions[] = { + /* order corresponds to flash descriptor */ + { .name = "Flash Descriptor" }, + { .name = "BIOS" }, + { .name = "Management Engine" }, + { .name = "Gigabit Ethernet" }, + { .name = "Platform Data" }, +}; + +static int check_fd_permissions(OPCODE *opcode, uint32_t addr, int count) +{ + int i; + uint8_t type = opcode->spi_type; + int ret = 0; + + /* check flash descriptor permissions (if present) */ + for (i = 0; i < num_fd_regions; i++) { + const char *name = fd_regions[i].name; + enum fd_access_level level; + + if ((addr + count - 1 < fd_regions[i].base) || + (addr > fd_regions[i].limit)) + continue; + + if (!fd_regions[i].permission) { + msg_perr("No permissions set for flash region %s\n", + fd_regions[i].name); + break; + } + + level = fd_regions[i].permission->level; + + if (type == SPI_OPCODE_TYPE_READ_WITH_ADDRESS) { + if (level != FD_REGION_READ_ONLY && + level != FD_REGION_READ_WRITE) { + msg_pspew("%s: Cannot read address 0x%08x in " + "region %s\n", __func__,addr,name); + ret = SPI_ACCESS_DENIED; + } + } else if (type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS) { + if (level != FD_REGION_WRITE_ONLY && + level != FD_REGION_READ_WRITE) { + msg_pspew("%s: Cannot write to address 0x%08x in" + "region %s\n", __func__,addr,name); + ret = SPI_ACCESS_DENIED; + } + } + break; + } + + return ret; +} + static int ich_spi_send_command(unsigned int writecnt, unsigned int readcnt, const unsigned char *writearr, unsigned char *readarr) { @@ -1045,19 +1134,6 @@ static int ich_spi_send_command(unsigned return SPI_INVALID_LENGTH; } - /* if opcode-type requires an address */ - if (opcode->spi_type == SPI_OPCODE_TYPE_READ_WITH_ADDRESS || - opcode->spi_type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS) { - addr = (writearr[1] << 16) | - (writearr[2] << 8) | (writearr[3] << 0); - if (addr < ichspi_bbar) { - msg_perr("%s: Address 0x%06x below allowed " - "range 0x%06x-0xffffff\n", __func__, - addr, ichspi_bbar); - return SPI_INVALID_ADDRESS; - } - } - /* Translate read/write array/count. * The maximum data length is identical for the maximum read length and * for the maximum write length excluding opcode and address. Opcode and @@ -1076,6 +1152,24 @@ static int ich_spi_send_command(unsigned count = readcnt; } + /* if opcode-type requires an address */ + if (opcode->spi_type == SPI_OPCODE_TYPE_READ_WITH_ADDRESS || + opcode->spi_type == SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS) { + addr = (writearr[1] << 16) | + (writearr[2] << 8) | (writearr[3] << 0); + if (addr < ichspi_bbar) { + msg_perr("%s: Address 0x%06x below allowed " + "range 0x%06x-0xffffff\n", __func__, + addr, ichspi_bbar); + return SPI_INVALID_ADDRESS; + } + if (num_fd_regions > 0) { + result = check_fd_permissions(opcode, addr, count); + if (result) + return result; + } + } + result = run_opcode(*opcode, addr, count, data); if (result) { msg_pdbg("Running OPCODE 0x%02x failed ", opcode->opcode); @@ -1421,32 +1515,25 @@ static int ich_spi_send_multicommand(str static void do_ich9_spi_frap(uint32_t frap, int i) { - static const char *const access_names[4] = { - "locked", "read-only", "write-only", "read-write" - }; - static const char *const region_names[5] = { - "Flash Descriptor", "BIOS", "Management Engine", - "Gigabit Ethernet", "Platform Data" - }; - uint32_t base, limit; int rwperms = (((ICH_BRWA(frap) >> i) & 1) << 1) | (((ICH_BRRA(frap) >> i) & 1) << 0); int offset = ICH9_REG_FREG0 + i * 4; uint32_t freg = mmio_readl(ich_spibar + offset); msg_pdbg("0x%02X: 0x%08x (FREG%i: %s)\n", - offset, freg, i, region_names[i]); + offset, freg, i, fd_regions[i].name); - base = ICH_FREG_BASE(freg); - limit = ICH_FREG_LIMIT(freg); - if (base > limit) { + fd_regions[i].base = ICH_FREG_BASE(freg); + fd_regions[i].limit = ICH_FREG_LIMIT(freg) | 0x0fff; + fd_regions[i].permission = &fd_region_permissions[rwperms]; + if (fd_regions[i].base > fd_regions[i].limit) { /* this FREG is disabled */ msg_pdbg("%s region is unused.\n", region_names[i]); return; } - msg_pdbg("0x%08x-0x%08x is %s\n", base, (limit | 0x0fff), - access_names[rwperms]); + msg_pdbg("0x%08x-0x%08x is %s\n", fd_regions[i].base, + fd_regions[i].limit, fd_regions[i].permission->name); } /* In contrast to FRAP and the master section of the descriptor the bits @@ -1460,9 +1547,6 @@ static void do_ich9_spi_frap(uint32_t fr static void prettyprint_ich9_reg_pr(int i) { - static const char *const access_names[4] = { - "locked", "read-only", "write-only", "read-write" - }; uint8_t off = ICH9_REG_PR0 + (i * 4); uint32_t pr = mmio_readl(ich_spibar + off); int rwperms = ICH_PR_PERMS(pr); @@ -1647,6 +1731,7 @@ int ich_init_spi(struct pci_dev *dev, ui ich_init_opcodes(); if (desc_valid) { + num_fd_regions = DEFAULT_NUM_FD_REGIONS; tmp2 = mmio_readw(ich_spibar + ICH9_REG_HSFC); msg_pdbg("0x06: 0x%04x (HSFC)\n", tmp2); prettyprint_ich9_reg_hsfc(tmp2); @@ -1664,15 +1749,15 @@ int ich_init_spi(struct pci_dev *dev, ui msg_pdbg("BRRA 0x%02x\n", ICH_BRRA(tmp)); /* Decode and print FREGx and FRAP registers */ - for (i = 0; i < 5; i++) + for (i = 0; i < num_fd_regions; i++) do_ich9_spi_frap(tmp, i); } /* try to disable PR locks before printing them */ if (!ichspi_lock) - for (i = 0; i < 5; i++) + for (i = 0; i < num_fd_regions; i++) ich9_set_pr(i, 0, 0); - for (i = 0; i < 5; i++) + for (i = 0; i < num_fd_regions; i++) prettyprint_ich9_reg_pr(i); tmp = mmio_readl(ich_spibar + ICH9_REG_SSFS); -- David Hendricks (dhendrix) Systems Software Engineer, Google Inc.
participants (1)
-
David Hendricks