[flashrom] [PATCH 2/3] Make read before write configurable (infrastructure part)
Stefan Tauner
stefan.tauner at student.tuwien.ac.at
Mon Aug 15 06:43:37 CEST 2011
- Introduce a variable in doit() that allows to influence
read-before-write and its consequences.
- Rename handle_romentries to build_new_image and a description of
its purpose.
- Modify build_new_image so that it still works even if the old content
is not read before.
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>
---
This is completely wrong.
When the user wants to skip most of the chip AND wants to avoid the
safety net, it makes no sense at all to read most of the chip in
build_new_image. This is needed because the write/skip logic works by
comparing the old content with the new content.
In the case that old content is already known (because we have read
the whole chip before), this is no problem and even nice from a
architectural point of view. Allowing to skip the reading breaks the
concept though. This patch provides the ugly code that is necessary
so that this concept does not explode. It does not fix the concept
itself. In the case the user wants to write the whole chip, this
provides a real benefit. The smaller the region is that he wants to
update, the small the benefit becomes.
The real solution would be to provide the erase and write logic not
just a complete image, from which it can deduce what it can and
should skip, but also a layout-like map of what the user wants to be
changed (or something similar that allows the following). If the old
contents are known, then fine, but if not, read only the parts
needed due to erase block overheads and not everything unnecessarily.
---
flash.h | 2 +-
flashrom.c | 42 ++++++++++++++++++++++--------------------
layout.c | 42 +++++++++++++++++++++++++++++++++++++-----
3 files changed, 60 insertions(+), 26 deletions(-)
diff --git a/flash.h b/flash.h
index 4b1cca2..c3644ce 100644
--- a/flash.h
+++ b/flash.h
@@ -257,7 +257,7 @@ int cli_classic(int argc, char *argv[]);
/* layout.c */
int read_romlayout(char *name);
int find_romentry(char *name);
-int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents);
+int build_new_image(struct flashchip *flash, int oldcontents_valid, uint8_t *oldcontents, uint8_t *newcontents);
/* spi.c */
struct spi_command {
diff --git a/flashrom.c b/flashrom.c
index 07cfdd9..b0e3b15 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -1865,6 +1865,7 @@ int doit(struct flashchip *flash, int force, const char *filename, int read_it,
uint8_t *newcontents;
int ret = 0;
unsigned long size = flash->total_size * 1024;
+ int read_all_first = 1; /* FIXME: Make this configurable. */
if (chip_safety_check(flash, force, read_it, write_it, erase_it, verify_it)) {
msg_cerr("Aborting.\n");
@@ -1931,28 +1932,27 @@ int doit(struct flashchip *flash, int force, const char *filename, int read_it,
/* 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
+ * The alternative is 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->read(flash, oldcontents, 0, size)) {
- ret = 1;
- msg_cinfo("FAILED.\n");
- goto out;
+ if (read_all_first) {
+ msg_cinfo("Reading old flash chip contents... ");
+ if (flash->read(flash, oldcontents, 0, size)) {
+ ret = 1;
+ msg_cinfo("FAILED.\n");
+ goto out;
+ }
}
msg_cinfo("done.\n");
- // This should be moved into each flash part's code to do it
- // cleanly. This does the job.
- handle_romentries(flash, oldcontents, newcontents);
+ /* Build a new image from the given layout. */
+ build_new_image(flash, read_all_first, oldcontents, newcontents);
- // ////////////////////////////////////////////////////////////
-
- if (write_it) {
- if (erase_and_write_flash(flash, oldcontents, newcontents)) {
- msg_cerr("Uh oh. Erase/write failed. Checking if "
- "anything changed.\n");
+ if (write_it && erase_and_write_flash(flash, oldcontents, newcontents)) {
+ msg_cerr("Uh oh. Erase/write failed.");
+ if (read_all_first) {
+ msg_cerr("Checking if anything changed... ");
if (!flash->read(flash, newcontents, 0, size)) {
if (!memcmp(oldcontents, newcontents, size)) {
msg_cinfo("Good. It seems nothing was "
@@ -1961,11 +1961,13 @@ int doit(struct flashchip *flash, int force, const char *filename, int read_it,
ret = 1;
goto out;
}
- }
- emergency_help_message();
- ret = 1;
- goto out;
- }
+ } else
+ msg_cerr("failed.\n");
+ } else
+ msg_cerr("\n");
+ emergency_help_message();
+ ret = 1;
+ goto out;
}
if (verify_it) {
diff --git a/layout.c b/layout.c
index 6b9627a..8283e36 100644
--- a/layout.c
+++ b/layout.c
@@ -213,7 +213,7 @@ int find_romentry(char *name)
return -1;
}
-struct rom_entry *get_next_included_romentry(unsigned int start)
+static struct rom_entry *get_next_included_romentry(unsigned int start)
{
int i;
unsigned int best_start = UINT_MAX;
@@ -240,11 +240,18 @@ struct rom_entry *get_next_included_romentry(unsigned int start)
return best_entry;
}
-int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents)
+/**
+ * Modify @newcontents so that it contains the data that should be on the chip
+ * eventually. In the case the user wants to update only parts of it, copy
+ * the chunks to be preserved from @oldcontents to @newcontents. If @oldcontents
+ * is not valid, we need to fetch the current data from the chip first.
+ */
+int build_new_image(struct flashchip *flash, int oldcontents_valid, uint8_t *oldcontents, uint8_t *newcontents)
{
unsigned int start = 0;
struct rom_entry *entry;
unsigned int size = flash->total_size * 1024;
+ unsigned int copy_size = 0;
/* If no layout file was specified or the layout file was empty, assume
* that the user wants to flash the complete new image.
@@ -258,13 +265,38 @@ int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *ne
entry = get_next_included_romentry(start);
/* No more romentries for remaining region? */
if (entry == NULL) {
+ copy_size = size - start;
+ if (!oldcontents_valid) {
+ /* oldcontents is a zero-filled buffer. By
+ * reading into oldcontents, we avoid a rewrite
+ * of identical regions even if an initial full
+ * chip read didn't happen. */
+ msg_gdbg("reading last old chunk 0x%06x (0x%06x)\n",
+ start, copy_size);
+ flash->read(flash, oldcontents + start,
+ start,
+ copy_size);
+ }
memcpy(newcontents + start, oldcontents + start,
- size - start);
+ copy_size);
break;
}
- if (entry->start > start)
+ if (entry->start > start) {
+ copy_size = entry->start - start;
+ if (!oldcontents_valid) {
+ /* oldcontents is a zero-filled buffer. By
+ * reading into oldcontents, we avoid a rewrite
+ * of identical regions even if an initial full
+ * chip read didn't happen. */
+ msg_gdbg("reading some chunk 0x%06x (0x%06x)\n",
+ start, copy_size);
+ flash->read(flash, oldcontents + start,
+ start,
+ copy_size);
+ }
memcpy(newcontents + start, oldcontents + start,
- entry->start - start);
+ copy_size);
+ }
/* Skip to location after current romentry. */
start = entry->end + 1;
/* Catch overflow. */
--
1.7.1
More information about the flashrom
mailing list