vboot_reference: lib/flashrom convert args into struct
This attempts to converge the signatures of host/lib/flashrom
with that of 'futility/updater_utils.h:struct firmware_image'.
with the eventual goal of converging the multiple flashrom
wrapper implementations.
BUG=b:207808292
BRANCH=none
TEST=`$ cros_run_unit_tests --board nocturne --packages vboot_reference`
Cq-Depend: chromium:3399963
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Change-Id: I016dacbdca6f1108def0dbc608d83e0066a30023
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/3301571
Reviewed-by: Sam McNally <sammc@chromium.org>
Tested-by: Edward O'Callaghan <quasisec@chromium.org>
Auto-Submit: Edward O'Callaghan <quasisec@chromium.org>
Commit-Queue: Edward O'Callaghan <quasisec@chromium.org>
diff --git a/host/lib/crossystem.c b/host/lib/crossystem.c
index 7c3ee87..a5427c2 100644
--- a/host/lib/crossystem.c
+++ b/host/lib/crossystem.c
@@ -821,21 +821,21 @@
{
int index;
int vbnv_size = vb2_nv_get_size(ctx);
- uint8_t *flash_buf;
- uint32_t flash_size;
- if (flashrom_read(FLASHROM_PROGRAMMER_INTERNAL_AP, VBNV_FMAP_REGION,
- &flash_buf, &flash_size))
+ struct firmware_image image = {
+ .programmer = FLASHROM_PROGRAMMER_INTERNAL_AP,
+ };
+ if (flashrom_read(&image, VBNV_FMAP_REGION))
return -1;
- index = vb2_nv_index(flash_buf, flash_size, vbnv_size);
+ index = vb2_nv_index(image.data, image.size, vbnv_size);
if (index < 0) {
- free(flash_buf);
+ free(image.data);
return -1;
}
- memcpy(ctx->nvdata, &flash_buf[index * vbnv_size], vbnv_size);
- free(flash_buf);
+ memcpy(ctx->nvdata, &image.data[index * vbnv_size], vbnv_size);
+ free(image.data);
return 0;
}
@@ -845,34 +845,33 @@
int current_index;
int next_index;
int vbnv_size = vb2_nv_get_size(ctx);
- uint8_t *flash_buf;
- uint32_t flash_size;
- if (flashrom_read(FLASHROM_PROGRAMMER_INTERNAL_AP, VBNV_FMAP_REGION,
- &flash_buf, &flash_size))
+ struct firmware_image image = {
+ .programmer = FLASHROM_PROGRAMMER_INTERNAL_AP,
+ };
+ if (flashrom_read(&image, VBNV_FMAP_REGION))
return -1;
- current_index = vb2_nv_index(flash_buf, flash_size, vbnv_size);
+ current_index = vb2_nv_index(image.data, image.size, vbnv_size);
if (current_index < 0) {
rv = -1;
goto exit;
}
next_index = current_index + 1;
- if (next_index * vbnv_size == flash_size) {
+ if (next_index * vbnv_size == image.size) {
/* VBNV is full. Erase and write at beginning. */
- memset(flash_buf, 0xff, flash_size);
+ memset(image.data, 0xff, image.size);
next_index = 0;
}
- memcpy(&flash_buf[next_index * vbnv_size], ctx->nvdata, vbnv_size);
- if (flashrom_write(FLASHROM_PROGRAMMER_INTERNAL_AP, VBNV_FMAP_REGION,
- flash_buf, flash_size)) {
+ memcpy(&image.data[next_index * vbnv_size], ctx->nvdata, vbnv_size);
+ if (flashrom_write(&image, VBNV_FMAP_REGION)) {
rv = -1;
goto exit;
}
exit:
- free(flash_buf);
+ free(image.data);
return rv;
}
diff --git a/host/lib/flashrom.c b/host/lib/flashrom.c
index e83dfb2..6ee5971 100644
--- a/host/lib/flashrom.c
+++ b/host/lib/flashrom.c
@@ -103,15 +103,14 @@
return VB2_SUCCESS;
}
-vb2_error_t flashrom_read(const char *programmer, const char *region,
- uint8_t **data_out, uint32_t *size_out)
+vb2_error_t flashrom_read(struct firmware_image *image, const char *region)
{
char *tmpfile;
char region_param[PATH_MAX];
vb2_error_t rv;
- *data_out = NULL;
- *size_out = 0;
+ image->data = NULL;
+ image->size = 0;
VB2_TRY(write_temp_file(NULL, 0, &tmpfile));
@@ -122,7 +121,7 @@
const char *const argv[] = {
FLASHROM_EXEC_NAME,
"-p",
- programmer,
+ image->programmer,
"-r",
region ? "-i" : tmpfile,
region ? region_param : NULL,
@@ -131,21 +130,20 @@
rv = run_flashrom(argv);
if (rv == VB2_SUCCESS)
- rv = vb2_read_file(tmpfile, data_out, size_out);
+ rv = vb2_read_file(tmpfile, &image->data, &image->size);
unlink(tmpfile);
free(tmpfile);
return rv;
}
-vb2_error_t flashrom_write(const char *programmer, const char *region,
- uint8_t *data, uint32_t size)
+vb2_error_t flashrom_write(struct firmware_image *image, const char *region)
{
char *tmpfile;
char region_param[PATH_MAX];
vb2_error_t rv;
- VB2_TRY(write_temp_file(data, size, &tmpfile));
+ VB2_TRY(write_temp_file(image->data, image->size, &tmpfile));
if (region)
snprintf(region_param, sizeof(region_param), "%s:%s", region,
@@ -154,7 +152,7 @@
const char *const argv[] = {
FLASHROM_EXEC_NAME,
"-p",
- programmer,
+ image->programmer,
"--noverify-all",
"-w",
region ? "-i" : tmpfile,
diff --git a/host/lib/include/flashrom.h b/host/lib/include/flashrom.h
index 560fbb0..0d93442 100644
--- a/host/lib/include/flashrom.h
+++ b/host/lib/include/flashrom.h
@@ -12,39 +12,39 @@
#define FLASHROM_PROGRAMMER_INTERNAL_AP "host"
#define FLASHROM_PROGRAMMER_INTERNAL_EC "ec"
+/* Utilities for firmware images and (FMAP) sections */
+struct firmware_image {
+ /**
+ * programmer The name of the programmer to use. Use either
+ * FLASHROM_PROGRAMMER_INTERNAL_AP or,
+ * FLASHROM_PROGRAMMER_INTERNAL_EC
+ * for the AP and EC respectively.
+ */
+ const char *programmer;
+ uint32_t size; /* buffer size. */
+ uint8_t *data; /* data allocated buffer to read/write with. */
+};
+
/**
* Read using flashrom into an allocated buffer.
*
- * @param programmer The name of the programmer to use. There are
- * named constants FLASHROM_PROGRAMMER_INTERNAL_AP
- * and FLASHROM_PROGRAMMER_INTERNAL_EC available
- * for the AP and EC respectively, or a custom
- * programmer string can be provided.
+ * @param image The parameter that contains the programmer, buffer and
+ * size to use in the read operation.
* @param region The name of the fmap region to read, or NULL to
* read the entire flash chip.
- * @param data_out Output parameter of allocated buffer to read into.
- * The caller should free the buffer.
- * @param size_out Output parameter of buffer size.
*
* @return VB2_SUCCESS on success, or a relevant error.
*/
-vb2_error_t flashrom_read(const char *programmer, const char *region,
- uint8_t **data_out, uint32_t *size_out);
+vb2_error_t flashrom_read(struct firmware_image *image, const char *region);
/**
* Write using flashrom from a buffer.
*
- * @param programmer The name of the programmer to use. There are
- * named constants FLASHROM_PROGRAMMER_INTERNAL_AP
- * and FLASHROM_PROGRAMMER_INTERNAL_EC available
- * for the AP and EC respectively, or a custom
- * programmer string can be provided.
+ * @param image The parameter that contains the programmer, buffer and
+ * size to use in the write operation.
* @param region The name of the fmap region to write, or NULL to
* write the entire flash chip.
- * @param data The buffer to write.
- * @param size The size of the buffer to write.
*
* @return VB2_SUCCESS on success, or a relevant error.
*/
-vb2_error_t flashrom_write(const char *programmer, const char *region,
- uint8_t *data, uint32_t size);
+vb2_error_t flashrom_write(struct firmware_image *image, const char *region);
diff --git a/tests/vb2_host_flashrom_tests.c b/tests/vb2_host_flashrom_tests.c
index 8552d0a..815657d 100644
--- a/tests/vb2_host_flashrom_tests.c
+++ b/tests/vb2_host_flashrom_tests.c
@@ -145,10 +145,11 @@
static void test_read_whole_chip(void)
{
- uint8_t *buf;
- uint32_t buf_sz;
+ struct firmware_image image = {
+ .programmer = "someprog",
+ };
- TEST_SUCC(flashrom_read("someprog", NULL, &buf, &buf_sz),
+ TEST_SUCC(flashrom_read(&image, NULL),
"Flashrom read succeeds");
TEST_STR_EQ(captured_programmer, "someprog",
"Using specified programmer");
@@ -158,19 +159,20 @@
TEST_STR_EQ(captured_op_filename, MOCK_TMPFILE_NAME,
"Reading to correct file");
TEST_PTR_EQ(captured_region_param, NULL, "Not operating on a region");
- TEST_EQ(buf_sz, strlen(MOCK_ROM_CONTENTS), "Contents correct size");
- TEST_SUCC(memcmp(buf, MOCK_ROM_CONTENTS, buf_sz),
+ TEST_EQ(image.size, strlen(MOCK_ROM_CONTENTS), "Contents correct size");
+ TEST_SUCC(memcmp(image.data, MOCK_ROM_CONTENTS, image.size),
"Buffer has correct contents");
- free(buf);
+ free(image.data);
}
static void test_read_region(void)
{
- uint8_t *buf;
- uint32_t buf_sz;
+ struct firmware_image image = {
+ .programmer = "someprog",
+ };
- TEST_SUCC(flashrom_read("someprog", "SOME_REGION", &buf, &buf_sz),
+ TEST_SUCC(flashrom_read(&image, "SOME_REGION"),
"Flashrom read succeeds");
TEST_STR_EQ(captured_programmer, "someprog",
"Using specified programmer");
@@ -181,20 +183,21 @@
"Not doing a read of the whole ROM");
TEST_STR_EQ(captured_region_param, "SOME_REGION:" MOCK_TMPFILE_NAME,
"Reading to correct file and from correct region");
- TEST_EQ(buf_sz, strlen(MOCK_ROM_CONTENTS), "Contents correct size");
- TEST_SUCC(memcmp(buf, MOCK_ROM_CONTENTS, buf_sz),
+ TEST_EQ(image.size, strlen(MOCK_ROM_CONTENTS), "Contents correct size");
+ TEST_SUCC(memcmp(image.data, MOCK_ROM_CONTENTS, image.size),
"Buffer has correct contents");
- free(buf);
+ free(image.data);
}
static void test_read_failure(void)
{
- uint8_t *buf;
- uint32_t buf_sz;
+ struct firmware_image image = {
+ .programmer = "someprog",
+ };
flashrom_mock_success = false;
- TEST_NEQ(flashrom_read("someprog", "SOME_REGION", &buf, &buf_sz),
+ TEST_NEQ(flashrom_read(&image, "SOME_REGION"),
VB2_SUCCESS, "Flashrom read fails");
flashrom_mock_success = true;
}
@@ -202,10 +205,15 @@
static void test_write_whole_chip(void)
{
uint8_t buf[sizeof(MOCK_ROM_CONTENTS) - 1];
+ struct firmware_image image = {
+ .programmer = "someprog",
+ .data = buf,
+ .size = sizeof(buf),
+ };
memcpy(buf, MOCK_ROM_CONTENTS, sizeof(buf));
- TEST_SUCC(flashrom_write("someprog", NULL, buf, sizeof(buf)),
+ TEST_SUCC(flashrom_write(&image, NULL),
"Flashrom write succeeds");
TEST_STR_EQ(captured_programmer, "someprog",
"Using specified programmer");
@@ -224,10 +232,15 @@
static void test_write_region(void)
{
uint8_t buf[sizeof(MOCK_ROM_CONTENTS) - 1];
+ struct firmware_image image = {
+ .programmer = "someprog",
+ .data = buf,
+ .size = sizeof(buf),
+ };
memcpy(buf, MOCK_ROM_CONTENTS, sizeof(buf));
- TEST_SUCC(flashrom_write("someprog", "SOME_REGION", buf, sizeof(buf)),
+ TEST_SUCC(flashrom_write(&image, "SOME_REGION"),
"Flashrom write succeeds");
TEST_STR_EQ(captured_programmer, "someprog",
"Using specified programmer");
@@ -247,9 +260,14 @@
static void test_write_failure(void)
{
uint8_t buf[20] = { 0 };
+ struct firmware_image image = {
+ .programmer = "someprog",
+ .data = buf,
+ .size = sizeof(buf),
+ };
flashrom_mock_success = false;
- TEST_NEQ(flashrom_write("someprog", "SOME_REGION", buf, sizeof(buf)),
+ TEST_NEQ(flashrom_write(&image, "SOME_REGION"),
VB2_SUCCESS, "Flashrom write fails");
flashrom_mock_success = true;
}
diff --git a/tests/vb2_host_nvdata_flashrom_tests.c b/tests/vb2_host_nvdata_flashrom_tests.c
index 33b435d..068b23a 100644
--- a/tests/vb2_host_nvdata_flashrom_tests.c
+++ b/tests/vb2_host_nvdata_flashrom_tests.c
@@ -81,35 +81,33 @@
}
/* Mocked flashrom_read for tests. */
-vb2_error_t flashrom_read(const char *programmer, const char *region,
- uint8_t **data_out, uint32_t *size_out)
+vb2_error_t flashrom_read(struct firmware_image *image, const char *region)
{
if (mock_flashrom_fail) {
- *data_out = NULL;
- *size_out = 0;
+ image->data = NULL;
+ image->size = 0;
return VB2_ERROR_FLASHROM;
}
- assert_mock_params(programmer, region);
+ assert_mock_params(image->programmer, region);
- *data_out = malloc(sizeof(fake_flash_region));
- *size_out = sizeof(fake_flash_region);
- memcpy(*data_out, fake_flash_region, sizeof(fake_flash_region));
+ image->data = malloc(sizeof(fake_flash_region));
+ image->size = sizeof(fake_flash_region);
+ memcpy(image->data, fake_flash_region, sizeof(fake_flash_region));
return VB2_SUCCESS;
}
/* Mocked flashrom_write for tests. */
-vb2_error_t flashrom_write(const char *programmer, const char *region,
- uint8_t *data, uint32_t data_size)
+vb2_error_t flashrom_write(struct firmware_image *image, const char *region)
{
if (mock_flashrom_fail)
return VB2_ERROR_FLASHROM;
- assert_mock_params(programmer, region);
+ assert_mock_params(image->programmer, region);
- TEST_EQ(data_size, sizeof(fake_flash_region),
+ TEST_EQ(image->size, sizeof(fake_flash_region),
"The flash size is correct");
- memcpy(fake_flash_region, data, data_size);
+ memcpy(fake_flash_region, image->data, image->size);
return VB2_SUCCESS;
}
