Edward O'Callaghan submitted this change.
layout: Add -i <region>[:<file>] support
Add an optional sub-parameter to the -i parameter to allow building the
image to be written from multiple files. This will also allow regions to
be read from flash and written to separate image files.
This is a rebase of a patch that was ported from chromiumos. A lot of
things have changed, but the idea is the same.
Original patch by Louis Yung-Chieh Lo <yjlou@chromium.org>:
Summary: Support -i partition:file feature for both read and write.
Commit: 9c7525f
Review URL: http://codereview.chromium.org/6611015
Ported version by Stefan Tauner <stefan.tauner@student.tuwien.ac.at>
and Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>:
Summary: [PATCH 2/6] layout: Add -i <region>[:<file>] support.
Review URL: https://mail.coreboot.org/pipermail/flashrom/2013-October/011729.html
Change-Id: Ic5465659605d8431d931053967b40290195cfd99
Signed-off-by: David Hendricks <dhendrix@chromium.org>
Signed-off-by: Stefan Tauner <stefan.tauner@student.tuwien.ac.at>
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
Signed-off-by: Arthur Heymans <arthur@aheymans.xyz>
Signed-off-by: Nico Huber <nico.huber@secunet.com>
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Signed-off-by: Daniel Campello <campello@chromium.org>
Co-Authored-by: Edward O'Callaghan <quasisec@google.com>
Co-Authored-by: Daniel Campello <campello@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/23021
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Sam McNally <sammc@google.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
---
M cli_classic.c
M flash.h
M flashrom.8.tmpl
M flashrom.c
M layout.c
M layout.h
6 files changed, 234 insertions(+), 37 deletions(-)
diff --git a/cli_classic.c b/cli_classic.c
index 3160d76..a22d642 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -41,7 +41,7 @@
"\n\t-p <programmername>[:<parameters>] [-c <chipname>]\n"
"\t\t(--flash-name|--flash-size|\n"
"\t\t [-E|(-r|-w|-v) <file>]\n"
- "\t\t [(-l <layoutfile>|--ifd| --fmap|--fmap-file <file>) [-i <imagename>]...]\n"
+ "\t\t [(-l <layoutfile>|--ifd| --fmap|--fmap-file <file>) [-i <region>[:<file>]]...]\n"
"\t\t [-n] [-N] [-f])]\n"
"\t[-V[V[V]]] [-o <logfile>]\n\n", name);
@@ -67,7 +67,8 @@
" --fmap read ROM layout from fmap embedded in ROM\n"
" --fmap-file <fmapfile> read ROM layout from fmap in <fmapfile>\n"
" --ifd read layout from an Intel Firmware Descriptor\n"
- " -i | --image <name> only flash image <name> from flash layout\n"
+ " -i | --image <region>[:<file>] only read/write image <region> from layout\n"
+ " (optionally with data from <file>)\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"
@@ -337,11 +338,8 @@
fmap = 1;
break;
case 'i':
- tempstr = strdup(optarg);
- if (register_include_arg(&include_args, tempstr)) {
- free(tempstr);
+ if (register_include_arg(&include_args, optarg))
cli_classic_abort_usage(NULL);
- }
break;
case OPTION_FLASH_CONTENTS:
if (referencefile)
diff --git a/flash.h b/flash.h
index 883b6f4..c0e5c27 100644
--- a/flash.h
+++ b/flash.h
@@ -417,7 +417,7 @@
#define msg_cspew(...) print(FLASHROM_MSG_SPEW, __VA_ARGS__) /* chip debug spew */
/* layout.c */
-int register_include_arg(struct layout_include_args **args, char *name);
+int register_include_arg(struct layout_include_args **args, const char *arg);
int read_romlayout(const char *name);
int normalize_romentries(const struct flashctx *flash);
void layout_cleanup(struct layout_include_args **args);
diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl
index 02f60dc..4634539 100644
--- a/flashrom.8.tmpl
+++ b/flashrom.8.tmpl
@@ -48,7 +48,8 @@
\fB\-p\fR <programmername>[:<parameters>] [\fB\-c\fR <chipname>]
(\fB\-\-flash\-name\fR|\fB\-\-flash\-size\fR|
[\fB\-E\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>]
- [(\fB\-l\fR <file>|\fB\-\-ifd|\fB \-\-fmap\fR|\fB\-\-fmap-file\fR <file>) [\fB\-i\fR <image>]]
+ [(\fB\-l\fR <file>|\fB\-\-ifd|\fB \-\-fmap\fR|\fB\-\-fmap-file\fR <file>)
+ [\fB\-i\fR <image>[:<file>]]]
[\fB\-n\fR] [\fB\-N\fR] [\fB\-f\fR])]
[\fB\-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>]
.SH DESCRIPTION
@@ -239,10 +240,45 @@
gbe gigabit ethernet firmware
pd platform specific data
.TP
-.B "\-i, \-\-image <imagename>"
-Only flash region/image
-.B <imagename>
-from flash layout.
+.B "\-i, \-\-include <region>[:<file>]"
+Read or write only
+.B <region>
+to or from ROM.
+The
+.B "\-i"
+option may be used multiple times if the user wishes to read or write
+multiple regions using a single command.
+.sp
+The user may optionally specify a corresponding
+.B <file>
+for any region they wish to read or write. A read operation will read the
+corresponding regions from ROM and write individual files for each one. A write
+option will read file(s) and write to the corresponding region(s) in ROM.
+.sp
+For write operations, files specified using
+.B "\-i"
+take precedence over content from the argument to
+.B "\-w."
+.sp
+Examples:
+.sp
+ To read regions named
+.BR "foo " and " bar"
+in layout file
+.B <layout>
+into region-sized files
+.BR "foo.bin " and " bar.bin" ", run:
+.sp
+.B " flashrom \-p prog \-l <layout> \-i foo:foo.bin -i bar:bar.bin -r rom.bin
+.sp
+ To write files
+.BR "foo.bin " and " bar.bin"
+into regions named
+.BR "foo " and " bar" " in layout file
+.BR <layout>
+to the ROM, run:
+.sp
+.B " flashrom \-p prog \-l <layout> \-i foo:foo.bin -i bar:bar.bin -w rom.bin
.TP
.B "\-\-flash\-name"
Prints out the detected flash chips name.
diff --git a/flashrom.c b/flashrom.c
index 056fc57..3a733df 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1360,7 +1360,7 @@
goto out;
}
if (image_stat.st_size != (intmax_t)size) {
- msg_gerr("Error: Image size (%jd B) doesn't match the flash chip's size (%lu B)!\n",
+ msg_gerr("Error: Image size (%jd B) doesn't match the expected size (%lu B)!\n",
(intmax_t)image_stat.st_size, size);
ret = 1;
goto out;
@@ -1378,6 +1378,51 @@
#endif
}
+/**
+ * @brief Reads content to buffer from one or more files.
+ *
+ * Reads content to supplied buffer from files. If a filename is specified for
+ * individual regions using the partial read syntax ('-i <region>[:<filename>]')
+ * then this will read file data into the corresponding region in the
+ * supplied buffer.
+ *
+ * @param flashctx Flash context to be used.
+ * @param buf Chip-sized buffer to write data to
+ * @return 0 on success
+ */
+static int read_buf_from_include_args(const struct flashctx *const flash,
+ unsigned char *buf)
+{
+ const struct flashrom_layout *const layout = get_layout(flash);
+ const struct romentry *entry = NULL;
+
+ /*
+ * Content will be read from -i args, so they must not overlap since
+ * we need to know exactly what content to write to the ROM.
+ */
+ if (included_regions_overlap(layout)) {
+ msg_gerr("Error: Included regions must not overlap when writing.\n");
+ return 1;
+ }
+
+ while ((entry = layout_next_included(layout, entry))) {
+ if (!entry->file)
+ continue;
+ if (read_buf_from_file(buf + entry->start,
+ entry->end - entry->start + 1, entry->file))
+ return 1;
+ }
+ return 0;
+}
+
+/**
+ * @brief Writes passed data buffer into a file
+ *
+ * @param buf Buffer with data to write
+ * @param size Size of buffer
+ * @param filename File path to write to
+ * @return 0 on success
+ */
int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename)
{
#ifdef __LIBPAYLOAD__
@@ -1430,6 +1475,35 @@
#endif
}
+/**
+ * @brief Writes content from buffer to one or more files.
+ *
+ * Writes content from supplied buffer to files. If a filename is specified for
+ * individual regions using the partial read syntax ('-i <region>[:<filename>]')
+ * then this will write files using data from the corresponding region in the
+ * supplied buffer.
+ *
+ * @param flashctx Flash context to be used.
+ * @param buf Chip-sized buffer to read data from
+ * @return 0 on success
+ */
+static int write_buf_to_include_args(const struct flashctx *const flash,
+ unsigned char *buf)
+{
+ const struct flashrom_layout *const layout = get_layout(flash);
+ const struct romentry *entry = NULL;
+
+ while ((entry = layout_next_included(layout, entry))) {
+ if (!entry->file)
+ continue;
+ if (write_buf_to_file(buf + entry->start,
+ entry->end - entry->start + 1, entry->file))
+ return 1;
+ }
+
+ return 0;
+}
+
static int read_by_layout(struct flashctx *, uint8_t *);
int read_flash_to_file(struct flashctx *flash, const char *filename)
{
@@ -1453,6 +1527,10 @@
ret = 1;
goto out_free;
}
+ if (write_buf_to_include_args(flash, buf)) {
+ ret = 1;
+ goto out_free;
+ }
ret = write_buf_to_file(buf, size, filename);
out_free:
@@ -2643,8 +2721,15 @@
goto _free_ret;
}
+ /* Read '-w' argument first... */
if (read_buf_from_file(newcontents, flash_size, filename))
goto _free_ret;
+ /*
+ * ... then update newcontents with contents from files provided to '-i'
+ * args if needed.
+ */
+ if (read_buf_from_include_args(flash, newcontents))
+ goto _free_ret;
if (referencefile) {
if (read_buf_from_file(refcontents, flash_size, referencefile))
@@ -2670,8 +2755,15 @@
goto _free_ret;
}
+ /* Read '-v' argument first... */
if (read_buf_from_file(newcontents, flash_size, filename))
goto _free_ret;
+ /*
+ * ... then update newcontents with contents from files provided to '-i'
+ * args if needed.
+ */
+ if (read_buf_from_include_args(flash, newcontents))
+ goto _free_ret;
ret = flashrom_image_verify(flash, newcontents, flash_size);
diff --git a/layout.c b/layout.c
index 272912c..d0d2607 100644
--- a/layout.c
+++ b/layout.c
@@ -52,7 +52,7 @@
romlayout = fopen(name, "r");
if (!romlayout) {
- msg_gerr("ERROR: Could not open ROM layout (%s).\n",
+ msg_gerr("ERROR: Could not open layout file (%s).\n",
name);
return -1;
}
@@ -82,6 +82,7 @@
layout->entries[layout->num_entries].start = strtol(tstr1, (char **)NULL, 16);
layout->entries[layout->num_entries].end = strtol(tstr2, (char **)NULL, 16);
layout->entries[layout->num_entries].included = false;
+ layout->entries[layout->num_entries].file = NULL;
layout->entries[layout->num_entries].name = strdup(tempname);
if (!layout->entries[layout->num_entries].name) {
msg_gerr("Error adding layout entry: %s\n", strerror(errno));
@@ -105,44 +106,76 @@
#endif
/* register an include argument (-i) for later processing */
-int register_include_arg(struct layout_include_args **args, char *name)
+int register_include_arg(struct layout_include_args **args, const char *arg)
{
struct layout_include_args *tmp;
- if (name == NULL) {
+ char *colon;
+ char *name;
+ char *file;
+
+ if (arg == NULL) {
msg_gerr("<NULL> is a bad region name.\n");
return 1;
}
- tmp = *args;
- while (tmp) {
+ /* -i <image>[:<file>] */
+ colon = strchr(arg, ':');
+ if (colon && !colon[1]) {
+ msg_gerr("Missing filename parameter in %s\n", arg);
+ return 1;
+ }
+ name = colon ? strndup(arg, colon - arg) : strdup(arg);
+ file = colon ? strdup(colon + 1) : NULL;
+
+ for (tmp = *args; tmp; tmp = tmp->next) {
if (!strcmp(tmp->name, name)) {
msg_gerr("Duplicate region name: \"%s\".\n", name);
- return 1;
+ goto error;
}
- tmp = tmp->next;
}
tmp = malloc(sizeof(struct layout_include_args));
if (tmp == NULL) {
msg_gerr("Could not allocate memory");
- return 1;
+ goto error;
}
tmp->name = name;
+ tmp->file = file;
tmp->next = *args;
*args = tmp;
-
return 0;
+
+error:
+ free(name);
+ free(file);
+ return 1;
+}
+
+/* returns 0 to indicate success, 1 to indicate failure */
+static int include_region(struct flashrom_layout *const l, const char *name,
+ const char *file)
+{
+ size_t i;
+ for (i = 0; i < l->num_entries; ++i) {
+ if (!strcmp(l->entries[i].name, name)) {
+ l->entries[i].included = true;
+ if (file)
+ l->entries[i].file = strdup(file);
+ return 0;
+ }
+ }
+ return 1;
}
/* returns -1 if an entry is not found, 0 if found. */
-static int find_romentry(struct flashrom_layout *const l, char *name)
+static int find_romentry(struct flashrom_layout *const l, char *name, char *file)
{
if (l->num_entries == 0)
return -1;
msg_gspew("Looking for region \"%s\"... ", name);
- if (flashrom_layout_include_region(l, name)) {
+ if (include_region(l, name, file)) {
msg_gspew("not found.\n");
return -1;
}
@@ -161,7 +194,7 @@
if (args == NULL)
return 0;
- /* User has specified an area, but no layout file is loaded. */
+ /* User has specified an include argument, but no layout is loaded. */
if (l->num_entries == 0) {
msg_gerr("Region requested (with -i \"%s\"), "
"but no layout data is available.\n",
@@ -171,7 +204,7 @@
tmp = args;
while (tmp) {
- if (find_romentry(l, tmp->name) < 0) {
+ if (find_romentry(l, tmp->name, tmp->file) < 0) {
msg_gerr("Invalid region specified: \"%s\".\n",
tmp->name);
return 1;
@@ -180,10 +213,14 @@
found++;
}
- msg_ginfo("Using region%s:", found > 1 ? "s" : "");
+ msg_ginfo("Using region%s: ", found > 1 ? "s" : "");
tmp = args;
while (tmp) {
- msg_ginfo(" \"%s\"%s", tmp->name, found > 1 ? "," : "");
+ msg_ginfo("\"%s\"", tmp->name);
+ if (tmp->file)
+ msg_ginfo(":\"%s\"", tmp->file);
+ if (found > 1)
+ msg_ginfo(", ");
found--;
tmp = tmp->next;
}
@@ -191,6 +228,39 @@
return 0;
}
+/* returns boolean 1 if any regions overlap, 0 otherwise */
+int included_regions_overlap(const struct flashrom_layout *const l)
+{
+ size_t i;
+ int overlap_detected = 0;
+
+ for (i = 0; i < l->num_entries; i++) {
+ size_t j;
+
+ if (!l->entries[i].included)
+ continue;
+
+ for (j = i + 1; j < l->num_entries; j++) {
+ if (!l->entries[j].included)
+ continue;
+
+ if (l->entries[i].start > l->entries[j].end)
+ continue;
+
+ if (l->entries[i].end < l->entries[j].start)
+ continue;
+
+ msg_gdbg("Regions %s [0x%08x-0x%08x] and "
+ "%s [0x%08x-0x%08x] overlap\n",
+ l->entries[i].name, l->entries[i].start,
+ l->entries[i].end, l->entries[j].name,
+ l->entries[j].start, l->entries[j].end);
+ overlap_detected = 1;
+ }
+ }
+ return overlap_detected;
+}
+
void layout_cleanup(struct layout_include_args **args)
{
struct flashrom_layout *const layout = get_global_layout();
@@ -199,12 +269,15 @@
while (*args) {
tmp = (*args)->next;
+ free((*args)->name);
+ free((*args)->file);
free(*args);
*args = tmp;
}
for (i = 0; i < layout->num_entries; i++) {
free(layout->entries[i].name);
+ free(layout->entries[i].file);
layout->entries[i].included = false;
}
layout->num_entries = 0;
@@ -287,14 +360,7 @@
*/
int flashrom_layout_include_region(struct flashrom_layout *const layout, const char *name)
{
- size_t i;
- for (i = 0; i < layout->num_entries; ++i) {
- if (!strcmp(layout->entries[i].name, name)) {
- layout->entries[i].included = true;
- return 0;
- }
- }
- return 1;
+ return include_region(layout, name, NULL);
}
/**
@@ -309,8 +375,10 @@
if (!layout || layout == get_global_layout())
return;
- for (i = 0; i < layout->num_entries; ++i)
+ for (i = 0; i < layout->num_entries; ++i) {
free(layout->entries[i].name);
+ free(layout->entries[i].file);
+ }
free(layout);
}
diff --git a/layout.h b/layout.h
index cceedd1..b11c432 100644
--- a/layout.h
+++ b/layout.h
@@ -40,6 +40,7 @@
chipoff_t end;
bool included;
char *name;
+ char *file;
};
struct flashrom_layout {
@@ -56,6 +57,7 @@
struct layout_include_args {
char *name;
+ char *file;
struct layout_include_args *next;
};
@@ -66,5 +68,6 @@
int process_include_args(struct flashrom_layout *l, const struct layout_include_args *const args);
const struct romentry *layout_next_included_region(const struct flashrom_layout *, chipoff_t);
const struct romentry *layout_next_included(const struct flashrom_layout *, const struct romentry *);
+int included_regions_overlap(const struct flashrom_layout *const flashrom_layout);
#endif /* !__LAYOUT_H__ */
To view, visit change 23021. To unsubscribe, or for help writing mail filters, visit settings.