David Hendricks would like Carl-Daniel Hailfinger to review this change.

View 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>
---
M cli_classic.c
M dummyflasher.c
M flash.h
M flashrom.8.tmpl
M flashrom.c
M layout.c
M layout.h
7 files changed, 181 insertions(+), 38 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/23021/1
diff --git a/cli_classic.c b/cli_classic.c
index 441fc91..c1481ec 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -58,7 +58,8 @@
" -N | --noverify-all verify included regions only (cf. -i)\n"
" -l | --layout <layoutfile> read ROM layout from <layoutfile>\n"
" --ifd read layout from an Intel Firmware Descriptor\n"
- " -i | --image <name> only flash image <name> from flash layout\n"
+ " -i | --include <region>[<:file>] only flash image <region> from layout\n"
+ " (optionally with data from <file>)\n"
" -o | --output <logfile> log output to <logfile>\n"
" -L | --list-supported print supported devices\n"
#if CONFIG_PRINT_WIKI == 1
diff --git a/dummyflasher.c b/dummyflasher.c
index f171128..1fa67d8 100644
--- a/dummyflasher.c
+++ b/dummyflasher.c
@@ -139,8 +139,10 @@
#if EMULATE_CHIP
if (emu_chip != EMULATE_NONE) {
if (emu_persistent_image) {
+ struct flashctx flash = { 0 };
+
msg_pdbg("Writing %s\n", emu_persistent_image);
- write_buf_to_file(flashchip_contents, emu_chip_size, emu_persistent_image);
+ write_buf_to_file(&flash, flashchip_contents, emu_persistent_image);
free(emu_persistent_image);
emu_persistent_image = NULL;
}
@@ -382,7 +384,7 @@
msg_pdbg("matches.\n");
msg_pdbg("Reading %s\n", emu_persistent_image);
read_buf_from_file(flashchip_contents, emu_chip_size,
- emu_persistent_image);
+ emu_persistent_image, NULL);
} else {
msg_pdbg("doesn't match.\n");
}
diff --git a/flash.h b/flash.h
index 62f2e34..b8344fc 100644
--- a/flash.h
+++ b/flash.h
@@ -296,8 +296,8 @@
void print_banner(void);
void list_programmers_linebreak(int startcol, int cols, int paren);
int selfcheck(void);
-int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename);
-int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename);
+int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename, const char *size_msg);
+int write_buf_to_file(struct flashctx *const flash, const unsigned char *buf, const char *filename);
int prepare_flash_access(struct flashctx *, bool read_it, bool write_it, bool erase_it, bool verify_it);
void finalize_flash_access(struct flashctx *);
int do_read(struct flashctx *, const char *filename);
diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl
index 143d76c..6a05e8a 100644
--- a/flashrom.8.tmpl
+++ b/flashrom.8.tmpl
@@ -48,7 +48,7 @@
\fB\-p\fR <programmername>[:<parameters>]
[\fB\-E\fR|\fB\-r\fR <file>|\fB\-w\fR <file>|\fB\-v\fR <file>] \
[\fB\-c\fR <chipname>]
- [(\fB\-l\fR <file>|\fB\-\-ifd\fR) [\fB\-i\fR <image>]] \
+ [(\fB\-l\fR <file>|\fB\-\-ifd\fR) [\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
@@ -211,10 +211,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
+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 "\-L, \-\-list\-supported"
List the flash chips, chipsets, mainboards, and external programmers
diff --git a/flashrom.c b/flashrom.c
index 12d7390..4f50c54 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1315,8 +1315,7 @@
return chip - flashchips;
}

-int read_buf_from_file(unsigned char *buf, unsigned long size,
- const char *filename)
+int read_buf_from_file(unsigned char *buf, unsigned long size, const char *filename, const char *size_msg)
{
#ifdef __LIBPAYLOAD__
msg_gerr("Error: No file I/O support in libpayload\n");
@@ -1337,8 +1336,10 @@
goto out;
}
if (image_stat.st_size != size) {
- msg_gerr("Error: Image size (%jd B) doesn't match the flash chip's size (%lu B)!\n",
- (intmax_t)image_stat.st_size, size);
+ if (size_msg == NULL)
+ size_msg = "the expected size";
+ msg_gerr("Error: Image size (%jd B) doesn't match %s (%lu B)!\n",
+ (intmax_t)image_stat.st_size, size_msg, size);
ret = 1;
goto out;
}
@@ -1355,12 +1356,21 @@
#endif
}

-int write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename)
+/**
+ * @brief Writes included layout regions into buffer(s)
+ *
+ * If partial read to file is to be done (-i <region>[:<regionfile>] syntax
+ * used) then this will write to corresponding region-sized files. If an
+ * argument to -r/-w/-v is specified then it will write a chip-sized file. Both
+ * partial read/writes and full read/write may be done at once.
+ *
+ * @param buf Buffer with data to write
+ * @param size Size of buffer
+ * @param filename Filename corresponding to optional argument to -r/-w/-v
+ * @return 0 on success
+ */
+static int do_write_buf_to_file(const unsigned char *buf, unsigned long size, const char *filename)
{
-#ifdef __LIBPAYLOAD__
- msg_gerr("Error: No file I/O support in libpayload\n");
- return 1;
-#else
FILE *image;
int ret = 0;

@@ -1404,9 +1414,59 @@
ret = 1;
}
return ret;
+}
+
+/**
+ * @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. If a filename is specified for -r/-w/-e options then a chip-
+ * sized file will be created.
+ *
+ * @param flashctx Flash context to be used.
+ * @param buf Chip-sized buffer to read data from
+ * @param filename Optional filename corresponding to -r/-w/-v argument
+ * @return 0 on success
+ */
+int write_buf_to_file(struct flashctx *const flash, const unsigned char *buf, const char *filename)
+{
+#ifdef __LIBPAYLOAD__
+ msg_gerr("Error: No file I/O support in libpayload\n");
+ return 1;
+#else
+ int ret = 0;
+ const struct flashrom_layout *const layout = get_layout(flash);
+
+ size_t i;
+ for (i = 0; i < layout->num_entries; i++) {
+ if (!layout->entries[i].included)
+ continue;
+ if (!layout->entries[i].file)
+ continue;
+
+ const chipoff_t region_start = layout->entries[i].start;
+ const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1;
+ const char *region_file = layout->entries[i].file;
+
+ ret = do_write_buf_to_file(buf + region_start, region_len, region_file);
+ if (ret)
+ goto out;
+ }
+
+ if (filename) {
+ ret = do_write_buf_to_file(buf, flash->chip->total_size * 1024, filename);
+ if (ret)
+ goto out;
+ }
+
+out:
+ return ret;
#endif
}

+
static int read_by_layout(struct flashctx *, uint8_t *);
int read_flash_to_file(struct flashctx *flash, const char *filename)
{
@@ -1431,7 +1491,7 @@
goto out_free;
}

- ret = write_buf_to_file(buf, size, filename);
+ ret = write_buf_to_file(flash, buf, filename);
out_free:
free(buf);
msg_cinfo("%s.\n", ret ? "FAILED" : "done");
@@ -2538,7 +2598,7 @@
goto _free_ret;
}

- if (read_buf_from_file(newcontents, flash_size, filename))
+ if (read_buf_from_file(newcontents, flash_size, filename, "the flash chip's size"))
goto _free_ret;

ret = flashrom_image_write(flash, newcontents, flash_size);
@@ -2559,7 +2619,7 @@
goto _free_ret;
}

- if (read_buf_from_file(newcontents, flash_size, filename))
+ if (read_buf_from_file(newcontents, flash_size, filename, "the flash chip's size"))
goto _free_ret;

ret = flashrom_image_verify(flash, newcontents, flash_size);
diff --git a/layout.c b/layout.c
index 7ce7c57..4ae4a69 100644
--- a/layout.c
+++ b/layout.c
@@ -58,7 +58,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;
}
@@ -67,8 +67,7 @@
char *tstr1, *tstr2;

if (layout.num_entries >= MAX_ROMLAYOUT) {
- msg_gerr("Maximum number of ROM images (%i) in layout "
- "file reached.\n", MAX_ROMLAYOUT);
+ msg_gerr("Maximum number of entries (%i) in layout file reached.\n", MAX_ROMLAYOUT);
(void)fclose(romlayout);
return 1;
}
@@ -90,6 +89,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 = 0;
+ layout.entries[layout.num_entries].file = NULL;
layout.num_entries++;
}

@@ -150,7 +150,6 @@
msg_gspew("Looking for region \"%s\"... ", name);
for (i = 0; i < l->num_entries; i++) {
if (!strcmp(l->entries[i].name, name)) {
- l->entries[i].included = 1;
msg_gspew("found.\n");
return i;
}
@@ -170,28 +169,71 @@
if (num_include_args == 0)
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",
+ msg_gerr("Region requested (with -i/--include \"%s\"),\n"
+ "but no layout data is available. Use the -l/--layout parameter to specify\n"
+ "a layout file.\n",
include_args[0]);
return 1;
}

for (i = 0; i < num_include_args; i++) {
- if (find_romentry(l, include_args[i]) < 0) {
- msg_gerr("Invalid region specified: \"%s\".\n",
- include_args[i]);
+ char *name = include_args[i];
+ char *file = strpbrk(name, ":"); /* -i <region>[:<file>] */
+ if (file != NULL) {
+ *file = '\0';
+ file++;
+ }
+
+ /* Quotes or whitespace are not allowed in region names. */
+ if (strlen(name) == 0 || strpbrk(name, "\" \t\r\n")) {
+ msg_gerr("Invalid region name specified: \"%s\".\n", name);
return 1;
}
- found++;
+
+ int idx = find_romentry(l, name);
+ if (idx < 0) {
+ msg_gerr("Nonexisting region name specified: \"%s\".\n", name);
+ return 1;
+ }
+ if (l->entries[idx].included == 0) {
+ l->entries[idx].included = 1;
+ found++;
+ } else {
+ msg_gerr("Duplicate region name specified: \"%s\".\n", name);
+ return 1;
+ }
+
+ if (file) {
+ if (strlen(file) == 0) {
+ msg_gerr("Empty region file name specified for region \"%s\".\n", name);
+ return 1;
+ }
+#ifdef __LIBPAYLOAD__
+ msg_gerr("Error: No file I/O support in libpayload\n");
+ return 1;
+#else
+ file = strdup(file);
+ if (file == NULL) {
+ msg_gerr("Out of memory!\n");
+ return 1;
+ }
+ l->entries[idx].file = file;
+#endif
+ }
}

- msg_ginfo("Using region%s: \"%s\"", num_include_args > 1 ? "s" : "",
- include_args[0]);
- for (i = 1; i < num_include_args; i++)
- msg_ginfo(", \"%s\"", include_args[i]);
- msg_ginfo(".\n");
+ msg_gdbg("Using %i region%s: ", found, found > 1 ? "s" : "");
+ for (i = 0; i < l->num_entries; i++)
+ if (l->entries[i].included) {
+ msg_gdbg("\"%s\"", l->entries[i].name);
+ if (l->entries[i].file)
+ msg_gdbg(":\"%s\"", l->entries[i].file);
+ if (--found)
+ msg_gdbg(", ");
+ }
+ msg_gdbg(".\n");
return 0;
}

@@ -205,6 +247,8 @@
num_include_args = 0;

for (i = 0; i < layout.num_entries; i++) {
+ free(layout.entries[i].file);
+ layout.entries[i].file = NULL;
layout.entries[i].included = 0;
}
layout.num_entries = 0;
diff --git a/layout.h b/layout.h
index fd1049d..d1c793f 100644
--- a/layout.h
+++ b/layout.h
@@ -43,6 +43,7 @@
chipoff_t end;
bool included;
char name[256];
+ char *file;
};

struct flashrom_layout {

To view, visit change 23021. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic5465659605d8431d931053967b40290195cfd99
Gerrit-Change-Number: 23021
Gerrit-PatchSet: 1
Gerrit-Owner: David Hendricks <david.hendricks@gmail.com>
Gerrit-Reviewer: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>