David Hendricks has uploaded a new change for review. ( https://review.coreboot.org/19342 )
Change subject: layout: Add -i <region>[:<file>] support. ......................................................................
layout: Add -i <region>[:<file>] support.
This is a rebased version of patch ported from chromiumos that adds syntax for working with region-sized files when using a layout.
Original patch: http://codereview.chromium.org/6611015 Ported version: https://mail.coreboot.org/pipermail/flashrom/2013-October/011729.html
TODO(dhendrix): Update description.
Change-Id: Ic5465659605d8431d931053967b40290195cfd99 Signed-off-by: David Hendricks dhendricks@fb.com --- 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, 211 insertions(+), 40 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/42/19342/1
diff --git a/cli_classic.c b/cli_classic.c index 0853a2f..0024483 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -58,7 +58,8 @@ " -A | --noverify-all don't auto-verify whole chip\n" " -l | --layout <layoutfile> read ROM layout from <layoutfile>\n" " -d | --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..cea6b35 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_chip_size, 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 0e24f61..08a6cf6 100644 --- a/flash.h +++ b/flash.h @@ -284,8 +284,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, unsigned long size, const char *filename); int prepare_flash_access(struct flashctx *, bool read_it, bool write_it, bool erase_it, bool verify_it); int do_read(struct flashctx *, const char *filename); int do_erase(struct flashctx *); diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl index c786a86..3bf9aa8 100644 --- a/flashrom.8.tmpl +++ b/flashrom.8.tmpl @@ -46,9 +46,9 @@ .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-E\fR|\fB-r\fR <imagefile>|\fB-w\fR <imagefile>|\fB-v\fR <imagefile>] \ [\fB-c\fR <chipname>] - [(\fB-l\fR <file>|\fB-d\fR) [\fB-i\fR <image>]] [\fB-n\fR] [\fB-f\fR]] + [(\fB-l\fR <file>|\fB-d\fR) [\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 @@ -80,14 +80,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> . 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. @@ -107,14 +107,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. @@ -144,7 +144,7 @@ .sp * Force write even if write is known bad. .TP -.B "-l, --layout <file>" +.B "-l, --layout <region>[:<file>]" Read ROM layout from .BR <file> . .sp @@ -289,6 +289,53 @@ in detail in the .B PROGRAMMER-SPECIFIC INFORMATION 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 diff --git a/flashrom.c b/flashrom.c index 4db3767..cd976cf 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1284,8 +1284,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"); @@ -1308,6 +1307,10 @@ 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; } @@ -1324,12 +1327,22 @@ #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, + * 1 if any read fails. + */ +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;
@@ -1337,6 +1350,7 @@ msg_gerr("No filename specified.\n"); return 1; } + if ((image = fopen(filename, "wb")) == NULL) { msg_gerr("Error: opening file "%s" failed: %s\n", filename, strerror(errno)); return 1; @@ -1373,8 +1387,61 @@ ret = 1; } return ret; +} + +static const struct fl_layout *get_layout(const struct flashctx *const flashctx); +/** + * @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 size Size of chip-sized buffer + * @param filename Optional filename corresponding to -r/-w/-v argument + * @return 0 on success, + * 1 if all available erase functions failed. + */ +int write_buf_to_file(struct flashctx *const flash, 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 + int ret = 0; + const struct fl_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, size, 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) @@ -1400,7 +1467,7 @@ goto out_free; }
- ret = write_buf_to_file(buf, size, filename); + ret = write_buf_to_file(flash, buf, size, filename); out_free: free(buf); msg_cinfo("%s.\n", ret ? "FAILED" : "done"); @@ -2500,7 +2567,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 = fl_image_write(flash, newcontents, flash_size); @@ -2521,7 +2588,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 = fl_image_verify(flash, newcontents, flash_size); diff --git a/layout.c b/layout.c index ab80bb8..6656b4b 100644 --- a/layout.c +++ b/layout.c @@ -50,7 +50,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; } @@ -59,8 +59,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; } @@ -82,6 +81,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++; }
@@ -142,7 +142,6 @@ msg_gspew("Looking for region "%s"... ", name); for (i = 0; i < fl_layout->num_entries; i++) { if (!strcmp(fl_layout->entries[i].name, name)) { - fl_layout->entries[i].included = 1; msg_gspew("found.\n"); return i; } @@ -162,28 +161,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 (fl_layout->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(fl_layout, 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(fl_layout, name); + if (idx < 0) { + msg_gerr("Nonexisting region name specified: "%s".\n", name); + return 1; + } + if (fl_layout->entries[idx].included == 0) { + fl_layout->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; + } + fl_layout->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 < fl_layout->num_entries; i++) + if (fl_layout->entries[i].included) { + msg_gdbg(""%s"", fl_layout->entries[i].name); + if (fl_layout->entries[i].file) + msg_gdbg(":"%s"", fl_layout->entries[i].file); + if (--found) + msg_gdbg(", "); + } + msg_gdbg(".\n"); return 0; }
@@ -197,6 +239,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; @@ -301,6 +345,15 @@ if (entry->start > start) copy_old_content(flash, oldcontents_valid, oldcontents, newcontents, 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. */ diff --git a/layout.h b/layout.h index 71bcac3..92cc6f5 100644 --- a/layout.h +++ b/layout.h @@ -40,6 +40,7 @@ chipoff_t end; bool included; char name[256]; + char *file; };
struct fl_layout {