Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/23021 )
Change subject: layout: Add -i <region>[:<file>] support ......................................................................
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(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Sam McNally: Looks good to me, approved
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__ */