Change in flashrom[master]: [v4, 6/6] Add support for reading the current flash contents from a file
Mike Banon has uploaded this change for review. ( https://review.coreboot.org/23263
Change subject: [v4,6/6] Add support for reading the current flash contents from a file ......................................................................
[v4,6/6] Add support for reading the current flash contents from a file
When developing software that has to be flashed to a flash chip to be executed, it often takes a long time to read the current flash contents (for flashrom to know what pages to erase and reprogram) each time when writing the new image. However, when the flash was just reprogrammed, its current state is known to be the previous image that was flashed (assuming it was verified).
Thus, it makes sense to provide that image as a file for the flash contents instead of wasting valuable time read the whole flash each time.
Original patch has been created by Paul Kocialkowski, the previous version: http://patchwork.coreboot.org/patch/4414/ Now it has been ported to the latest source code of flashrom master branch
Change-Id: Idf153b6955f37779ae9bfb228a434ed10c304947 Signed-off-by: Mike Banon mikebdp2@gmail.com --- M cli_classic.c M flash.h M flashrom.c M libflashrom.h 4 files changed, 82 insertions(+), 50 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/23263/1
diff --git a/cli_classic.c b/cli_classic.c index 441fc91..c210e03 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -45,26 +45,27 @@ "[-E|(-r|-w|-v) <file>] [(-l <layoutfile>|--ifd) [-i <imagename>]...] [-n] [-N] [-f]]\n" "[-V[V[V]]] [-o <logfile>]\n\n", name);
- printf(" -h | --help print this help text\n" - " -R | --version print version (release)\n" - " -r | --read <file> read flash and save to <file>\n" - " -w | --write <file> write <file> to flash\n" - " -v | --verify <file> verify flash against <file>\n" - " -E | --erase erase flash memory\n" - " -V | --verbose more verbose output\n" - " -c | --chip <chipname> probe only for specified flash chip\n" - " -f | --force force specific operations (see man page)\n" - " -n | --noverify don't auto-verify\n" - " -N | --noverify-all verify included regions only (cf. -i)\n" - " -l | --layout <layoutfile> read ROM layout from <layoutfile>\n" - " --ifd read layout from an Intel Firmware Descriptor\n" - " -i | --image <name> only flash image <name> from flash layout\n" - " -o | --output <logfile> log output to <logfile>\n" - " -L | --list-supported print supported devices\n" + printf(" -h | --help print this help text\n" + " -R | --version print version (release)\n" + " -r | --read <file> read flash and save to <file>\n" + " -w | --write <file> write <file> to flash\n" + " -v | --verify <file> verify flash against <file>\n" + " -E | --erase erase flash memory\n" + " -V | --verbose more verbose output\n" + " -c | --chip <chipname> probe only for specified flash chip\n" + " -f | --force force specific operations (see man page)\n" + " -n | --noverify don't auto-verify\n" + " -N | --noverify-all verify included regions only (cf. -i)\n" + " -l | --layout <layoutfile> read ROM layout from <layoutfile>\n" + " --ifd read layout from an Intel Firmware Descriptor\n" + " -i | --image <name> only flash image <name> from flash layout\n" + " -o | --output <logfile> log output to <logfile>\n" + " -C | --flash-contents <contentsfile> assume flash contents to be <contentsfile>\n" + " -L | --list-supported print supported devices\n" #if CONFIG_PRINT_WIKI == 1 - " -z | --list-supported-wiki print supported devices in wiki syntax\n" + " -z | --list-supported-wiki print supported devices in wiki syntax\n" #endif - " -p | --programmer <name>[:<param>] specify the programmer device. One of\n"); + " -p | --programmer <name>[:<param>] specify the programmer device. One of\n"); list_programmers_linebreak(4, 80, 0); printf(".\n\nYou can specify one of -h, -R, -L, " #if CONFIG_PRINT_WIKI == 1 @@ -110,7 +111,7 @@ enum programmer prog = PROGRAMMER_INVALID; int ret = 0;
- static const char optstring[] = "r:Rw:v:nNVEfc:l:i:p:Lzho:"; + static const char optstring[] = "r:Rw:v:nNVEfc:l:i:C:p:Lzho:"; static const struct option long_options[] = { {"read", 1, NULL, 'r'}, {"write", 1, NULL, 'w'}, @@ -124,6 +125,7 @@ {"layout", 1, NULL, 'l'}, {"ifd", 0, NULL, 0x0100}, {"image", 1, NULL, 'i'}, + {"flash-contents", 1, NULL, 'C'}, {"list-supported", 0, NULL, 'L'}, {"list-supported-wiki", 0, NULL, 'z'}, {"programmer", 1, NULL, 'p'}, @@ -134,6 +136,7 @@ };
char *filename = NULL; + char *contentsfile = NULL; char *layoutfile = NULL; #ifndef STANDALONE char *logfile = NULL; @@ -243,6 +246,9 @@ cli_classic_abort_usage(); } break; + case 'C': + contentsfile = strdup(optarg); + break; case 'L': if (++operation_specified > 1) { fprintf(stderr, "More than one operation " @@ -354,6 +360,9 @@ if (layoutfile && check_filename(layoutfile, "layout")) { cli_classic_abort_usage(); } + if (contentsfile && check_filename(contentsfile, "contents")) { + cli_classic_abort_usage(); + }
#ifndef STANDALONE if (logfile && check_filename(logfile, "log")) @@ -569,7 +578,7 @@ else if (erase_it) ret = do_erase(fill_flash); else if (write_it) - ret = do_write(fill_flash, filename); + ret = do_write(fill_flash, filename, contentsfile); else if (verify_it) ret = do_verify(fill_flash, filename);
@@ -583,6 +592,7 @@
layout_cleanup(); free(filename); + free(contentsfile); free(layoutfile); free(pparam); /* clean up global variables */ diff --git a/flash.h b/flash.h index 1c4a38c..c1c827e 100644 --- a/flash.h +++ b/flash.h @@ -325,7 +325,7 @@ void finalize_flash_access(struct flashctx *); int do_read(struct flashctx *, const char *filename); int do_erase(struct flashctx *); -int do_write(struct flashctx *, const char *const filename); +int do_write(struct flashctx *, const char *const filename, const char *const contentsfile); int do_verify(struct flashctx *, const char *const filename);
/* Something happened that shouldn't happen, but we can go on. */ diff --git a/flashrom.c b/flashrom.c index f699798..1190fda 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2352,7 +2352,7 @@ * 2 if write failed and flash contents changed, * or 1 on any other failure. */ -int flashrom_image_write(struct flashctx *const flashctx, void *const buffer, const size_t buffer_len) +int flashrom_image_write(struct flashctx *const flashctx, void *const buffer, const size_t buffer_len, const char *contentsfile) { const size_t flash_size = flashctx->chip->total_size * 1024; const bool verify_all = flashctx->flags.verify_whole_chip; @@ -2389,42 +2389,64 @@ goto _free_ret;
/* - * 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 is to read only the regions which are to be - * preserved, but in that case we might perform unneeded erase which - * takes time as well. + * When developing software that has to be flashed to a flash chip to be executed, + * it often takes a long time to read the current flash contents (for flashrom to + * know what pages to erase and reprogram) each time, when writing the new image. + * However, when the flash was just reprogrammed, its current state is known to be + * the previous image that was flashed (assuming it was verified). + * + * Thus, it makes sense to provide that image as a file for the flash contents + * instead of wasting valuable time read the whole flash each time. */ - msg_cinfo("Reading old flash chip contents... "); - if (verify_all) { - if (flashctx->chip->read(flashctx, oldcontents, 0, flash_size)) { + + if (contentsfile) { + msg_cinfo("Reading old flash chip contents from file... "); + if (read_buf_from_file(oldcontents, flash_size, contentsfile)) { + ret = 1; msg_cinfo("FAILED.\n"); goto _finalize_ret; } - memcpy(curcontents, oldcontents, flash_size); } else { - if (read_by_layout(flashctx, curcontents)) { - msg_cinfo("FAILED.\n"); - goto _finalize_ret; + + /* + * 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 is 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 (verify_all) { + if (flashctx->chip->read(flashctx, oldcontents, 0, flash_size)) { + msg_cinfo("FAILED.\n"); + goto _finalize_ret; + } + memcpy(curcontents, oldcontents, flash_size); + } else { + if (read_by_layout(flashctx, curcontents)) { + msg_cinfo("FAILED.\n"); + goto _finalize_ret; + } } + msg_cinfo("done.\n"); } - msg_cinfo("done.\n");
if (write_by_layout(flashctx, curcontents, newcontents)) { msg_cerr("Uh oh. Erase/write failed. "); ret = 2; - if (verify_all) { - msg_cerr("Checking if anything has changed.\n"); - msg_cinfo("Reading current flash chip contents... "); - if (!flashctx->chip->read(flashctx, curcontents, 0, flash_size)) { - msg_cinfo("done.\n"); - if (!memcmp(oldcontents, curcontents, flash_size)) { - nonfatal_help_message(); - goto _finalize_ret; - } - msg_cerr("Apparently at least some data has changed.\n"); - } else - msg_cerr("Can't even read anymore!\n"); + if (verify_all) { + msg_cerr("Checking if anything has changed.\n"); + msg_cinfo("Reading current flash chip contents... "); + if (!flashctx->chip->read(flashctx, curcontents, 0, flash_size)) { + msg_cinfo("done.\n"); + if (!memcmp(oldcontents, curcontents, flash_size)) { + nonfatal_help_message(); + goto _finalize_ret; + } + msg_cerr("Apparently at least some data has changed.\n"); + } else + msg_cerr("Can't even read anymore!\n"); emergency_help_message(); goto _finalize_ret; } else { @@ -2543,7 +2565,7 @@ return ret; }
-int do_write(struct flashctx *const flash, const char *const filename) +int do_write(struct flashctx *const flash, const char *const filename, const char *contentsfile) { const size_t flash_size = flash->chip->total_size * 1024; int ret = 1; @@ -2557,7 +2579,7 @@ if (read_buf_from_file(newcontents, flash_size, filename)) goto _free_ret;
- ret = flashrom_image_write(flash, newcontents, flash_size); + ret = flashrom_image_write(flash, newcontents, flash_size, contentsfile);
_free_ret: free(newcontents); diff --git a/libflashrom.h b/libflashrom.h index d3f3ded..7ebac8d 100644 --- a/libflashrom.h +++ b/libflashrom.h @@ -60,7 +60,7 @@ bool flashrom_flag_get(const struct flashrom_flashctx *, enum flashrom_flag);
int flashrom_image_read(struct flashrom_flashctx *, void *buffer, size_t buffer_len); -int flashrom_image_write(struct flashrom_flashctx *, void *buffer, size_t buffer_len); +int flashrom_image_write(struct flashrom_flashctx *, void *buffer, size_t buffer_len, const char *contentsfile); int flashrom_image_verify(struct flashrom_flashctx *, const void *buffer, size_t buffer_len);
struct flashrom_layout;
-
Mike Banon (Code Review)