Signed-off-by: Alexander Couzens lynxis@fe80.eu --- Hi everyone,
I've rewritten read/write/verify functions to get romregions/layouts working in all cases. A write to a specific region required before a complete read of the chip. Because of this it wasn't possible to write to a specific region, when another region is locked (ME lockdown). And flashrom is faster now when writing to a rom region.
Best, lynxis
I've melt all my commits into a single one. A small commit version can be found at https://github.com/lynxis/flashrom/tree/v1/
cli_classic.c | 5 - flash.h | 13 +++ flashrom.c | 336 +++++++++++++++++++++++++++++++++++----------------------- layout.c | 23 ++-- 4 files changed, 228 insertions(+), 149 deletions(-)
diff --git a/cli_classic.c b/cli_classic.c index 8588881..613c7aa 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -368,11 +368,6 @@ int main(int argc, char *argv[]) ret = 1; goto out; } - if (layoutfile != NULL && !write_it) { - msg_gerr("Layout files are currently supported for write operations only.\n"); - ret = 1; - goto out; - }
if (process_include_args()) { ret = 1; diff --git a/flash.h b/flash.h index 03b26e7..ec85398 100644 --- a/flash.h +++ b/flash.h @@ -337,6 +337,19 @@ __attribute__((format(printf, 2, 3))); #define msg_cspew(...) print(MSG_SPEW, __VA_ARGS__) /* chip debug spew */
/* layout.c */ + +#define MAX_ROMLAYOUT 32 + +typedef struct { + chipoff_t start; + chipoff_t end; + unsigned int included; + char name[256]; +} romentry_t; + +extern romentry_t rom_entries[]; +extern int num_rom_entries; + int register_include_arg(char *name); int process_include_args(void); int read_romlayout(const char *name); diff --git a/flashrom.c b/flashrom.c index 9b82d4c..b2c59c2 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1434,45 +1434,67 @@ static int erase_and_write_block_helper(struct flashctx *flash, return ret; }
-static int walk_eraseregions(struct flashctx *flash, int erasefunction, - int (*do_something) (struct flashctx *flash, - unsigned int addr, - unsigned int len, - uint8_t *param1, - uint8_t *param2, - int (*erasefn) ( - struct flashctx *flash, - unsigned int addr, - unsigned int len)), - void *param1, void *param2) +static int walk_eraseregions_offset_len( + struct flashctx *flash, int erasefunction, + int (*do_something) (struct flashctx *flash, + unsigned int addr, + unsigned int len, + uint8_t *param1, + uint8_t *param2, + int (*erasefn) ( + struct flashctx *flash, + unsigned int addr, + unsigned int len) + ), + void *oldcontent, + void *newcontent, + unsigned offset, + unsigned len) { int i, j; unsigned int start = 0; - unsigned int len; + unsigned int eraselen; struct block_eraser eraser = flash->chip->block_erasers[erasefunction];
for (i = 0; i < NUM_ERASEREGIONS; i++) { /* count==0 for all automatically initialized array * members so the loop below won't be executed for them. */ - len = eraser.eraseblocks[i].size; - for (j = 0; j < eraser.eraseblocks[i].count; j++) { + eraselen = eraser.eraseblocks[i].size; + for (j = 0; j < eraser.eraseblocks[i].count; j++, start += eraselen) { + if (start < offset) + continue; + /* Print this for every block except the first one. */ if (i || j) msg_cdbg(", "); - msg_cdbg("0x%06x-0x%06x", start, - start + len - 1); - if (do_something(flash, start, len, param1, param2, - eraser.block_erase)) { + msg_cdbg("0x%06x-0x%06x", start, start + eraselen - 1); + if (do_something(flash, start, eraselen, oldcontent, newcontent, eraser.block_erase)) { return 1; } - start += len; } } msg_cdbg("\n"); return 0; }
+static int walk_eraseregions(struct flashctx *flash, int erasefunction, + int (*do_something) (struct flashctx *flash, + unsigned int addr, + unsigned int len, + uint8_t *param1, + uint8_t *param2, + int (*erasefn) ( + struct flashctx *flash, + unsigned int addr, + unsigned int len) + ), + void *oldcontent, + void *newcontent) +{ + return walk_eraseregions_offset_len(flash, erasefunction, do_something, oldcontent, newcontent, 0, flash->chip->total_size); +} + static int check_block_eraser(const struct flashctx *flash, int k, int log) { struct block_eraser eraser = flash->chip->block_erasers[k]; @@ -1562,7 +1584,7 @@ int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents, uint8_t } return ret; } - +/* FIXME: Unused! static void nonfatal_help_message(void) { msg_gerr("Good, writing to the flash chip apparently didn't do anything.\n"); @@ -1579,7 +1601,7 @@ static void nonfatal_help_message(void) "the programmer and the flash chip. If you think the error is caused by flashrom\n" "please report this on IRC at chat.freenode.net (channel #flashrom) or\n" "mail flashrom@flashrom.org, thanks!\n"); -} +} */
static void emergency_help_message(void) { @@ -1895,153 +1917,201 @@ int chip_safety_check(const struct flashctx *flash, int force, int read_it, int return 0; }
-/* This function signature is horrible. We need to design a better interface, - * but right now it allows us to split off the CLI code. - * Besides that, the function itself is a textbook example of abysmal code flow. - */ -int doit(struct flashctx *flash, int force, const char *filename, int read_it, - int write_it, int erase_it, int verify_it) -{ - uint8_t *oldcontents; - uint8_t *newcontents; +int read_flash(struct flashctx *flash, const char *filename) { int ret = 0; - unsigned long size = flash->chip->total_size * 1024; + int err = 0; /* used as ret for read_flash if a single partition fails */ + size_t flashsize = flash->chip->total_size * 1024; + int i;
- if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) { - msg_cerr("Aborting.\n"); - return 1; - } + uint8_t *contents = calloc(1, flashsize);
- if (normalize_romentries(flash)) { - msg_cerr("Requested regions can not be handled. Aborting.\n"); - return 1; - } + for (i=0; i < num_rom_entries; i++) { + chipoff_t start = rom_entries[i].start; + chipoff_t end = rom_entries[i].end; + chipsize_t length = end - start;
- /* Given the existence of read locks, we want to unlock for read, - * erase and write. - */ - if (flash->chip->unlock) - flash->chip->unlock(flash); + /* ignore entries not included */ + if (!rom_entries[i].included) + continue;
- if (read_it) { - return read_flash_to_file(flash, filename); + ret = flash->chip->read(flash, contents + start, start, length); + if (ret) { + msg_cerr("Read for part %s (0x%x - 0x%x) failed!", rom_entries[i].name, start, end); + err = -1; + continue; + } }
- oldcontents = malloc(size); - if (!oldcontents) { - msg_gerr("Out of memory!\n"); - exit(1); + ret = write_buf_to_file(contents, flashsize, filename); + if (ret) { + msg_cerr("Writing to file %s failed!", filename); + err = -1; } + + free(contents); + return err; +} + +int write_flash(struct flashctx *flash, const char *filename) { + int ret = 0; + size_t flashsize = flash->chip->total_size * 1024; + int i; + + uint8_t *oldcontents = calloc(1, flashsize); + uint8_t *newcontents = calloc(1, flashsize); + /* Assume worst case: All bits are 0. */ - memset(oldcontents, 0x00, size); - newcontents = malloc(size); - if (!newcontents) { - msg_gerr("Out of memory!\n"); - exit(1); - } - /* Assume best case: All bits should be 1. */ - memset(newcontents, 0xff, size); + memset(oldcontents, 0x00, flashsize); + + /* Assume worst case: All bits are 0. */ + memset(newcontents, 0xff, flashsize); /* Side effect of the assumptions above: Default write action is erase * because newcontents looks like a completely erased chip, and * oldcontents being completely 0x00 means we have to erase everything * before we can write. */
- if (erase_it) { + if(read_buf_from_file(newcontents, flashsize, filename)) { + msg_cerr("Can not read file %s\n. Aborting.\n", filename); + return -1; + } + +#if CONFIG_INTERNAL == 1 + /* FIXME: move this code into a small function and move it out of write_flash */ + if (programmer == PROGRAMMER_INTERNAL && cb_check_image(newcontents, flashsize) < 0) { + if (force_boardmismatch) { + msg_pinfo("Proceeding anyway because user forced us to.\n"); + } else { + msg_perr("Aborting. You can override this with " + "-p internal:boardmismatch=force.\n"); + return 1; + } + } +#endif + + for (i=0; i < num_rom_entries; i++) { + chipoff_t start = rom_entries[i].start; + chipoff_t end = rom_entries[i].end; + chipsize_t length = end - start; + + /* ignore entries not included */ + if (!rom_entries[i].included) + continue; + + /* read specified flash region */ + ret = flash->chip->read(flash, oldcontents + start, start, length); + if(ret) { + msg_cerr("Can not read flash position 0x%x len: 0x%x\n. Ret: %d Ignoring.\n", start, length, ret); + return -1; + } + + int k; + for (k = 0; k < NUM_ERASEFUNCTIONS; k++) { + if (k != 0) + msg_cinfo("Looking for another erase function.\n"); + + msg_cdbg("Trying erase function %i... ", k); + if (check_block_eraser(flash, k, 1)) + continue; + + ret = walk_eraseregions_offset_len(flash, k, &erase_and_write_block_helper, oldcontents, newcontents, start, length); + + /* If everything is OK, don't try another erase function. */ + if (!ret) + break; + } + + if (!ret) { + msg_cinfo("Finished flashing %d - region %s", ret, rom_entries[i].name); + } else { + msg_cerr("Failed flashing!"); + emergency_help_message(); /* FIXME: Do we really want the scary warning if erase failed? * After all, after erase the chip is either blank or partially * blank or it has the old contents. A blank chip won't boot, * so if the user wanted erase and reboots afterwards, the user * knows very well that booting won't work. */ - if (erase_and_write_flash(flash, oldcontents, newcontents)) { - emergency_help_message(); - ret = 1; } - goto out; }
- if (write_it || verify_it) { - if (read_buf_from_file(newcontents, size, filename)) { - ret = 1; - goto out; - } + return 0; +}
-#if CONFIG_INTERNAL == 1 - if (programmer == PROGRAMMER_INTERNAL && cb_check_image(newcontents, size) < 0) { - if (force_boardmismatch) { - msg_pinfo("Proceeding anyway because user forced us to.\n"); - } else { - msg_perr("Aborting. You can override this with " - "-p internal:boardmismatch=force.\n"); - ret = 1; - goto out; - } - } -#endif +int verify_flash(struct flashctx *flash, const char *filename) { + int ret = 0; + int verified = 0; + size_t flashsize = flash->chip->total_size * 1024; + int i; + + /* we alloc here the whole flashsize to be sure we have enought space */ + uint8_t *filecontents = calloc(1, flashsize); + + if(read_buf_from_file(filecontents, flashsize, filename)) { + msg_cerr("Can not read file %s\n. Aborting.\n", filename); + return -1; }
- /* Read the whole chip to be able to check whether regions need to be - * erased and to give better diagnostics in case write fails. - * The alternative would be to read only the regions which are to be - * preserved, but in that case we might perform unneeded erase which - * takes time as well. - */ - msg_cinfo("Reading old flash chip contents... "); - if (flash->chip->read(flash, oldcontents, 0, size)) { - ret = 1; - msg_cinfo("FAILED.\n"); - goto out; + for (i=0; i < num_rom_entries; i++) { + chipoff_t start = rom_entries[i].start; + chipoff_t end = rom_entries[i].end; + chipsize_t length = end - start; + + /* ignore entries not included */ + if (!rom_entries[i].included) + continue; + + ret = verify_range(flash, filecontents + start, start, length); + if (ret) + verified = -1; } - msg_cinfo("done.\n");
- /* Build a new image taking the given layout into account. */ - build_new_image(flash, oldcontents, newcontents); + return verified; +} + +/* This function signature is horrible. We need to design a better interface, + * but right now it allows us to split off the CLI code. + * Besides that, the function itself is a textbook example of abysmal code flow. + */ +int doit(struct flashctx *flash, int force, const char *filename, int read_it, + int write_it, int erase_it, int verify_it) +{ + + int ret = 0;
- // //////////////////////////////////////////////////////////// + if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) { + msg_cerr("Aborting.\n"); + return 1; + }
- if (write_it) { - if (erase_and_write_flash(flash, oldcontents, newcontents)) { - msg_cerr("Uh oh. Erase/write failed. Checking if anything has changed.\n"); - msg_cinfo("Reading current flash chip contents... "); - if (!flash->chip->read(flash, newcontents, 0, size)) { - msg_cinfo("done.\n"); - if (!memcmp(oldcontents, newcontents, size)) { - nonfatal_help_message(); - ret = 1; - goto out; - } - msg_cerr("Apparently at least some data has changed.\n"); - } else - msg_cerr("Can't even read anymore!\n"); - emergency_help_message(); - ret = 1; - goto out; - } + if (normalize_romentries(flash)) { + msg_cerr("Requested regions can not be handled. Aborting.\n"); + return 1; }
- /* Verify only if we either did not try to write (verify operation) or actually changed something. */ - if (verify_it && (!write_it || !all_skipped)) { - msg_cinfo("Verifying flash... "); + /* Given the existence of read locks, we want to unlock for read, + * erase and write. + */ + if (flash->chip->unlock) + flash->chip->unlock(flash);
- if (write_it) { - /* Work around chips which need some time to calm down. */ - programmer_delay(1000*1000); - ret = verify_range(flash, newcontents, 0, size); - /* If we tried to write, and verification now fails, we - * might have an emergency situation. - */ - if (ret) - emergency_help_message(); - } else { - ret = compare_range(newcontents, oldcontents, 0, size); - } - if (!ret) - msg_cinfo("VERIFIED.\n"); + if (read_it) { + return read_flash(flash, filename); }
-out: - free(oldcontents); - free(newcontents); + if (write_it) { + ret = write_flash(flash, filename); + if (ret) + return ret; + } + + if (verify_it) { + ret = verify_flash(flash, filename); + if (ret) + return ret; + } + + msg_cinfo("done.\n"); + return ret; } diff --git a/layout.c b/layout.c index a12eb28..9bdf9b6 100644 --- a/layout.c +++ b/layout.c @@ -26,18 +26,9 @@ #include "flash.h" #include "programmer.h"
-#define MAX_ROMLAYOUT 32 - -typedef struct { - chipoff_t start; - chipoff_t end; - unsigned int included; - char name[256]; -} romentry_t; - /* rom_entries store the entries specified in a layout file and associated run-time data */ -static romentry_t rom_entries[MAX_ROMLAYOUT]; -static int num_rom_entries = 0; /* the number of successfully parsed rom_entries */ +romentry_t rom_entries[MAX_ROMLAYOUT]; +int num_rom_entries = 0; /* the number of successfully parsed rom_entries */
/* include_args holds the arguments specified at the command line with -i. They must be processed at some point * so that desired regions are marked as "included" in the rom_entries list. */ @@ -254,6 +245,16 @@ int normalize_romentries(const struct flashctx *flash) } }
+ /* create a partition over the complete flash when no layout given */ + if (num_rom_entries == 0) { + rom_entries[0].start = 0x0; + rom_entries[0].end = flash->chip->total_size * 1024 - 1; + rom_entries[0].included = 1; + strncpy(rom_entries[0].name, "complete flash", 15); + + num_rom_entries = 1; + } + return ret; }