Make read before write configurable (infrastructure part)

- Introduce a variable in doit() that allows to influence
   read-before-write and its consequences.
 - Modify build_new_image so that it still works even if the old content
   is not read before.
 - Add copy_old_content() to ease the pain for future patches.

Corresponding to flashrom svn r1851.

Signed-off-by: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Acked-by: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>

flash.h

@@ -341,7 +341,7 @@ int register_include_arg(char *name);
 int process_include_args(void);
 int read_romlayout(const char *name);
 int normalize_romentries(const struct flashctx *flash);
-int build_new_image(const struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents);
+int build_new_image(struct flashctx *flash, bool oldcontents_valid, uint8_t *oldcontents, uint8_t *newcontents);
 void layout_cleanup(void);
 
 /* spi.c */

flashrom.c

@@ -1906,6 +1906,7 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
 	uint8_t *newcontents;
 	int ret = 0;
 	unsigned long size = flash->chip->total_size * 1024;
+	int read_all_first = 1; /* FIXME: Make this configurable. */
 
 	if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) {
 		msg_cerr("Aborting.\n");
@@ -1983,26 +1984,33 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
 
 	/* 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
+	 * 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 (read_all_first) {
-	if (flash->chip->read(flash, oldcontents, 0, size)) {
+		msg_cinfo("Reading old flash chip contents... ");
-		ret = 1;
+		if (flash->chip->read(flash, oldcontents, 0, size)) {
-		msg_cinfo("FAILED.\n");
+			ret = 1;
-		goto out;
+			msg_cinfo("FAILED.\n");
+			goto out;
+		}
 	}
 	msg_cinfo("done.\n");
 
 	/* Build a new image taking the given layout into account. */
-	build_new_image(flash, oldcontents, newcontents);
+	if (build_new_image(flash, read_all_first, oldcontents, newcontents)) {
+		msg_gerr("Could not prepare the data to be written, aborting.\n");
+		ret = 1;
+		goto out;
+	}
 
 	// ////////////////////////////////////////////////////////////
 
-	if (write_it) {
+	if (write_it && erase_and_write_flash(flash, oldcontents, newcontents)) {
-		if (erase_and_write_flash(flash, oldcontents, newcontents)) {
+		msg_cerr("Uh oh. Erase/write failed.");
-			msg_cerr("Uh oh. Erase/write failed. Checking if anything has changed.\n");
+		if (read_all_first) {
+			msg_cerr("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");
@@ -2017,7 +2025,11 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
 			emergency_help_message();
 			ret = 1;
 			goto out;
-		}
+		} else
+			msg_cerr("\n");
+		emergency_help_message();
+		ret = 1;
+		goto out;
 	}
 
 	/* Verify only if we either did not try to write (verify operation) or actually changed something. */

layout.c

@@ -257,7 +257,28 @@ int normalize_romentries(const struct flashctx *flash)
 	return ret;
 }
 
-int build_new_image(const struct flashctx *flash, uint8_t *oldcontents, uint8_t *newcontents)
+static int copy_old_content(struct flashctx *flash, int oldcontents_valid, uint8_t *oldcontents, uint8_t *newcontents, unsigned int start, unsigned int size)
+{
+	if (!oldcontents_valid) {
+		/* oldcontents is a zero-filled buffer. By reading the current data into oldcontents here, we
+		 * avoid a rewrite of identical regions even if an initial full chip read didn't happen. */
+		msg_gdbg2("Read a chunk starting at 0x%06x (len=0x%06x).\n", start, size);
+		int ret = flash->chip->read(flash, oldcontents + start, start, size);
+		if (ret != 0) {
+			msg_gerr("Failed to read chunk 0x%06x-0x%06x.\n", start, start + size - 1);
+			return 1;
+		}
+	}
+	memcpy(newcontents + start, oldcontents + start, size);
+	return 0;
+}
+
+/**
+ * Modify @newcontents so that it contains the data that should be on the chip eventually. In the case the user
+ * wants to update only parts of it, copy the chunks to be preserved from @oldcontents to @newcontents. If
+ * @oldcontents is not valid, we need to fetch the current data from the chip first.
+ */
+int build_new_image(struct flashctx *flash, bool oldcontents_valid, uint8_t *oldcontents, uint8_t *newcontents)
 {
 	unsigned int start = 0;
 	romentry_t *entry;
@@ -276,14 +297,14 @@ int build_new_image(const struct flashctx *flash, uint8_t *oldcontents, uint8_t
 		entry = get_next_included_romentry(start);
 		/* No more romentries for remaining region? */
 		if (!entry) {
-			memcpy(newcontents + start, oldcontents + start,
+			copy_old_content(flash, oldcontents_valid, oldcontents, newcontents, start,
-			       size - start);
+					 size - start);
 			break;
 		}
 		/* For non-included region, copy from old content. */
 		if (entry->start > start)
-			memcpy(newcontents + start, oldcontents + start,
+			copy_old_content(flash, oldcontents_valid, oldcontents, newcontents, start,
-			       entry->start - start);
+					 entry->start - start);
 		/* Skip to location after current romentry. */
 		start = entry->end + 1;
 		/* Catch overflow. */