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>
diff --git a/futility/updater.c b/futility/updater.c
index c1ac58a..ae51534 100644
--- a/futility/updater.c
+++ b/futility/updater.c
@@ -323,132 +323,6 @@
}
/*
- * 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 @@
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 @@
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 @@
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 @@
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 079f16d..82813ca 100644
--- a/futility/updater_utils.c
+++ b/futility/updater_utils.c
@@ -524,25 +524,102 @@
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 @@
* 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;
+ 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;
- 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);
- }
+ if (cfg->emulation)
+ return emulate_write_firmware(cfg->emulation, image, sections);
- 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);
+ if (cfg->use_diff_image && cfg->image_current.data &&
+ is_the_same_programmer(&cfg->image_current, image))
+ flash_contents = &cfg->image_current;
- for (i = 1, r = -1; i <= retries && r != 0; i++) {
+ 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 3eb1d33..0376eb3 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 @@
/* 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 @@
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 @@
* 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;
