Am 24.09.2013 01:14 schrieb Stefan Tauner:
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 in a later patch. Existing function read_buf_from_file() is refined and reused to read the file.
based on chromiumos' d0ea9ed71e7f86bb8e8db2ca7c32a96de25343d8 Signed-off-by: David Hendricks dhendrix@chromium.org Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
Counter-proposal, slightly edited patch, comments below are addressed.
diff --git a/cli_classic.c b/cli_classic.c index a0c2d64..ed6d6aa 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -40,8 +40,8 @@ static void cli_classic_usage(const char *name) #if CONFIG_PRINT_WIKI == 1 "-z|" #endif
"-p <programmername>[:<parameters>] [-c <chipname>]\n""[-E|(-r|-w|-v) <file>] [-l <layoutfile> [-i <imagename>]...] [-n] [-f]]\n"
"-p <programmername>[:<parameter>[,<parameter>]...] [-c <chipname>]\n""[-E|(-r|-w|-v) <imagefile>] [-l <layoutfile> [-i <region>[:<regionfile>]...] [-n] [-f]]\n"
80 col violation. Unclosed [.
"[-V[V[V]]] [-o <logfile>]\n\n", name);printf(" -h | --help print this help text\n" diff --git a/layout.c b/layout.c index 08cc776..7e2e904 100644 --- a/layout.c +++ b/layout.c @@ -165,27 +161,61 @@ int process_include_args(void) 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 file is loaded. */ if (num_rom_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. To include one use the -l/--layout syntax).\n",
"To include one [layout data] ..." sounds odd. I rewrote it slightly.
include_args[0]); return 1;}
for (i = 0; i < num_include_args; i++) {
if (find_romentry(include_args[i]) < 0) {msg_gerr("Invalid region specified: \"%s\".\n",include_args[i]);
char *file;char *name = include_args[i];int ret = unquote_string(&name, &file, ":"); /* -i <region>[:<file>] */
I killed the unquote_string requirement here. Whitespace and colons have been outlawed in region names.
if (ret != 0) {msg_gerr("Invalid include argument specified: \"%s\".\n", name); return 1;int idx = find_romentry(name);if (idx < 0) {msg_gerr("Invalid region name specified: \"%s\".\n", name);return 1;}rom_entries[idx].included = 1;if (file[0] != '\0') {/* The remaining characters are interpreted as possible quoted filename. */ret = unquote_string(&file, NULL, NULL);
A file name on the command line should never require quote parsing in the application. Same for any other flashrom UI.
if (ret != 0) {msg_gerr("Invalid region file name specified: \"%s\".\n", file);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;}rom_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("Using region%s: ", num_rom_entries > 1 ? "s" : "");
Should be found, not num_rom_entries.
- bool first = true;
- for (i = 0; i < num_rom_entries; i++)
if (rom_entries[i].included) {if (first)first = false;elsemsg_ginfo(", ");msg_ginfo("\"%s\"", rom_entries[i].name);- } msg_ginfo(".\n"); return 0;
}
There was another bug: register_include_arg() tried to detect duplicate include region arguments. It could only detect identical include strings, but not stuff like this: -i region1 -i region1:region1file.bin The duplicate --include detection has been moved to process_include_args(). Patch 1/6 is no longer a requirement for 2/6 and can thus be committed after 2/6.
TODO: Move the relevant man page chunk from 3/6 to 2/6. Will do so in a followup mail.
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 in a later patch. Existing function read_buf_from_file() is refined and reused to read the file. Document criteria for valid region names. Based on chromiumos' d0ea9ed71e7f86bb8e8db2ca7c32a96de25343d8
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
Index: flashrom-layout_read_region_from_file/flash.h =================================================================== --- flashrom-layout_read_region_from_file/flash.h (Revision 1757) +++ flashrom-layout_read_region_from_file/flash.h (Arbeitskopie) @@ -261,7 +261,7 @@ void list_programmers_linebreak(int startcol, int cols, int paren); int selfcheck(void); int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it); -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); int write_buf_to_file(unsigned char *buf, unsigned long size, const char *filename);
enum test_state { Index: flashrom-layout_read_region_from_file/dummyflasher.c =================================================================== --- flashrom-layout_read_region_from_file/dummyflasher.c (Revision 1757) +++ flashrom-layout_read_region_from_file/dummyflasher.c (Arbeitskopie) @@ -389,7 +389,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"); } Index: flashrom-layout_read_region_from_file/cli_classic.c =================================================================== --- flashrom-layout_read_region_from_file/cli_classic.c (Revision 1757) +++ flashrom-layout_read_region_from_file/cli_classic.c (Arbeitskopie) @@ -34,15 +34,15 @@ static void cli_classic_usage(const char *name) { printf("Please note that the command line interface for flashrom has changed between\n" - "0.9.5 and 0.9.6 and will change again before flashrom 1.0.\n\n"); + "0.9.7 and 0.9.8 and will change again before flashrom 1.0.\n\n");
printf("Usage: %s [-h|-R|-L|" #if CONFIG_PRINT_WIKI == 1 "-z|" #endif - "-p <programmername>[:<parameters>] [-c <chipname>]\n" - "[-E|(-r|-w|-v) <file>] [-l <layoutfile> [-i <imagename>]...] [-n] [-f]]\n" - "[-V[V[V]]] [-o <logfile>]\n\n", name); + "-p <programmername>[:<parameter>[,<parameter>]...] [-c <chipname>]\n" + "[-E|(-r|-w|-v) <imagefile>] [-l <layoutfile> [-i <region>[:<regionfile>]...]]\n" + " [-n] [-f]] [-V[V[V]]] [-o <logfile>]\n\n", name);
printf(" -h | --help print this help text\n" " -R | --version print version (release)\n" @@ -54,8 +54,9 @@ " -c | --chip <chipname> probe only for specified flash chip\n" " -f | --force force specific operations (see man page)\n" " -n | --noverify don't auto-verify\n" - " -l | --layout <layoutfile> read ROM layout from <layoutfile>\n" - " -i | --image <name> only flash image <name> from flash layout\n" + " -l | --layout <layoutfile> read layout from <layoutfile>\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 Index: flashrom-layout_read_region_from_file/layout.c =================================================================== --- flashrom-layout_read_region_from_file/layout.c (Revision 1757) +++ flashrom-layout_read_region_from_file/layout.c (Arbeitskopie) @@ -33,6 +33,7 @@ chipoff_t end; unsigned int included; char name[256]; + char *file; } romentry_t;
/* rom_entries store the entries specified in a layout file and associated run-time data */ @@ -54,7 +55,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; } @@ -63,8 +64,7 @@ char *tstr1, *tstr2;
if (num_rom_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); return 1; } if (2 != fscanf(romlayout, "%s %s\n", tempstr, rom_entries[num_rom_entries].name)) @@ -85,6 +85,7 @@ rom_entries[num_rom_entries].start = strtol(tstr1, (char **)NULL, 16); rom_entries[num_rom_entries].end = strtol(tstr2, (char **)NULL, 16); rom_entries[num_rom_entries].included = 0; + rom_entries[num_rom_entries].file = NULL; num_rom_entries++; }
@@ -100,17 +101,6 @@ } #endif
-/* returns the index of the entry (or a negative value if it is not found) */ -int find_include_arg(const char *const name) -{ - unsigned int i; - for (i = 0; i < num_include_args; i++) { - if (!strcmp(include_args[i], name)) - return i; - } - return -1; -} - /* register an include argument (-i) for later processing */ int register_include_arg(char *name) { @@ -124,11 +114,6 @@ return 1; }
- if (find_include_arg(name) != -1) { - msg_gerr("Duplicate region name: "%s".\n", name); - return 1; - } - include_args[num_include_args] = name; num_include_args++; return 0; @@ -138,14 +123,9 @@ static int find_romentry(char *name) { int i; - - if (num_rom_entries == 0) - return -1; - msg_gspew("Looking for region "%s"... ", name); for (i = 0; i < num_rom_entries; i++) { - if (!strcmp(rom_entries[i].name, name)) { - rom_entries[i].included = 1; + if (strcmp(rom_entries[i].name, name) == 0) { msg_gspew("found.\n"); return i; } @@ -165,28 +145,69 @@ 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 file is loaded. */ if (num_rom_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(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(name); + if (idx < 0) { + msg_gerr("Nonexisting region name specified: "%s".\n", name); + return 1; + } + if (rom_entries[idx].included == 0) { + rom_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; + } + rom_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 < num_rom_entries; i++) + if (rom_entries[i].included) { + msg_gdbg(""%s"", rom_entries[i].name); + if (rom_entries[i].file) + msg_gdbg(":"%s"", rom_entries[i].file); + if (--found) + msg_gdbg(", "); + } + msg_gdbg(".\n"); return 0; }
@@ -200,6 +221,8 @@ num_include_args = 0;
for (i = 0; i < num_rom_entries; i++) { + free(rom_entries[i].file); + rom_entries[i].file = NULL; rom_entries[i].included = 0; } num_rom_entries = 0; @@ -283,6 +306,14 @@ if (entry->start > start) memcpy(newcontents + start, oldcontents + start, entry->start - start); + /* If a file name is specified for this region, read the file contents and + * overwrite @newcontents in the range specified by @entry. */ + if (entry->file != NULL) { + if (read_buf_from_file(newcontents + entry->start, entry->end - entry->start + 1, + entry->file, "the region's size") != 0) + return 1; + } + /* Skip to location after current romentry. */ start = entry->end + 1; /* Catch overflow. */ Index: flashrom-layout_read_region_from_file/flashrom.c =================================================================== --- flashrom-layout_read_region_from_file/flashrom.c (Revision 1757) +++ flashrom-layout_read_region_from_file/flashrom.c (Arbeitskopie) @@ -1165,8 +1165,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"); @@ -1186,8 +1185,10 @@ return 1; } 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); fclose(image); return 1; } @@ -1968,7 +1969,7 @@ }
if (write_it || verify_it) { - if (read_buf_from_file(newcontents, size, filename)) { + if (read_buf_from_file(newcontents, size, filename, "the flash chip's size")) { ret = 1; goto out; } @@ -2002,7 +2003,12 @@ msg_cinfo("done.\n");
/* Build a new image taking the given layout into account. */ - build_new_image(flash, oldcontents, newcontents); + if (build_new_image(flash, oldcontents, newcontents)) { + msg_gerr("Could not prepare the data to be written due to problems with the layout,\n" + "aborting.\n"); + ret = 1; + goto out; + }
// ////////////////////////////////////////////////////////////
Am 08.10.2013 02:34 schrieb Carl-Daniel Hailfinger:
Am 24.09.2013 01:14 schrieb Stefan Tauner:
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 in a later patch. Existing function read_buf_from_file() is refined and reused to read the file.
based on chromiumos' d0ea9ed71e7f86bb8e8db2ca7c32a96de25343d8 Signed-off-by: David Hendricks dhendrix@chromium.org Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
Counter-proposal, slightly edited patch, comments below are addressed.
diff --git a/cli_classic.c b/cli_classic.c index a0c2d64..ed6d6aa 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -40,8 +40,8 @@ static void cli_classic_usage(const char *name) #if CONFIG_PRINT_WIKI == 1 "-z|" #endif
"-p <programmername>[:<parameters>] [-c <chipname>]\n""[-E|(-r|-w|-v) <file>] [-l <layoutfile> [-i <imagename>]...] [-n] [-f]]\n"
"-p <programmername>[:<parameter>[,<parameter>]...] [-c <chipname>]\n""[-E|(-r|-w|-v) <imagefile>] [-l <layoutfile> [-i <region>[:<regionfile>]...] [-n] [-f]]\n"80 col violation. Unclosed [.
"[-V[V[V]]] [-o <logfile>]\n\n", name);printf(" -h | --help print this help text\n" diff --git a/layout.c b/layout.c index 08cc776..7e2e904 100644 --- a/layout.c +++ b/layout.c @@ -165,27 +161,61 @@ int process_include_args(void) 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 file is loaded. */ if (num_rom_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. To include one use the -l/--layout syntax).\n","To include one [layout data] ..." sounds odd. I rewrote it slightly.
include_args[0]); return 1;}
for (i = 0; i < num_include_args; i++) {
if (find_romentry(include_args[i]) < 0) {msg_gerr("Invalid region specified: \"%s\".\n",include_args[i]);
char *file;char *name = include_args[i];int ret = unquote_string(&name, &file, ":"); /* -i <region>[:<file>] */I killed the unquote_string requirement here. Whitespace and colons have been outlawed in region names.
if (ret != 0) {msg_gerr("Invalid include argument specified: \"%s\".\n", name); return 1;int idx = find_romentry(name);if (idx < 0) {msg_gerr("Invalid region name specified: \"%s\".\n", name);return 1;}rom_entries[idx].included = 1;if (file[0] != '\0') {/* The remaining characters are interpreted as possible quoted filename. */ret = unquote_string(&file, NULL, NULL);A file name on the command line should never require quote parsing in the application. Same for any other flashrom UI.
if (ret != 0) {msg_gerr("Invalid region file name specified: \"%s\".\n", file);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;}rom_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("Using region%s: ", num_rom_entries > 1 ? "s" : "");
Should be found, not num_rom_entries.
- bool first = true;
- for (i = 0; i < num_rom_entries; i++)
if (rom_entries[i].included) {if (first)first = false;elsemsg_ginfo(", ");msg_ginfo("\"%s\"", rom_entries[i].name);- } msg_ginfo(".\n"); return 0;
}
There was another bug: register_include_arg() tried to detect duplicate include region arguments. It could only detect identical include strings, but not stuff like this: -i region1 -i region1:region1file.bin The duplicate --include detection has been moved to process_include_args(). Patch 1/6 is no longer a requirement for 2/6 and can thus be committed after 2/6.
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 in a later patch. Existing function read_buf_from_file() is refined and reused to read the file. Document criteria for valid region names. Based on chromiumos' d0ea9ed71e7f86bb8e8db2ca7c32a96de25343d8
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
Latest version. Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-layout_read_region_from_file/flash.h =================================================================== --- flashrom-layout_read_region_from_file/flash.h (Revision 1757) +++ flashrom-layout_read_region_from_file/flash.h (Arbeitskopie) @@ -261,7 +261,7 @@ void list_programmers_linebreak(int startcol, int cols, int paren); int selfcheck(void); int doit(struct flashctx *flash, int force, const char *filename, int read_it, int write_it, int erase_it, int verify_it); -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); int write_buf_to_file(unsigned char *buf, unsigned long size, const char *filename);
enum test_state { Index: flashrom-layout_read_region_from_file/dummyflasher.c =================================================================== --- flashrom-layout_read_region_from_file/dummyflasher.c (Revision 1757) +++ flashrom-layout_read_region_from_file/dummyflasher.c (Arbeitskopie) @@ -389,7 +389,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"); } Index: flashrom-layout_read_region_from_file/cli_classic.c =================================================================== --- flashrom-layout_read_region_from_file/cli_classic.c (Revision 1757) +++ flashrom-layout_read_region_from_file/cli_classic.c (Arbeitskopie) @@ -34,15 +34,15 @@ static void cli_classic_usage(const char *name) { printf("Please note that the command line interface for flashrom has changed between\n" - "0.9.5 and 0.9.6 and will change again before flashrom 1.0.\n\n"); + "0.9.7 and 0.9.8 and will change again before flashrom 1.0.\n\n");
printf("Usage: %s [-h|-R|-L|" #if CONFIG_PRINT_WIKI == 1 "-z|" #endif - "-p <programmername>[:<parameters>] [-c <chipname>]\n" - "[-E|(-r|-w|-v) <file>] [-l <layoutfile> [-i <imagename>]...] [-n] [-f]]\n" - "[-V[V[V]]] [-o <logfile>]\n\n", name); + "-p <programmername>[:<parameter>[,<parameter>]...]\n" + " [-E|(-r|-w|-v) <imagefile>] [-l <layoutfile> [-i <region>[:<regionfile>]]...]\n" + " [-c <chipname>] [-n] [-f]] [-V[V[V]]] [-o <logfile>]\n\n", name);
printf(" -h | --help print this help text\n" " -R | --version print version (release)\n" @@ -54,8 +54,9 @@ " -c | --chip <chipname> probe only for specified flash chip\n" " -f | --force force specific operations (see man page)\n" " -n | --noverify don't auto-verify\n" - " -l | --layout <layoutfile> read ROM layout from <layoutfile>\n" - " -i | --image <name> only flash image <name> from flash layout\n" + " -l | --layout <layoutfile> read layout from <layoutfile>\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 Index: flashrom-layout_read_region_from_file/flashrom.8.tmpl =================================================================== --- flashrom-layout_read_region_from_file/flashrom.8.tmpl (Revision 1757) +++ flashrom-layout_read_region_from_file/flashrom.8.tmpl (Arbeitskopie) @@ -3,10 +3,10 @@ flashrom - detect, read, write, verify and erase flash chips .SH SYNOPSIS .B flashrom \fR[\fB-h\fR|\fB-R\fR|\fB-L\fR|\fB-z\fR|\ -\fB-p\fR <programmername>[:<parameters>] - [\fB-E\fR|\fB-r\fR <file>|\fB-w\fR <file>|\fB-v\fR <file>] \ +\fB-p\fR <programmername>[:<parameter>[,<parameter>]...] + [\fB-E\fR|\fB-r\fR <imagefile>|\fB-w\fR <imagefile>|\fB-v\fR <imagefile>] \ [\fB-c\fR <chipname>] - [\fB-l\fR <file> [\fB-i\fR <image>]] [\fB-n\fR] [\fB-f\fR]] + [\fB-l\fR <layoutfile> [\fB-i\fR <regionname>[:<regionfile>]]...] [\fB-n\fR] [\fB-f\fR]] [\fB-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>] .SH DESCRIPTION .B flashrom @@ -22,8 +22,9 @@ parallel flash, or SPI. .SH OPTIONS .B IMPORTANT: -Please note that the command line interface for flashrom will change before -flashrom 1.0. Do not use flashrom in scripts or other automated tools without +Please note that the command line interface for flashrom has changed between +flashrom 0.9.7 and 0.9.8 and will change again before flashrom 1.0. +Do not use flashrom in scripts or other automated tools without checking that your flashrom version won't interpret options in a different way. .PP You can specify one of @@ -38,14 +39,14 @@ .B -p/--programmer option to be used (please see below). .TP -.B "-r, --read <file>" +.B "-r, --read <imagefile>" Read flash ROM contents and save them into the given -.BR <file> . +.BR <imagefile> . If the file already exists, it will be overwritten. .TP -.B "-w, --write <file>" +.B "-w, --write <imagefile>" Write -.B <file> +.B <imagefile> into flash ROM. This will first automatically .B erase the chip, then write to it. @@ -65,14 +66,14 @@ feel that the time for verification takes too long. .sp Typical usage is: -.B "flashrom -p prog -n -w <file>" +.B "flashrom -p prog -n -w <imagefile>" .sp This option is only useful in combination with .BR --write . .TP -.B "-v, --verify <file>" +.B "-v, --verify <imagefile>" Verify the flash ROM contents against the given -.BR <file> . +.BR <imagefile> . .TP .B "-E, --erase" Erase the flash ROM chip. @@ -102,46 +103,6 @@ .sp * Force write even if write is known bad. .TP -.B "-l, --layout <file>" -Read ROM layout from -.BR <file> . -.sp -flashrom supports ROM layouts. This allows you to flash certain parts of -the flash chip only. A ROM layout file contains multiple lines with the -following syntax: -.sp -.B " startaddr:endaddr imagename" -.sp -.BR "startaddr " "and " "endaddr " -are hexadecimal addresses within the ROM file and do not refer to any -physical address. Please note that using a 0x prefix for those hexadecimal -numbers is not necessary, but you can't specify decimal/octal numbers. -.BR "imagename " "is an arbitrary name for the region/image from" -.BR " startaddr " "to " "endaddr " "(both addresses included)." -.sp -Example: -.sp - 00000000:00008fff gfxrom - 00009000:0003ffff normal - 00040000:0007ffff fallback -.sp -If you only want to update the image named -.BR "normal " "in a ROM based on the layout above, run" -.sp -.B " flashrom -p prog --layout rom.layout --image normal -w some.rom" -.sp -To update only the images named -.BR "normal " "and " "fallback" ", run:" -.sp -.B " flashrom -p prog -l rom.layout -i normal -i fallback -w some.rom" -.sp -Overlapping sections are not supported. -.TP -.B "-i, --image <imagename>" -Only flash region/image -.B <imagename> -from flash layout. -.TP .B "-L, --list-supported" List the flash chips, chipsets, mainboards, and external programmers (including PCI, USB, parallel port, and serial port based devices) @@ -225,6 +186,53 @@ in detail in the .B PROGRAMMER SPECIFIC INFO section. Support for some programmers can be disabled at compile time. +.TP +flashrom supports reading/writing/erasing/verifying flash chips in whole (default) or in part. To access only \ +parts of a chip one has to use layout files and respective arguments described below. +.TP +.B "-l, --layout <layoutfile>" +Read layout entries from +.BR <layoutfile> . +.sp +Each layout entry describes an address region of the flash chip and gives it a name (hereinafter referred to as +a region). One entry per line is allowed with the following syntax: +.sp +.B " startaddr:endaddr regionname" +.sp +.BR "startaddr " "and " "endaddr " +are hexadecimal addresses within the ROM image representing the flash ROM contents and do not refer to any +physical address. Please note that using a 0x prefix for those hexadecimal numbers is not necessary, but you +can't specify decimal/octal numbers. +.BR "regionname " "is the name for the region from " "startaddr " "to " "endaddr " "(both addresses included)." +Spaces, tabs, newlines, colons, single or double quotes are not allowed in region names. +.sp +Example content of file rom.layout: +.sp + 0x00000000:0x00008fff gfx_rom + 0x00009000:0x0003ffff normal + 0x00040000:0x0007ffff fallback +.sp +.TP +.B "-i, --include <name>[:<regionfile>]" +Work on the flash region +.B name +instead of the full address space if a layout file is given and parsed correctly. +Multiple such include parameters can be used to work on the union of different regions. +.sp +The optional +.B regionfile +parameter specifies the name of a file that is used to map the contents of that very region only. +The optional file parameter causes the contents of this region to be replaced by the contents of the file +specified here. +.sp +If you only want to update the regions named +.BR "normal " "and " "gfx_rom " "in a ROM based on the layout mentioned above, run" +.sp +.B " flashrom -p prog -l rom.layout -i normal -i "gfx_rom" -w some.rom" +.sp +Overlapping regions are resolved in an implementation-dependent manner (or may even yield an error) - do +.BR "not " "rely on it." +.sp .B "flashrom -h" lists all supported programmers. .TP Index: flashrom-layout_read_region_from_file/layout.c =================================================================== --- flashrom-layout_read_region_from_file/layout.c (Revision 1757) +++ flashrom-layout_read_region_from_file/layout.c (Arbeitskopie) @@ -33,6 +33,7 @@ chipoff_t end; unsigned int included; char name[256]; + char *file; } romentry_t;
/* rom_entries store the entries specified in a layout file and associated run-time data */ @@ -54,7 +55,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; } @@ -63,8 +64,7 @@ char *tstr1, *tstr2;
if (num_rom_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); return 1; } if (2 != fscanf(romlayout, "%s %s\n", tempstr, rom_entries[num_rom_entries].name)) @@ -85,6 +85,7 @@ rom_entries[num_rom_entries].start = strtol(tstr1, (char **)NULL, 16); rom_entries[num_rom_entries].end = strtol(tstr2, (char **)NULL, 16); rom_entries[num_rom_entries].included = 0; + rom_entries[num_rom_entries].file = NULL; num_rom_entries++; }
@@ -100,17 +101,6 @@ } #endif
-/* returns the index of the entry (or a negative value if it is not found) */ -int find_include_arg(const char *const name) -{ - unsigned int i; - for (i = 0; i < num_include_args; i++) { - if (!strcmp(include_args[i], name)) - return i; - } - return -1; -} - /* register an include argument (-i) for later processing */ int register_include_arg(char *name) { @@ -124,11 +114,6 @@ return 1; }
- if (find_include_arg(name) != -1) { - msg_gerr("Duplicate region name: "%s".\n", name); - return 1; - } - include_args[num_include_args] = name; num_include_args++; return 0; @@ -138,14 +123,9 @@ static int find_romentry(char *name) { int i; - - if (num_rom_entries == 0) - return -1; - msg_gspew("Looking for region "%s"... ", name); for (i = 0; i < num_rom_entries; i++) { - if (!strcmp(rom_entries[i].name, name)) { - rom_entries[i].included = 1; + if (strcmp(rom_entries[i].name, name) == 0) { msg_gspew("found.\n"); return i; } @@ -165,28 +145,69 @@ 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 file is loaded. */ if (num_rom_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(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(name); + if (idx < 0) { + msg_gerr("Nonexisting region name specified: "%s".\n", name); + return 1; + } + if (rom_entries[idx].included == 0) { + rom_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; + } + rom_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 < num_rom_entries; i++) + if (rom_entries[i].included) { + msg_gdbg(""%s"", rom_entries[i].name); + if (rom_entries[i].file) + msg_gdbg(":"%s"", rom_entries[i].file); + if (--found) + msg_gdbg(", "); + } + msg_gdbg(".\n"); return 0; }
@@ -200,6 +221,8 @@ num_include_args = 0;
for (i = 0; i < num_rom_entries; i++) { + free(rom_entries[i].file); + rom_entries[i].file = NULL; rom_entries[i].included = 0; } num_rom_entries = 0; @@ -283,6 +306,14 @@ if (entry->start > start) memcpy(newcontents + start, oldcontents + start, entry->start - start); + /* If a file name is specified for this region, read the file contents and + * overwrite @newcontents in the range specified by @entry. */ + if (entry->file != NULL) { + if (read_buf_from_file(newcontents + entry->start, entry->end - entry->start + 1, + entry->file, "the region's size") != 0) + return 1; + } + /* Skip to location after current romentry. */ start = entry->end + 1; /* Catch overflow. */ Index: flashrom-layout_read_region_from_file/flashrom.c =================================================================== --- flashrom-layout_read_region_from_file/flashrom.c (Revision 1757) +++ flashrom-layout_read_region_from_file/flashrom.c (Arbeitskopie) @@ -1165,8 +1165,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"); @@ -1186,8 +1185,10 @@ return 1; } 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); fclose(image); return 1; } @@ -1968,7 +1969,7 @@ }
if (write_it || verify_it) { - if (read_buf_from_file(newcontents, size, filename)) { + if (read_buf_from_file(newcontents, size, filename, "the flash chip's size")) { ret = 1; goto out; } @@ -2002,7 +2003,12 @@ msg_cinfo("done.\n");
/* Build a new image taking the given layout into account. */ - build_new_image(flash, oldcontents, newcontents); + if (build_new_image(flash, oldcontents, newcontents)) { + msg_gerr("Could not prepare the data to be written due to problems with the layout,\n" + "aborting.\n"); + ret = 1; + goto out; + }
// ////////////////////////////////////////////////////////////
-
Carl-Daniel Hailfinger