[flashrom] [PATCH][RFC] rewrite read/write/verify to support layout/region based operations
Alexander Couzens
lynxis at fe80.eu
Sun Oct 12 22:37:23 CEST 2014
Signed-off-by: Alexander Couzens <lynxis at fe80.eu>
---
Hi everyone,
I've rewritten read/write/verify functions to get romregions/layouts working in all cases.
A write to a specific region required before a complete read of the chip.
Because of this it wasn't possible to write to a specific region, when another region is locked (ME lockdown).
And flashrom is faster now when writing to a rom region.
Best,
lynxis
I've melt all my commits into a single one. A small commit version can be found at
https://github.com/lynxis/flashrom/tree/v1/
cli_classic.c | 5 -
flash.h | 13 +++
flashrom.c | 336 +++++++++++++++++++++++++++++++++++-----------------------
layout.c | 23 ++--
4 files changed, 228 insertions(+), 149 deletions(-)
diff --git a/cli_classic.c b/cli_classic.c
index 8588881..613c7aa 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -368,11 +368,6 @@ int main(int argc, char *argv[])
ret = 1;
goto out;
}
- if (layoutfile != NULL && !write_it) {
- msg_gerr("Layout files are currently supported for write operations only.\n");
- ret = 1;
- goto out;
- }
if (process_include_args()) {
ret = 1;
diff --git a/flash.h b/flash.h
index 03b26e7..ec85398 100644
--- a/flash.h
+++ b/flash.h
@@ -337,6 +337,19 @@ __attribute__((format(printf, 2, 3)));
#define msg_cspew(...) print(MSG_SPEW, __VA_ARGS__) /* chip debug spew */
/* layout.c */
+
+#define MAX_ROMLAYOUT 32
+
+typedef struct {
+ chipoff_t start;
+ chipoff_t end;
+ unsigned int included;
+ char name[256];
+} romentry_t;
+
+extern romentry_t rom_entries[];
+extern int num_rom_entries;
+
int register_include_arg(char *name);
int process_include_args(void);
int read_romlayout(const char *name);
diff --git a/flashrom.c b/flashrom.c
index 9b82d4c..b2c59c2 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1434,45 +1434,67 @@ static int erase_and_write_block_helper(struct flashctx *flash,
return ret;
}
-static int walk_eraseregions(struct flashctx *flash, int erasefunction,
- int (*do_something) (struct flashctx *flash,
- unsigned int addr,
- unsigned int len,
- uint8_t *param1,
- uint8_t *param2,
- int (*erasefn) (
- struct flashctx *flash,
- unsigned int addr,
- unsigned int len)),
- void *param1, void *param2)
+static int walk_eraseregions_offset_len(
+ struct flashctx *flash, int erasefunction,
+ int (*do_something) (struct flashctx *flash,
+ unsigned int addr,
+ unsigned int len,
+ uint8_t *param1,
+ uint8_t *param2,
+ int (*erasefn) (
+ struct flashctx *flash,
+ unsigned int addr,
+ unsigned int len)
+ ),
+ void *oldcontent,
+ void *newcontent,
+ unsigned offset,
+ unsigned len)
{
int i, j;
unsigned int start = 0;
- unsigned int len;
+ unsigned int eraselen;
struct block_eraser eraser = flash->chip->block_erasers[erasefunction];
for (i = 0; i < NUM_ERASEREGIONS; i++) {
/* count==0 for all automatically initialized array
* members so the loop below won't be executed for them.
*/
- len = eraser.eraseblocks[i].size;
- for (j = 0; j < eraser.eraseblocks[i].count; j++) {
+ eraselen = eraser.eraseblocks[i].size;
+ for (j = 0; j < eraser.eraseblocks[i].count; j++, start += eraselen) {
+ if (start < offset)
+ continue;
+
/* Print this for every block except the first one. */
if (i || j)
msg_cdbg(", ");
- msg_cdbg("0x%06x-0x%06x", start,
- start + len - 1);
- if (do_something(flash, start, len, param1, param2,
- eraser.block_erase)) {
+ msg_cdbg("0x%06x-0x%06x", start, start + eraselen - 1);
+ if (do_something(flash, start, eraselen, oldcontent, newcontent, eraser.block_erase)) {
return 1;
}
- start += len;
}
}
msg_cdbg("\n");
return 0;
}
+static int walk_eraseregions(struct flashctx *flash, int erasefunction,
+ int (*do_something) (struct flashctx *flash,
+ unsigned int addr,
+ unsigned int len,
+ uint8_t *param1,
+ uint8_t *param2,
+ int (*erasefn) (
+ struct flashctx *flash,
+ unsigned int addr,
+ unsigned int len)
+ ),
+ void *oldcontent,
+ void *newcontent)
+{
+ return walk_eraseregions_offset_len(flash, erasefunction, do_something, oldcontent, newcontent, 0, flash->chip->total_size);
+}
+
static int check_block_eraser(const struct flashctx *flash, int k, int log)
{
struct block_eraser eraser = flash->chip->block_erasers[k];
@@ -1562,7 +1584,7 @@ int erase_and_write_flash(struct flashctx *flash, uint8_t *oldcontents, uint8_t
}
return ret;
}
-
+/* FIXME: Unused!
static void nonfatal_help_message(void)
{
msg_gerr("Good, writing to the flash chip apparently didn't do anything.\n");
@@ -1579,7 +1601,7 @@ static void nonfatal_help_message(void)
"the programmer and the flash chip. If you think the error is caused by flashrom\n"
"please report this on IRC at chat.freenode.net (channel #flashrom) or\n"
"mail flashrom at flashrom.org, thanks!\n");
-}
+} */
static void emergency_help_message(void)
{
@@ -1895,153 +1917,201 @@ int chip_safety_check(const struct flashctx *flash, int force, int read_it, int
return 0;
}
-/* This function signature is horrible. We need to design a better interface,
- * but right now it allows us to split off the CLI code.
- * Besides that, the function itself is a textbook example of abysmal code flow.
- */
-int doit(struct flashctx *flash, int force, const char *filename, int read_it,
- int write_it, int erase_it, int verify_it)
-{
- uint8_t *oldcontents;
- uint8_t *newcontents;
+int read_flash(struct flashctx *flash, const char *filename) {
int ret = 0;
- unsigned long size = flash->chip->total_size * 1024;
+ int err = 0; /* used as ret for read_flash if a single partition fails */
+ size_t flashsize = flash->chip->total_size * 1024;
+ int i;
- if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) {
- msg_cerr("Aborting.\n");
- return 1;
- }
+ uint8_t *contents = calloc(1, flashsize);
- if (normalize_romentries(flash)) {
- msg_cerr("Requested regions can not be handled. Aborting.\n");
- return 1;
- }
+ for (i=0; i < num_rom_entries; i++) {
+ chipoff_t start = rom_entries[i].start;
+ chipoff_t end = rom_entries[i].end;
+ chipsize_t length = end - start;
- /* Given the existence of read locks, we want to unlock for read,
- * erase and write.
- */
- if (flash->chip->unlock)
- flash->chip->unlock(flash);
+ /* ignore entries not included */
+ if (!rom_entries[i].included)
+ continue;
- if (read_it) {
- return read_flash_to_file(flash, filename);
+ ret = flash->chip->read(flash, contents + start, start, length);
+ if (ret) {
+ msg_cerr("Read for part %s (0x%x - 0x%x) failed!", rom_entries[i].name, start, end);
+ err = -1;
+ continue;
+ }
}
- oldcontents = malloc(size);
- if (!oldcontents) {
- msg_gerr("Out of memory!\n");
- exit(1);
+ ret = write_buf_to_file(contents, flashsize, filename);
+ if (ret) {
+ msg_cerr("Writing to file %s failed!", filename);
+ err = -1;
}
+
+ free(contents);
+ return err;
+}
+
+int write_flash(struct flashctx *flash, const char *filename) {
+ int ret = 0;
+ size_t flashsize = flash->chip->total_size * 1024;
+ int i;
+
+ uint8_t *oldcontents = calloc(1, flashsize);
+ uint8_t *newcontents = calloc(1, flashsize);
+
/* Assume worst case: All bits are 0. */
- memset(oldcontents, 0x00, size);
- newcontents = malloc(size);
- if (!newcontents) {
- msg_gerr("Out of memory!\n");
- exit(1);
- }
- /* Assume best case: All bits should be 1. */
- memset(newcontents, 0xff, size);
+ memset(oldcontents, 0x00, flashsize);
+
+ /* Assume worst case: All bits are 0. */
+ memset(newcontents, 0xff, flashsize);
/* Side effect of the assumptions above: Default write action is erase
* because newcontents looks like a completely erased chip, and
* oldcontents being completely 0x00 means we have to erase everything
* before we can write.
*/
- if (erase_it) {
+ if(read_buf_from_file(newcontents, flashsize, filename)) {
+ msg_cerr("Can not read file %s\n. Aborting.\n", filename);
+ return -1;
+ }
+
+#if CONFIG_INTERNAL == 1
+ /* FIXME: move this code into a small function and move it out of write_flash */
+ if (programmer == PROGRAMMER_INTERNAL && cb_check_image(newcontents, flashsize) < 0) {
+ if (force_boardmismatch) {
+ msg_pinfo("Proceeding anyway because user forced us to.\n");
+ } else {
+ msg_perr("Aborting. You can override this with "
+ "-p internal:boardmismatch=force.\n");
+ return 1;
+ }
+ }
+#endif
+
+ for (i=0; i < num_rom_entries; i++) {
+ chipoff_t start = rom_entries[i].start;
+ chipoff_t end = rom_entries[i].end;
+ chipsize_t length = end - start;
+
+ /* ignore entries not included */
+ if (!rom_entries[i].included)
+ continue;
+
+ /* read specified flash region */
+ ret = flash->chip->read(flash, oldcontents + start, start, length);
+ if(ret) {
+ msg_cerr("Can not read flash position 0x%x len: 0x%x\n. Ret: %d Ignoring.\n", start, length, ret);
+ return -1;
+ }
+
+ int k;
+ for (k = 0; k < NUM_ERASEFUNCTIONS; k++) {
+ if (k != 0)
+ msg_cinfo("Looking for another erase function.\n");
+
+ msg_cdbg("Trying erase function %i... ", k);
+ if (check_block_eraser(flash, k, 1))
+ continue;
+
+ ret = walk_eraseregions_offset_len(flash, k, &erase_and_write_block_helper, oldcontents, newcontents, start, length);
+
+ /* If everything is OK, don't try another erase function. */
+ if (!ret)
+ break;
+ }
+
+ if (!ret) {
+ msg_cinfo("Finished flashing %d - region %s", ret, rom_entries[i].name);
+ } else {
+ msg_cerr("Failed flashing!");
+ emergency_help_message();
/* FIXME: Do we really want the scary warning if erase failed?
* After all, after erase the chip is either blank or partially
* blank or it has the old contents. A blank chip won't boot,
* so if the user wanted erase and reboots afterwards, the user
* knows very well that booting won't work.
*/
- if (erase_and_write_flash(flash, oldcontents, newcontents)) {
- emergency_help_message();
- ret = 1;
}
- goto out;
}
- if (write_it || verify_it) {
- if (read_buf_from_file(newcontents, size, filename)) {
- ret = 1;
- goto out;
- }
+ return 0;
+}
-#if CONFIG_INTERNAL == 1
- if (programmer == PROGRAMMER_INTERNAL && cb_check_image(newcontents, size) < 0) {
- if (force_boardmismatch) {
- msg_pinfo("Proceeding anyway because user forced us to.\n");
- } else {
- msg_perr("Aborting. You can override this with "
- "-p internal:boardmismatch=force.\n");
- ret = 1;
- goto out;
- }
- }
-#endif
+int verify_flash(struct flashctx *flash, const char *filename) {
+ int ret = 0;
+ int verified = 0;
+ size_t flashsize = flash->chip->total_size * 1024;
+ int i;
+
+ /* we alloc here the whole flashsize to be sure we have enought space */
+ uint8_t *filecontents = calloc(1, flashsize);
+
+ if(read_buf_from_file(filecontents, flashsize, filename)) {
+ msg_cerr("Can not read file %s\n. Aborting.\n", filename);
+ return -1;
}
- /* Read the whole chip to be able to check whether regions need to be
- * erased and to give better diagnostics in case write fails.
- * The alternative would be to read only the regions which are to be
- * preserved, but in that case we might perform unneeded erase which
- * takes time as well.
- */
- msg_cinfo("Reading old flash chip contents... ");
- if (flash->chip->read(flash, oldcontents, 0, size)) {
- ret = 1;
- msg_cinfo("FAILED.\n");
- goto out;
+ for (i=0; i < num_rom_entries; i++) {
+ chipoff_t start = rom_entries[i].start;
+ chipoff_t end = rom_entries[i].end;
+ chipsize_t length = end - start;
+
+ /* ignore entries not included */
+ if (!rom_entries[i].included)
+ continue;
+
+ ret = verify_range(flash, filecontents + start, start, length);
+ if (ret)
+ verified = -1;
}
- msg_cinfo("done.\n");
- /* Build a new image taking the given layout into account. */
- build_new_image(flash, oldcontents, newcontents);
+ return verified;
+}
+
+/* This function signature is horrible. We need to design a better interface,
+ * but right now it allows us to split off the CLI code.
+ * Besides that, the function itself is a textbook example of abysmal code flow.
+ */
+int doit(struct flashctx *flash, int force, const char *filename, int read_it,
+ int write_it, int erase_it, int verify_it)
+{
+
+ int ret = 0;
- // ////////////////////////////////////////////////////////////
+ if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) {
+ msg_cerr("Aborting.\n");
+ return 1;
+ }
- if (write_it) {
- if (erase_and_write_flash(flash, oldcontents, newcontents)) {
- msg_cerr("Uh oh. Erase/write failed. Checking if anything has changed.\n");
- msg_cinfo("Reading current flash chip contents... ");
- if (!flash->chip->read(flash, newcontents, 0, size)) {
- msg_cinfo("done.\n");
- if (!memcmp(oldcontents, newcontents, size)) {
- nonfatal_help_message();
- ret = 1;
- goto out;
- }
- msg_cerr("Apparently at least some data has changed.\n");
- } else
- msg_cerr("Can't even read anymore!\n");
- emergency_help_message();
- ret = 1;
- goto out;
- }
+ if (normalize_romentries(flash)) {
+ msg_cerr("Requested regions can not be handled. Aborting.\n");
+ return 1;
}
- /* Verify only if we either did not try to write (verify operation) or actually changed something. */
- if (verify_it && (!write_it || !all_skipped)) {
- msg_cinfo("Verifying flash... ");
+ /* Given the existence of read locks, we want to unlock for read,
+ * erase and write.
+ */
+ if (flash->chip->unlock)
+ flash->chip->unlock(flash);
- if (write_it) {
- /* Work around chips which need some time to calm down. */
- programmer_delay(1000*1000);
- ret = verify_range(flash, newcontents, 0, size);
- /* If we tried to write, and verification now fails, we
- * might have an emergency situation.
- */
- if (ret)
- emergency_help_message();
- } else {
- ret = compare_range(newcontents, oldcontents, 0, size);
- }
- if (!ret)
- msg_cinfo("VERIFIED.\n");
+ if (read_it) {
+ return read_flash(flash, filename);
}
-out:
- free(oldcontents);
- free(newcontents);
+ if (write_it) {
+ ret = write_flash(flash, filename);
+ if (ret)
+ return ret;
+ }
+
+ if (verify_it) {
+ ret = verify_flash(flash, filename);
+ if (ret)
+ return ret;
+ }
+
+ msg_cinfo("done.\n");
+
return ret;
}
diff --git a/layout.c b/layout.c
index a12eb28..9bdf9b6 100644
--- a/layout.c
+++ b/layout.c
@@ -26,18 +26,9 @@
#include "flash.h"
#include "programmer.h"
-#define MAX_ROMLAYOUT 32
-
-typedef struct {
- chipoff_t start;
- chipoff_t end;
- unsigned int included;
- char name[256];
-} romentry_t;
-
/* rom_entries store the entries specified in a layout file and associated run-time data */
-static romentry_t rom_entries[MAX_ROMLAYOUT];
-static int num_rom_entries = 0; /* the number of successfully parsed rom_entries */
+romentry_t rom_entries[MAX_ROMLAYOUT];
+int num_rom_entries = 0; /* the number of successfully parsed rom_entries */
/* include_args holds the arguments specified at the command line with -i. They must be processed at some point
* so that desired regions are marked as "included" in the rom_entries list. */
@@ -254,6 +245,16 @@ int normalize_romentries(const struct flashctx *flash)
}
}
+ /* create a partition over the complete flash when no layout given */
+ if (num_rom_entries == 0) {
+ rom_entries[0].start = 0x0;
+ rom_entries[0].end = flash->chip->total_size * 1024 - 1;
+ rom_entries[0].included = 1;
+ strncpy(rom_entries[0].name, "complete flash", 15);
+
+ num_rom_entries = 1;
+ }
+
return ret;
}
--
2.1.2
More information about the flashrom
mailing list