[flashrom] Contribution: option to disable read_all_first

Willy Tarreau w at 1wt.eu
Thu Sep 10 23:13:36 CEST 2015


Hello,

I was bothered by having to read all the contents of an empty flash
before programming just a boot loader to it. It's particularly long
when using a buspirate board. I looked into the code to see how to
bypass this and discovered it was already planned but not implemented
due to the (presumably) complex API of the doit() function.

I took a different route : I'm using 3 different write levels in write_it:
  - normal write (read first)
  - trusted write (no need to read but still erase)
  - fully trusted write (flash assumed to be clean)

These ones are set using a new "-t" flag for which I have even updated
the man page and indicated that it's not recommended.

It worked well for me so I'm sending the patches assuming they'll be
useful for someone else.

BTW, to give you a bit more context, I was writing a 8MB flash to upgrade
a small router from its 4MB one, so I just had to add 4MB of \xff after the
existing image before flashing it.

Best regards,
Willy

-------------- next part --------------
>From 0d6a5248b79b2d62c4f2355772ed0538c8284883 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w at 1wt.eu>
Date: Thu, 10 Sep 2015 22:16:01 +0200
Subject: cleanup: move misplaced done message in read_all_first

---
 flashrom.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/flashrom.c b/flashrom.c
index a389cb2..fdd95c3 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -2020,8 +2020,8 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
 			msg_cinfo("FAILED.\n");
 			goto out;
 		}
+		msg_cinfo("done.\n");
 	}
-	msg_cinfo("done.\n");
 
 	/* Build a new image taking the given layout into account. */
 	if (build_new_image(flash, read_all_first, oldcontents, newcontents)) {
-- 
1.7.12.1

-------------- next part --------------
>From 94c83f456eb2499de477b3465fd74d79dcb1791d Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w at 1wt.eu>
Date: Thu, 10 Sep 2015 22:26:00 +0200
Subject: improvement: support a trusted mode to avoid reading everything

Passing -t once avoids the full read, a second time assumes the
flash is initially clean.
---
 cli_classic.c | 12 +++++++++++-
 flashrom.8    | 18 ++++++++++++++++--
 flashrom.c    | 12 ++++++++++++
 3 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/cli_classic.c b/cli_classic.c
index a2c2014..5efe0cf 100644
--- a/cli_classic.c
+++ b/cli_classic.c
@@ -61,6 +61,7 @@ static void cli_classic_usage(const char *name)
 #if CONFIG_PRINT_WIKI == 1
 	       " -z | --list-supported-wiki         print supported devices in wiki syntax\n"
 #endif
+	       " -t | --trust                       once=don't read first, twice=assume clean\n"
 	       " -p | --programmer <name>[:<param>] specify the programmer device. One of\n");
 	list_programmers_linebreak(4, 80, 0);
 	printf(".\n\nYou can specify one of -h, -R, -L, "
@@ -102,11 +103,12 @@ int main(int argc, char *argv[])
 	int list_supported_wiki = 0;
 #endif
 	int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0;
+	int trust_it = 0;
 	int dont_verify_it = 0, list_supported = 0, operation_specified = 0;
 	enum programmer prog = PROGRAMMER_INVALID;
 	int ret = 0;
 
-	static const char optstring[] = "r:Rw:v:nVEfc:l:i:p:Lzho:";
+	static const char optstring[] = "r:Rw:v:nVEfc:l:i:p:Lzho:t";
 	static const struct option long_options[] = {
 		{"read",		1, NULL, 'r'},
 		{"write",		1, NULL, 'w'},
@@ -124,6 +126,7 @@ int main(int argc, char *argv[])
 		{"help",		0, NULL, 'h'},
 		{"version",		0, NULL, 'R'},
 		{"output",		1, NULL, 'o'},
+		{"trust",		0, NULL, 't'},
 		{NULL,			0, NULL, 0},
 	};
 
@@ -315,6 +318,9 @@ int main(int argc, char *argv[])
 			}
 #endif /* STANDALONE */
 			break;
+		case 't':
+			trust_it++;
+			break;
 		default:
 			cli_classic_abort_usage();
 			break;
@@ -531,6 +537,10 @@ int main(int argc, char *argv[])
 	if (write_it && !dont_verify_it)
 		verify_it = 1;
 
+	/* we may want to go faster if we know what we're doing */
+	if (write_it)
+		write_it += trust_it;
+
 	/* Map the selected flash chip again. */
 	if (map_flash(fill_flash) != 0) {
 		ret = 1;
diff --git a/flashrom.8 b/flashrom.8
index 8a2c279..d6f42ae 100644
--- a/flashrom.8
+++ b/flashrom.8
@@ -6,7 +6,7 @@ flashrom \- detect, read, write, verify and erase flash chips
 \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\-i\fR <image>]] [\fB\-n\fR] [\fB\-f\fR]]
+               [\fB\-l\fR <file> [\fB\-i\fR <image>]] [\fB\-n\fR] [\fB\-f\fR] [\fB\-t\fR]*]
          [\fB\-V\fR[\fBV\fR[\fBV\fR]]] [\fB-o\fR <logfile>]
 .SH DESCRIPTION
 .B flashrom
@@ -55,7 +55,9 @@ is made for disaster recovery and to be able to skip regions that are
 already equal to the image file. This copy is updated along with the write
 operation. In case of erase errors it is even re-read completely. After
 writing has finished and if verification is enabled, the whole flash chip is
-read out and compared with the input image.
+read out and compared with the input image. Note that it is possible in some
+circumstances to bypass the backup of a chip you don't care about or you trust
+to be clean.
 .TP
 .B "\-n, \-\-noverify"
 Skip the automatic verification of flash ROM contents after writing. Using this
@@ -83,6 +85,18 @@ More verbose output. This option can be supplied multiple times
 .BR \-VVV )
 for even more debug output.
 .TP
+.BR \-t
+Increase trust in the chip you're programming. A single \-t means you don't
+want to backup its contents before writing, so that
+.B
+they are definitely lost.
+A second \-t means you trust it even further and you
+.B
+know
+the chip is clean and does not need to be erased. It is highly recommended to
+verify the chip after writing when doing this. Use this only on brand new chips
+coming directly from the factory.
+.TP
 .B "\-c, \-\-chip" <chipname>
 Probe only for the specified flash ROM chip. This option takes the chip name as
 printed by
diff --git a/flashrom.c b/flashrom.c
index fdd95c3..2ca0e9e 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1923,6 +1923,12 @@ int chip_safety_check(const struct flashctx *flash, int force, int read_it, int
 /* 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.
+ *
+ * For now write_it supports 4 modes :
+ *   - 0 : no write
+ *   - 1 : read_all_first, then write
+ *   - 2 : assume all zero, then write
+ *   - 3 : assume all 0xFF, then write (valid only for brand new chips)
  */
 int doit(struct flashctx *flash, int force, const char *filename, int read_it,
 	 int write_it, int erase_it, int verify_it)
@@ -1993,6 +1999,12 @@ int doit(struct flashctx *flash, int force, const char *filename, int read_it,
 			goto out;
 		}
 
+		if (write_it > 1)
+			read_all_first = 0;
+
+		if (write_it > 2)
+			memset(oldcontents, 0xff, size);
+
 #if CONFIG_INTERNAL == 1
 		if (programmer == PROGRAMMER_INTERNAL && cb_check_image(newcontents, size) < 0) {
 			if (force_boardmismatch) {
-- 
1.7.12.1



More information about the flashrom mailing list