Change in flashrom[master]: Add support for reading the current flash contents from a file

[Nico Huber (Code Review)]

Nico Huber has submitted this change and it was merged. ( https://review.coreboot.org/23263 )

Change subject: Add support for reading the current flash contents from a file
......................................................................

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.

Change-Id: Idf153b6955f37779ae9bfb228a434ed10c304947
Signed-off-by: Mike Banon 
Signed-off-by: Paul Kocialkowski 
Reviewed-on: https://review.coreboot.org/23263
Reviewed-by: Nico Huber 
Tested-by: build bot (Jenkins) 
---
M cli_classic.c
M flash.h
M flashrom.c
M libflashrom.h
4 files changed, 61 insertions(+), 28 deletions(-)

Approvals:
  Nico Huber: Looks good to me, approved
  build bot (Jenkins): Verified
  Paul Kocialkowski: Looks good to me, approved

diff --git a/cli_classic.c b/cli_classic.c
index 441fc91..31f7394 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -60,6 +60,7 @@
           "      --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"
+	       "      --flash-contents <ref-file>   assume flash contents to be <ref-file>\n"
           " -L | --list-supported              print supported devices\n"
 #if CONFIG_PRINT_WIKI == 1
           " -z | --list-supported-wiki         print supported devices in wiki syntax\n"
@@ -108,6 +109,10 @@
    int dont_verify_it = 0, dont_verify_all = 0, list_supported = 0, operation_specified = 0;
    struct flashrom_layout *layout = NULL;
    enum programmer prog = PROGRAMMER_INVALID;
+	enum {
+		OPTION_IFD = 0x0100,
+		OPTION_FLASH_CONTENTS,
+	};
    int ret = 0;

static const char optstring[] = "r:Rw:v:nNVEfc:l:i:p:Lzho:";
@@ -122,8 +127,9 @@
    	{"verbose",		0, NULL, 'V'},
    	{"force",		0, NULL, 'f'},
    	{"layout",		1, NULL, 'l'},
-		{"ifd",			0, NULL, 0x0100},
+		{"ifd",			0, NULL, OPTION_IFD},
    	{"image",		1, NULL, 'i'},
+		{"flash-contents",	1, NULL, OPTION_FLASH_CONTENTS},
    	{"list-supported",	0, NULL, 'L'},
    	{"list-supported-wiki",	0, NULL, 'z'},
    	{"programmer",		1, NULL, 'p'},
@@ -134,6 +140,7 @@
    };

char *filename = NULL;
+	char *referencefile = NULL;
    char *layoutfile = NULL;
 #ifndef STANDALONE
    char *logfile = NULL;
@@ -229,7 +236,7 @@
    		}
    		layoutfile = strdup(optarg);
    		break;
-		case 0x0100:
+		case OPTION_IFD:
    		if (layoutfile) {
    			fprintf(stderr, "Error: --layout and --ifd both specified. Aborting.\n");
    			cli_classic_abort_usage();
@@ -243,6 +250,9 @@
    			cli_classic_abort_usage();
    		}
    		break;
+		case OPTION_FLASH_CONTENTS:
+			referencefile = strdup(optarg);
+			break;
    	case 'L':
    		if (++operation_specified > 1) {
    			fprintf(stderr, "More than one operation "
@@ -354,6 +364,9 @@
    if (layoutfile && check_filename(layoutfile, "layout")) {
    	cli_classic_abort_usage();
    }
+	if (referencefile && check_filename(referencefile, "reference")) {
+		cli_classic_abort_usage();
+	}

#ifndef STANDALONE
    if (logfile && check_filename(logfile, "log"))
@@ -569,7 +582,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, referencefile);
    else if (verify_it)
    	ret = do_verify(fill_flash, filename);

@@ -583,6 +596,7 @@

layout_cleanup();
    free(filename);
+	free(referencefile);
    free(layoutfile);
    free(pparam);
    /* clean up global variables */
diff --git a/flash.h b/flash.h
index 7bf3cc7..a80a9c2 100644
--- a/flash.h
+++ b/flash.h
@@ -329,7 +329,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 referencefile);
 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 26b9863..d87a431 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -2345,13 +2345,15 @@
  * @param flashctx The context of the flash chip.
  * @param buffer Source buffer to read image from (may be altered for full verification).
  * @param buffer_len Size of source buffer in bytes.
+ * @param refbuffer If given, assume flash chip contains same data as `refbuffer`.
  * @return 0 on success,
  *         4 if buffer_len doesn't match the size of the flash chip,
  *         3 if write was tried but nothing has changed,
  *         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 void *const refbuffer)
 {
    const size_t flash_size = flashctx->chip->total_size * 1024;
    const bool verify_all = flashctx->flags.verify_whole_chip;
@@ -2363,6 +2365,7 @@
    int ret = 1;

uint8_t *const newcontents = buffer;
+	const uint8_t *const refcontents = refbuffer;
    uint8_t *const curcontents = malloc(flash_size);
    uint8_t *oldcontents = NULL;
    if (verify_all)
@@ -2387,27 +2390,35 @@
    if (prepare_flash_access(flashctx, false, true, false, verify))
    	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.
-	 */
-	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);
+	/* If given, assume flash chip contains same data as `refcontents`. */
+	if (refcontents) {
+		msg_cinfo("Assuming old flash chip contents as ref-file...\n");
+		memcpy(curcontents, refcontents, flash_size);
+		if (oldcontents)
+			memcpy(oldcontents, refcontents, 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. ");
@@ -2542,13 +2553,15 @@
    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 *const referencefile)
 {
    const size_t flash_size = flash->chip->total_size * 1024;
    int ret = 1;

uint8_t *const newcontents = malloc(flash_size);
-	if (!newcontents) {
+	uint8_t *const refcontents = referencefile ? malloc(flash_size) : NULL;
+
+	if (!newcontents || (referencefile && !refcontents)) {
    	msg_gerr("Out of memory!\n");
    	goto _free_ret;
    }
@@ -2556,9 +2569,15 @@
    if (read_buf_from_file(newcontents, flash_size, filename))
    	goto _free_ret;

-	ret = flashrom_image_write(flash, newcontents, flash_size);
+	if (referencefile) {
+		if (read_buf_from_file(refcontents, flash_size, referencefile))
+			goto _free_ret;
+	}
+
+	ret = flashrom_image_write(flash, newcontents, flash_size, refcontents);

_free_ret:
+	free(refcontents);
    free(newcontents);
    return ret;
 }
diff --git a/libflashrom.h b/libflashrom.h
index d3f3ded..786147b 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 void *refbuffer);
 int flashrom_image_verify(struct flashrom_flashctx *, const void *buffer, size_t buffer_len);

struct flashrom_layout;
@@ -69,4 +69,4 @@
 void flashrom_layout_release(struct flashrom_layout *);
 void flashrom_layout_set(struct flashrom_flashctx *, const struct flashrom_layout *);

-#endif				/* !__LIBFLASHROM_H__ */
+#endif				/* !__LIBFLASHROM_H__ */
\ No newline at end of file