futility: updater: refactor {write,load}_system_firmware

The 'write_system_firmware' and 'load_system_firmware' have too many
parameters that are already contained in the updater_config.
To simplify the interfaces, we should just pass the updater_config to
{write,load}_system_firmware, and merge the write_firmware_sections
(only handling emulation and deciding the diff image) to the
write_system_firmware.

Also moved the utility functions only used by *_system_firmware (for
example is_the_same_programmer and emulate_write_firmware) to the
updater_utils.c.

The emulate_write_firmware is also revised to handle a list of sections
directly (so we don't need to handle that in write_system_firmware).

BUG=None
TEST=make; build and run test.
BRANCH=None

Change-Id: I4d123d12e8057da82d6c301899472e3773f3266e
Signed-off-by: Hung-Te Lin <hungte@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3508121
Reviewed-by: Yu-Ping Wu <yupingso@chromium.org>
Commit-Queue: Yu-Ping Wu <yupingso@chromium.org>

3 files changed, 154 insertions, 187 deletions

diff --git a/futility/updater.c b/futility/updater.c
index c1ac58a5..ae51534c 100644
--- a/futility/updater.c
+++ b/futility/updater.c
@@ -323,132 +323,6 @@ static int set_try_cookies(struct updater_config *cfg, const char *target,
 }
 
 /*
- * Emulates writing to firmware.
- * Returns 0 if success, non-zero if error.
- */
-static int emulate_write_firmware(const char *filename,
-				  const struct firmware_image *image,
-				  const char *section_name)
-{
-	struct firmware_image to_image = {0};
-	struct firmware_section from, to;
-	int errorcnt = 0;
-
-	INFO("(emulation) Writing %s from %s to %s (emu=%s).\n",
-	     section_name ? section_name : "the whole image",
-	     image->file_name, image->programmer, filename);
-
-	from.data = image->data;
-	from.size = image->size;
-
-	if (load_firmware_image(&to_image, filename, NULL)) {
-		ERROR("Cannot load image from %s.\n", filename);
-		return -1;
-	}
-
-	if (section_name) {
-		find_firmware_section(&from, image, section_name);
-		if (!from.data) {
-			ERROR("No section %s in source image %s.\n",
-			      section_name, image->file_name);
-			errorcnt++;
-		}
-		find_firmware_section(&to, &to_image, section_name);
-		if (!to.data) {
-			ERROR("No section %s in destination image %s.\n",
-			      section_name, filename);
-			errorcnt++;
-		}
-	} else if (image->size != to_image.size) {
-		ERROR("Image size is different (%s:%d != %s:%d)\n",
-		      image->file_name, image->size, to_image.file_name,
-		      to_image.size);
-		errorcnt++;
-	} else {
-		to.data = to_image.data;
-		to.size = to_image.size;
-	}
-
-	if (!errorcnt) {
-		size_t to_write = VB2_MIN(to.size, from.size);
-
-		assert(from.data && to.data);
-		VB2_DEBUG("Writing %zu bytes\n", to_write);
-		memcpy(to.data, from.data, to_write);
-	}
-
-	if (!errorcnt && vb2_write_file(
-			filename, to_image.data, to_image.size)) {
-		ERROR("Failed writing to file: %s\n", filename);
-		errorcnt++;
-	}
-
-	free_firmware_image(&to_image);
-	return errorcnt;
-}
-
-/*
- * Returns the number of retries when reading or writing to flash.
- */
-static int get_io_retries(struct updater_config *cfg)
-{
-	return 1 + get_config_quirk(QUIRK_EXTRA_RETRIES, cfg);
-}
-
-/*
- * Returns 1 if the programmers in image1 and image2 are the same.
- */
-static int is_the_same_programmer(const struct firmware_image *image1,
-				  const struct firmware_image *image2)
-{
-	assert(image1 && image2);
-
-	/* Including if both are NULL. */
-	if (image1->programmer == image2->programmer)
-		return 1;
-
-	/* Not the same if either one is NULL. */
-	if (!image1->programmer || !image2->programmer)
-		return 0;
-
-	return strcmp(image1->programmer, image2->programmer) == 0;
-}
-
-/*
- * Writes multiple sections from the given firmware image to the system.
- * The 'sections' should be NULL (write the whole image) or a list of section
- * names to write, and ended with NULL.
- * Returns 0 if success, non-zero if error.
- */
-static int write_firmware_sections(struct updater_config *cfg,
-				   const struct firmware_image *image,
-				   const char * const sections[])
-{
-	int r = 0;
-	struct firmware_image *diff_image = NULL;
-
-	if (cfg->emulation) {
-		int i;
-		if (!sections)
-			return emulate_write_firmware(
-					cfg->emulation, image, NULL);
-		for (i = 0; sections[i] && !r; i++) {
-			r |= emulate_write_firmware(
-					cfg->emulation, image, sections[i]);
-		}
-		return r;
-	}
-
-	if (cfg->use_diff_image && cfg->image_current.data &&
-	    is_the_same_programmer(&cfg->image_current, image))
-		diff_image = &cfg->image_current;
-
-	return write_system_firmware(image, diff_image, sections,
-				     &cfg->tempfiles, cfg->do_verify,
-				     get_io_retries(cfg), cfg->verbosity + 1);
-}
-
-/*
  * Writes a single section from the given firmware image to the system.
  * Writes the whole firmware image if the section_name is NULL.
  * Returns 0 if success, non-zero if error.
@@ -460,8 +334,8 @@ static int write_firmware(struct updater_config *cfg,
 	const char *sections[2] = {0};
 
 	sections[0] = section_name;
-	return write_firmware_sections(cfg, image,
-				       section_name ? sections : NULL);
+	return write_system_firmware(
+			cfg, image, section_name ? sections : NULL);
 }
 
 /*
@@ -1188,7 +1062,7 @@ static enum updater_error_codes update_rw_firmware(
 	assert(num < ARRAY_SIZE(sections));
 	sections[num] = NULL;
 
-	if (write_firmware_sections(cfg, image_to, sections))
+	if (write_system_firmware(cfg, image_to, sections))
 		return UPDATE_ERR_WRITE_FIRMWARE;
 
 	return UPDATE_ERR_DONE;
@@ -1295,8 +1169,7 @@ enum updater_error_codes update_firmware(struct updater_config *cfg)
 		int ret;
 
 		INFO("Loading current system firmware...\n");
-		ret = load_system_firmware(image_from, &cfg->tempfiles,
-					   get_io_retries(cfg), cfg->verbosity);
+		ret = load_system_firmware(cfg, image_from);
 		if (ret == IMAGE_PARSE_FAILURE && cfg->force_update) {
 			WARN("No compatible firmware in system.\n");
 			cfg->check_platform = 0;
@@ -1478,10 +1351,7 @@ static int updater_apply_custom_label(struct updater_config *cfg,
 	if (!signature_id) {
 		if (!cfg->image_current.data) {
 			INFO("Loading system firmware for custom label...\n");
-			load_system_firmware(&cfg->image_current,
-					     &cfg->tempfiles,
-					     get_io_retries(cfg),
-					     cfg->verbosity);
+			load_system_firmware(cfg, &cfg->image_current);
 		}
 		tmp_image = get_firmware_image_temp_file(
 				&cfg->image_current, &cfg->tempfiles);
diff --git a/futility/updater_utils.c b/futility/updater_utils.c
index 079f16d4..82813ca7 100644
--- a/futility/updater_utils.c
+++ b/futility/updater_utils.c
@@ -524,25 +524,102 @@ char *host_detect_servo(int *need_prepare_ptr)
 
 	return ret;
 }
+/*
+ * Returns 1 if the programmers in image1 and image2 are the same.
+ */
+static int is_the_same_programmer(const struct firmware_image *image1,
+				  const struct firmware_image *image2)
+{
+	assert(image1 && image2);
+
+	/* Including if both are NULL. */
+	if (image1->programmer == image2->programmer)
+		return 1;
+
+	/* Not the same if either one is NULL. */
+	if (!image1->programmer || !image2->programmer)
+		return 0;
+
+	return strcmp(image1->programmer, image2->programmer) == 0;
+}
+
+/*
+ * Emulates writing a firmware image to the system.
+ * Returns 0 if success, non-zero if error.
+ */
+static int emulate_write_firmware(const char *filename,
+				  const struct firmware_image *image,
+				  const char * const sections[])
+{
+	int i, errorcnt = 0;
+	struct firmware_image to_image = {0};
+
+	INFO("Writing from %s to %s (emu=%s).\n",
+	     image->file_name, image->programmer, filename);
+
+	if (load_firmware_image(&to_image, filename, NULL)) {
+		ERROR("Cannot load image from %s.\n", filename);
+		return -1;
+	}
+
+	if (image->size != to_image.size) {
+		ERROR("Image size is different (%s:%d != %s:%d)\n",
+		      image->file_name, image->size, to_image.file_name,
+		      to_image.size);
+		errorcnt++;
+		goto exit;
+	}
+
+	if (!sections) {
+		VB2_DEBUG(" - write the whole image.\n");
+		memmove(to_image.data, image->data, image->size);
+	}
+	for (i = 0; sections && sections[i]; i++) {
+		VB2_DEBUG(" - write the section: %s.\n", sections[i]);
+		if (preserve_firmware_section(image, &to_image, sections[i])) {
+			ERROR("Failed to write the section: %s\n", sections[i]);
+			errorcnt++;
+			/*
+			 * Exit the loop, but still write the file to reflect
+			 * the partial changes - same as real flashrom behavior.
+			 */
+			break;
+		}
+	}
+
+	if (vb2_write_file(filename, to_image.data, to_image.size)) {
+		ERROR("Failed writing to file: %s\n", filename);
+		errorcnt++;
+		goto exit;
+	}
+
+exit:
+	free_firmware_image(&to_image);
+	return errorcnt;
+}
 
 /*
  * Loads the active system firmware image (usually from SPI flash chip).
  * Returns 0 if success, non-zero if error.
  */
-int load_system_firmware(struct firmware_image *image,
-			 struct tempfile *tempfiles,
-			 int retries, int verbosity)
+int load_system_firmware(struct updater_config *cfg,
+			 struct firmware_image *image)
 {
 	int r, i;
+	const int tries = 1 + get_config_quirk(QUIRK_EXTRA_RETRIES, cfg);
+	struct flashrom_params params = {0};
+
+	params.image = image;
+	params.verbose = cfg->verbosity + 1; /* libflashrom verbose 1 = WARN. */
 
 	INFO("flashrom -r <IMAGE> -p %s%s\n",
-	     image->programmer,
-	     verbosity ? " -V" : "");
+	     params.image->programmer,
+	     params.verbose > 1 ? " -V" : "");
 
-	for (i = 1, r = -1; i <= retries && r != 0; i++) {
+	for (i = 1, r = -1; i <= tries && r != 0; i++, params.verbose++) {
 		if (i > 1)
-			WARN("Retry reading firmware (%d/%d)...\n", i, retries);
-		r = flashrom_read_image(image, NULL, verbosity + i);
+			WARN("Retry reading firmware (%d/%d)...\n", i, tries);
+		r = flashrom_read_image(params.image, NULL, params.verbose);
 	}
 	if (!r)
 		r = parse_firmware_image(image);
@@ -555,44 +632,46 @@ int load_system_firmware(struct firmware_image *image,
  * FMAP section names (and ended with a NULL).
  * Returns 0 if success, non-zero if error.
  */
-int write_system_firmware(const struct firmware_image *image,
-			  const struct firmware_image *diff_image,
-			  const char * const sections[],
-			  struct tempfile *tempfiles,
-			  int do_verify, int retries, int verbosity)
+int write_system_firmware(struct updater_config *cfg,
+			  const struct firmware_image *image,
+			  const char * const sections[])
 {
-	int r, i, len = 0;
-	char *partial = NULL;
-
-	for (i = 0; sections && sections[i]; i++)
-		len += strlen(sections[i]) + strlen(" -i ");
-	if (len) {
-		partial = (char *)malloc(len + 1);
-		if (!partial) {
-			ERROR("Failed to allocate a string buffer.\n");
-			return -1;
-		}
-		partial[0] = '\0';
-		for (i = 0; sections[i]; i++) {
-			strcat(partial, " -i ");
-			strcat(partial, sections[i]);
-		}
-		assert(strlen(partial) == len);
-	}
-
-	INFO("flashrom -w <IMAGE> -p %s%s%s%s%s\n",
-	     image->programmer,
-	     diff_image ? " --flash-contents <DIFF_IMAGE>" : "",
-	     do_verify ? "" : " --noverify",
-	     verbosity > 1 ? " -V" : "",
-	     partial ? partial : "");
-	free(partial);
-
-	for (i = 1, r = -1; i <= retries && r != 0; i++) {
+	int r = 0, i;
+	const int tries = 1 + get_config_quirk(QUIRK_EXTRA_RETRIES, cfg);
+	struct flashrom_params params = {0};
+	struct firmware_image *flash_contents = NULL;
+
+	if (cfg->emulation)
+		return emulate_write_firmware(cfg->emulation, image, sections);
+
+	if (cfg->use_diff_image && cfg->image_current.data &&
+	    is_the_same_programmer(&cfg->image_current, image))
+		flash_contents = &cfg->image_current;
+
+	params.image = (struct firmware_image *)image;
+	params.flash_contents = flash_contents;
+	params.regions = sections;
+	params.noverify = !cfg->do_verify;
+	params.noverify_all = true;
+	params.verbose = cfg->verbosity + 1; /* libflashrom verbose 1 = WARN. */
+
+	INFO("flashrom -w <IMAGE> -p %s%s%s%s%s%s%s\n",
+	     params.image->programmer,
+	     params.flash_contents ? " --flash-contents" : "",
+	     params.flash_contents ? "<OLD-IMAGE>" : "",
+	     params.regions ? " -i <REGIONS>" : "",
+	     params.noverify ? " --noverify" : "",
+	     params.noverify_all ? " --noverify-all" : "",
+	     params.verbose > 1 ? " -V" : "");
+
+	for (i = 1, r = -1; i <= tries && r != 0; i++, params.verbose++) {
 		if (i > 1)
-			WARN("Retry writing firmware (%d/%d)...\n", i, retries);
-		r = flashrom_write_image(image, sections, diff_image, do_verify,
-					 verbosity + i);
+			WARN("Retry writing firmware (%d/%d)...\n", i, tries);
+		r = flashrom_write_image(params.image,
+					 params.regions,
+					 params.flash_contents,
+					 !params.noverify,
+					 params.verbose);
 		/*
 		 * Force a newline to flush stdout in case if
 		 * flashrom_write_image left some messages in the buffer.
diff --git a/futility/updater_utils.h b/futility/updater_utils.h
index 3eb1d33c..0376eb36 100644
--- a/futility/updater_utils.h
+++ b/futility/updater_utils.h
@@ -8,6 +8,7 @@
 #ifndef VBOOT_REFERENCE_FUTILITY_UPDATER_UTILS_H_
 #define VBOOT_REFERENCE_FUTILITY_UPDATER_UTILS_H_
 
+#include <stdbool.h>
 #include <stdio.h>
 #include "fmap.h"
 
@@ -57,6 +58,23 @@ void remove_all_temp_files(struct tempfile *head);
 /* Include definition of 'struct firmware_image;' */
 #include "flashrom.h"
 
+/* Parameters when invoking flashrom. */
+struct flashrom_params {
+	struct firmware_image *image; /* The firmware image to read/write. */
+	const struct firmware_image *flash_contents; /* --flash-contents */
+	const char *const *regions; /* -i: only read/write <region> */
+	bool noverify; /* -n: don't auto-verify */
+	bool noverify_all; /* -N: verify included regions only */
+	int verbose; /* -V: more verbose output */
+	/* Supported by libflashrom but no exported by flashrom_drv:
+	 *  - force
+	 *  - noverify_all
+	 * Not supported by libflashrom:
+	 *  - do_not_diff
+	 *  - ignore_lock
+	 */
+};
+
 enum {
 	IMAGE_LOAD_SUCCESS = 0,
 	IMAGE_READ_FAILURE = -1,
@@ -73,13 +91,15 @@ enum {
 int load_firmware_image(struct firmware_image *image, const char *file_name,
 			struct archive *archive);
 
+/* Structure(s) declared in updater.h */
+struct updater_config;
+
 /*
  * Loads the active system firmware image (usually from SPI flash chip).
  * Returns 0 if success, non-zero if error.
  */
-int load_system_firmware(struct firmware_image *image,
-			 struct tempfile *tempfiles,
-			 int retries, int verbosity);
+int load_system_firmware(struct updater_config *cfg,
+			 struct firmware_image *image);
 
 /* Frees the allocated resource from a firmware image object. */
 void free_firmware_image(struct firmware_image *image);
@@ -98,11 +118,9 @@ const char *get_firmware_image_temp_file(const struct firmware_image *image,
  * FMAP section names (and ended with a NULL).
  * Returns 0 if success, non-zero if error.
  */
-int write_system_firmware(const struct firmware_image *image,
-			  const struct firmware_image *diff_image,
-			  const char * const sections[],
-			  struct tempfile *tempfiles,
-			  int do_verify, int retries, int verbosity);
+int write_system_firmware(struct updater_config *cfg,
+			  const struct firmware_image *image,
+			  const char * const sections[]);
 
 struct firmware_section {
 	uint8_t *data;