[flashrom] [PATCH 2/6] layout: Add -i <region>[:<file>] support.
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Thu Oct 10 07:35:48 CEST 2013
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 at chromium.org>
>> Signed-off-by: Stefan Tauner <stefan.tauner at 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;
>> found++;
>> +
>> + 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;
>> + else
>> + msg_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 at chromium.org>
> Signed-off-by: Stefan Tauner <stefan.tauner at student.tuwien.ac.at>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Latest version.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at 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;
+ }
// ////////////////////////////////////////////////////////////
--
http://www.hailfinger.org/
More information about the flashrom
mailing list