[flashrom] Patches + RFC: Deal with locked ICH flash descriptor regions transparently
David Hendricks
dhendrix at google.com
Sat Dec 3 02:38:20 CET 2011
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 at 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 at 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 at 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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20111202/90770f67/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 1-3_error_handling_helper_function.diff
Type: text/x-patch
Size: 1742 bytes
Desc: not available
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20111202/90770f67/attachment.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 2-3_update_spi_error_handling.diff
Type: text/x-patch
Size: 8587 bytes
Desc: not available
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20111202/90770f67/attachment-0001.diff>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 3-3_ichspi_deal_with_locked_regions.diff
Type: text/x-patch
Size: 6860 bytes
Desc: not available
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20111202/90770f67/attachment-0002.diff>
More information about the flashrom
mailing list