flashrom November 2010

flashrom@flashrom.org

58 participants
146 discussions

[commit] r1224 - trunk
by repository service 04 Nov '10

04 Nov '10

Author: hailfinger Date: Thu Nov 4 02:04:27 2010 New Revision: 1224 URL: http://flashrom.org/trac/flashrom/changeset/1224 Log: This patch makes flashrom use real partial writes. If you write an image full of 0xff, flashrom will erase and detect that no write is needed. If you write an image which differs only in some parts from the current flash contents, flashrom will detect that and not touch unchanged areas. Fix a long-standing bug in need_erase() for 256 byte granularity as well. Nice side benefit: Detailed progress printing. S means skipped E means erased W means written Thanks to Andrew Morgan for testing countless iterations of this patch. Thanks to Richard A. Smith for testing on Dediprog SF100. Thanks to David Hendricks for the review and for creating a partial write torture test script and testing with it on Intel NM10 and AMD SB700 SPI. Thanks to Idwer Vollering for testing with Intel SPI NICs. Thanks to Rudolf Marek for testing on AMD SB710 and SiI SATA controllers. Thanks to Michael Karcher for the review. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Acked-by: David Hendricks <dhendrix(a)google.com> Acked-by: Rudolf Marek <r.marek(a)assembler.cz> Acked-by: Idwer Vollering <vidwer(a)gmail.com> Acked-by: Michael Karcher <flashrom(a)mkarcher.dialup.fu-berlin.de> Modified: trunk/flashrom.c Modified: trunk/flashrom.c ============================================================================== --- trunk/flashrom.c Tue Nov 2 04:12:51 2010 (r1223) +++ trunk/flashrom.c Thu Nov 4 02:04:27 2010 (r1224) @@ -793,6 +793,7 @@ * Check if the buffer @have can be programmed to the content of @want without * erasing. This is only possible if all chunks of size @gran are either kept * as-is or changed from an all-ones state to any other state. + * * The following write granularities (enum @gran) are known: * - 1 bit. Each bit can be cleared individually. * - 1 byte. A byte can be written once. Further writes to an already written @@ -803,10 +804,12 @@ * this function. * - 256 bytes. If less than 256 bytes are written, the contents of the * unwritten bytes are undefined. + * Warning: This function assumes that @have and @want point to naturally + * aligned regions. * * @have buffer with current content * @want buffer with desired content - * @len length of the verified area + * @len length of the checked area * @gran write granularity (enum, not count) * @return 0 if no erase is needed, 1 otherwise */ @@ -838,7 +841,7 @@ continue; /* have needs to be in erased state. */ for (i = 0; i < limit; i++) - if (have[i] != 0xff) { + if (have[j * 256 + i] != 0xff) { result = 1; break; } @@ -846,7 +849,100 @@ break; } break; + default: + msg_cerr("%s: Unsupported granularity! Please report a bug at " + "flashrom(a)flashrom.org\n", __func__); + } + return result; +} + +/** + * Check if the buffer @have needs to be programmed to get the content of @want. + * If yes, return 1 and fill in first_start with the start address of the + * write operation and first_len with the length of the first to-be-written + * chunk. If not, return 0 and leave first_start and first_len undefined. + * + * Warning: This function assumes that @have and @want point to naturally + * aligned regions. + * + * @have buffer with current content + * @want buffer with desired content + * @len length of the checked area + * @gran write granularity (enum, not count) + * @return 0 if no write is needed, 1 otherwise + * @first_start offset of the first byte which needs to be written + * @first_len length of the first contiguous area which needs to be written + * + * FIXME: This function needs a parameter which tells it about coalescing + * in relation to the max write length of the programmer and the max write + * length of the chip. + */ +static int get_next_write(uint8_t *have, uint8_t *want, int len, + int *first_start, int *first_len, + enum write_granularity gran) +{ + int result = 0, rel_start = 0; + int i, limit; + + /* The passed in variable might be uninitialized. */ + *first_len = 0; + switch (gran) { + case write_gran_1bit: + case write_gran_1byte: + for (i = 0; i < len; i++) { + if (have[i] != want[i]) { + if (!result) { + /* First location where have and want + * differ. + */ + result = 1; + rel_start = i; + } + } else { + if (result) { + /* First location where have and want + * do not differ anymore. + */ + *first_len = i - rel_start; + break; + } + } + } + /* Did the loop terminate without setting first_len? */ + if (result && ! *first_len) + *first_len = i - rel_start; + break; + case write_gran_256bytes: + for (i = 0; i < len / 256; i++) { + limit = min(256, len - i * 256); + /* Are 'have' and 'want' identical? */ + if (memcmp(have + i * 256, want + i * 256, limit)) { + if (!result) { + /* First location where have and want + * differ. + */ + result = 1; + rel_start = i * 256; + } + } else { + if (result) { + /* First location where have and want + * do not differ anymore. + */ + *first_len = i * 256 - rel_start; + break; + } + } + } + /* Did the loop terminate without setting first_len? */ + if (result && ! *first_len) + *first_len = min(i * 256 - rel_start, len); + break; + default: + msg_cerr("%s: Unsupported granularity! Please report a bug at " + "flashrom(a)flashrom.org\n", __func__); } + *first_start += rel_start; return result; } @@ -1203,7 +1299,8 @@ return ret; } -/* This function shares a lot of its structure with erase_flash(). +/* This function shares a lot of its structure with erase_and_write_flash() and + * walk_eraseregions(). * Even if an error is found, the function will keep going and check the rest. */ static int selfcheck_eraseblocks(struct flashchip *flash) @@ -1271,10 +1368,67 @@ return ret; } +static int erase_and_write_block_helper(struct flashchip *flash, + unsigned int start, unsigned int len, + uint8_t *oldcontents, + uint8_t *newcontents, + int (*erasefn) (struct flashchip *flash, + unsigned int addr, + unsigned int len)) +{ + int starthere = 0; + int lenhere = 0; + int ret = 0; + int skip = 1; + int writecount = 0; + enum write_granularity gran = write_gran_256bytes; /* FIXME */ + + /* oldcontents and newcontents are opaque to walk_eraseregions, and + * need to be adjusted here to keep the impression of proper abstraction + */ + oldcontents += start; + newcontents += start; + msg_cdbg(":"); + /* FIXME: Assume 256 byte granularity for now to play it safe. */ + if (need_erase(oldcontents, newcontents, len, gran)) { + msg_cdbg("E"); + ret = erasefn(flash, start, len); + if (ret) + return ret; + /* Erase was successful. Adjust oldcontents. */ + memset(oldcontents, 0xff, len); + skip = 0; + } + while (get_next_write(oldcontents + starthere, + newcontents + starthere, + len - starthere, &starthere, + &lenhere, gran)) { + if (!writecount++) + msg_cdbg("W"); + /* Needs the partial write function signature. */ + ret = flash->write(flash, newcontents + starthere, + start + starthere, lenhere); + if (ret) + return ret; + starthere += lenhere; + skip = 0; + } + if (skip) + msg_cdbg("S"); + return ret; +} + static int walk_eraseregions(struct flashchip *flash, int erasefunction, int (*do_something) (struct flashchip *flash, unsigned int addr, - unsigned int len)) + unsigned int len, + uint8_t *param1, + uint8_t *param2, + int (*erasefn) ( + struct flashchip *flash, + unsigned int addr, + unsigned int len)), + void *param1, void *param2) { int i, j; unsigned int start = 0; @@ -1286,21 +1440,34 @@ */ len = eraser.eraseblocks[i].size; for (j = 0; j < eraser.eraseblocks[i].count; j++) { - msg_cdbg("0x%06x-0x%06x, ", start, + /* 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)) + if (do_something(flash, start, len, param1, param2, + eraser.block_erase)) { + msg_cdbg("\n"); return 1; + } start += len; } } + msg_cdbg("\n"); return 0; } -int erase_flash(struct flashchip *flash) +int erase_and_write_flash(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents) { int k, ret = 0, found = 0; + uint8_t *curcontents; + unsigned long size = flash->total_size * 1024; - msg_cinfo("Erasing flash chip... "); + curcontents = (uint8_t *) malloc(size); + /* Copy oldcontents to curcontents to avoid clobbering oldcontents. */ + memcpy(curcontents, oldcontents, size); + + msg_cinfo("Erasing and writing flash chip... "); for (k = 0; k < NUM_ERASEFUNCTIONS; k++) { struct block_eraser eraser = flash->block_erasers[k]; @@ -1324,12 +1491,19 @@ } found = 1; msg_cdbg("trying... "); - ret = walk_eraseregions(flash, k, eraser.block_erase); + ret = walk_eraseregions(flash, k, &erase_and_write_block_helper, curcontents, newcontents); msg_cdbg("\n"); /* If everything is OK, don't try another erase function. */ if (!ret) break; + /* FIXME: Reread the whole chip here so we know the current + * chip contents? curcontents might be up to date, but this + * code is only reached if something failed, and then we don't + * know exactly what failed, and how. + */ } + /* Free the scratchpad. */ + free(curcontents); if (!found) { msg_cerr("ERROR: flashrom has no erase function for this flash chip.\n"); return 1; @@ -1338,7 +1512,7 @@ if (ret) { msg_cerr("FAILED!\n"); } else { - msg_cinfo("SUCCESS.\n"); + msg_cinfo("Done.\n"); } return ret; } @@ -1637,6 +1811,19 @@ programmer_shutdown(); return ret; } + + oldcontents = (uint8_t *) malloc(size); + /* Assume worst case: All bits are 0. */ + memset(oldcontents, 0x00, size); + newcontents = (uint8_t *) malloc(size); + /* Assume best case: All bits should be 1. */ + memset(newcontents, 0xff, size); + /* 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) { /* FIXME: Do we really want the scary warning if erase failed? * After all, after erase the chip is either blank or partially @@ -1644,15 +1831,14 @@ * so if the user wanted erase and reboots afterwards, the user * knows very well that booting won't work. */ - if (erase_flash(flash)) { + if (erase_and_write_flash(flash, oldcontents, newcontents)) { emergency_help_message(); - programmer_shutdown(); - return 1; + ret = 1; } + programmer_shutdown(); + return ret; } - newcontents = (uint8_t *) calloc(size, sizeof(char)); - if (write_it || verify_it) { if (read_buf_from_file(newcontents, size, filename)) { programmer_shutdown(); @@ -1665,8 +1851,6 @@ #endif } - oldcontents = (uint8_t *) calloc(size, sizeof(char)); - /* 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 @@ -1686,27 +1870,9 @@ // //////////////////////////////////////////////////////////// if (write_it) { - if (erase_flash(flash)) { - msg_cerr("Uh oh. Erase failed. Checking if anything " - "changed.\n"); - if (!flash->read(flash, newcontents, 0, size)) { - if (!memcmp(oldcontents, newcontents, size)) { - msg_cinfo("Good. It seems nothing was " - "changed.\n"); - nonfatal_help_message(); - programmer_shutdown(); - return 1; - } - } - emergency_help_message(); - programmer_shutdown(); - return 1; - } - msg_cinfo("Writing flash chip... "); - ret = flash->write(flash, newcontents, 0, size); - if (ret) { - msg_cerr("Uh oh. Write failed. Checking if anything " - "changed.\n"); + if (erase_and_write_flash(flash, oldcontents, newcontents)) { + msg_cerr("Uh oh. Erase/write failed. Checking if " + "anything changed.\n"); if (!flash->read(flash, newcontents, 0, size)) { if (!memcmp(oldcontents, newcontents, size)) { msg_cinfo("Good. It seems nothing was " @@ -1719,8 +1885,6 @@ emergency_help_message(); programmer_shutdown(); return 1; - } else { - msg_cinfo("COMPLETE.\n"); } }

1 0

"flashrom -V on K9N Neo V3
by Иван Касенков 03 Nov '10

03 Nov '10

my output of "flashrom -V"

1 0

"flashrom -V" on "K9N Neo V3"
by Иван Касенков 03 Nov '10

03 Nov '10

my output of "flashrom -V"

1 0

Gigabyte GA-6VXE7 revision 3.0
by Nicholas Adrian 03 Nov '10

03 Nov '10

Your paste can be seen at http://paste.flashrom.org/view.php?id=174 Your upload can be seen at http://paste.flashrom.org/view.php?id=175 This is to verify that erase is working on this board.

1 0

ASRock ALiveXFire-eSATA2 rev 3.0
by Kjell Braden 02 Nov '10

02 Nov '10

Hi, I've got a new ASRock AliveXFire-eSATA2 rev 3.0 board. Chip/chipset are detected fine but erasing doesn't seem to work. Most relevant parts of the logs: Detection: **** snip **** Found chipset "AMD SB600", enabling flash write... OK. This chipset supports the following protocols: LPC,FWH. Found chip "Winbond W39V040(F)C" (512 KB, LPC) at physical address 0xfff80000. === This flash part has status UNTESTED for operations: PROBE READ ERASE WRITE **** snip **** Read/Verify works fine. Erase: **** snip **** Erasing flash chip... Looking at blockwise erase function 0... trying... 0x000000-0x00ffff, ERASE FAILED at 0x00007408! Expected=0xff, Read=0x48, failed byte count from 0x00000000-0x0000ffff: 0x891b ERASE FAILED! Looking at blockwise erase function 1... trying... 0x000000-0x07ffff, ERASE FAILED at 0x00007408! Expected=0xff, Read=0x48, failed byte count from 0x00000000-0x0007ffff: 0x679cd ERASE FAILED! Looking at blockwise erase function 2... not defined. Looking for another erase function. Looking at blockwise erase function 3... not defined. Looking for another erase function. Looking at blockwise erase function 4... not defined. Looking for another erase function. Looking at blockwise erase function 5... not defined. Looking for another erase function. FAILED! **** snip **** Verify succeeds afterwards, the board is still perfectly fine. Let me know if you need any more information or testing. Regards, Kjell

1 0

Re: [flashrom] [v2] DediProg: support for firmware version 5.1.5
by Patrick Georgi 02 Nov '10

02 Nov '10

Hi, the above mentioned patch by Mathias Krause (http://patchwork.coreboot.org/patch/2178/) looks good. I didn't test it, but it's a good improvement over the original, relatively static code to test the device's firmware version. So, Acked-by: Patrick Georgi <patrick(a)georgi-clan.de> Regards, Patrick

1 0

PSA (Public Service Announcement): Please test partial writes.
by David Hendricks 02 Nov '10

02 Nov '10

Hello everyone, Carl-Daniel recently posted what will likely be the final patch in the "Real partial writes" thread. The patch has many benefits, but also carries some risk. There's a fair amount of core logic being changed and we'd really appreciate any/all testing and code reviews. If you're feeling up for it, please head over to http://patchwork.coreboot.org/project/flashrom/list/ and grab the latest "Real partial writes" patch! We now have test script in the util/ directory that you may use to automatically test partial writes. Simple usage is as easy as building flashrom with the patch, cd'ing to the util/ directory, and running the flashrom_partial_write_test.sh script. The patch should go in soon, so it's important to find any show stoppers. Heck, even after the patch goes in it would be great for folks to run the script and let us know if anything looks suspicious. Thanks, and sorry for the PSA spam! -- David Hendricks (dhendrix) Systems Software Engineer, Google Inc.

1 0

[PATCH] Change image building in layout code
by Carl-Daniel Hailfinger 02 Nov '10

02 Nov '10

Change semantics of image building in the layout code. If a layout file was specified, all regions not mentioned in the layout file were taken from the new image instead of being preserved. If regions overlap, the non-included regions won. New behaviour: If a layout file is specified, only the regions explicitly requested for inclusion will be taken from the new image. If regions overlap, the included regions win. The following alternatives exist: - Warn if regions overlap - Warn if a layout file does not cover the whole chip Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Index: flashrom-layout_exclude_unspecified_regions/layout.c =================================================================== --- flashrom-layout_exclude_unspecified_regions/layout.c (revision 1217) +++ flashrom-layout_exclude_unspecified_regions/layout.c (working copy) @@ -22,6 +22,7 @@ #include <stdlib.h> #include <string.h> #include <ctype.h> +#include <limits.h> #include "flash.h" #include "programmer.h" @@ -205,33 +206,62 @@ return -1; } -int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents) +int find_next_included_romentry(unsigned int start) { int i; + unsigned int best_start = INT_MAX; + int best_entry = -1; - // This function does not save flash write cycles. - // - // Also it does not cope with overlapping rom layout - // sections. - // example: - // 00000000:00008fff gfxrom - // 00009000:0003ffff normal - // 00040000:0007ffff fallback - // 00000000:0007ffff all - // - // If you'd specify -i all the included flag of all other - // sections is still 0, so no changes will be made to the - // flash. Same thing if you specify -i normal -i all only - // normal will be updated and the rest will be kept. - + /* First come, first serve for overlapping regions. */ for (i = 0; i < romimages; i++) { - if (rom_entries[i].included) + if (!rom_entries[i].included) continue; + /* Already past the current entry? */ + if (start > rom_entries[i].end) + continue; + /* Inside the current entry? */ + if (start >= rom_entries[i].start) + return i; + /* Entry begins after start. */ + if (best_start > rom_entries[i].start) { + best_start = rom_entries[i].start; + best_entry = i; + } + } + return best_entry; +} - memcpy(newcontents + rom_entries[i].start, - oldcontents + rom_entries[i].start, - rom_entries[i].end - rom_entries[i].start + 1); +int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents) +{ + unsigned int start = 0; + int entry; + unsigned int size = flash->total_size * 1024; + + /* If no layout file was specified or the layout file was empty, assume + * that the user wants to flash the complete new image. + */ + if (!romimages) + return 0; + /* Non-included romentries are ignored. + * The union of all included romentries is used from the new image. + */ + while (start < size) { + entry = find_next_included_romentry(start); + /* No more romentries for remaining region? */ + if (entry < 0) { + memcpy(newcontents + start, oldcontents + start, + size - start); + break; + } + if (rom_entries[entry].start > start) + memcpy(newcontents + start, oldcontents + start, + rom_entries[entry].start - start); + /* Skip to location after current romentry. */ + start = rom_entries[entry].end + 1; + /* Catch overflow. */ + if (!start) + break; } - + return 0; } -- http://www.hailfinger.org/

2 2

[commit] r1223 - trunk
by repository service 02 Nov '10

02 Nov '10

Author: hailfinger Date: Tue Nov 2 04:12:51 2010 New Revision: 1223 URL: http://flashrom.org/trac/flashrom/changeset/1223 Log: Change semantics of image building in the layout code. If a layout file was specified, all regions not mentioned in the layout file were taken from the new image instead of being preserved. If regions overlap, the non-included regions won. New behaviour: If a layout file is specified, only the regions explicitly requested for inclusion will be taken from the new image. If regions overlap, the included regions win. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Acked-by: David Hendricks <dhendrix(a)google.com> Modified: trunk/layout.c Modified: trunk/layout.c ============================================================================== --- trunk/layout.c Tue Nov 2 04:03:38 2010 (r1222) +++ trunk/layout.c Tue Nov 2 04:12:51 2010 (r1223) @@ -22,6 +22,7 @@ #include <stdlib.h> #include <string.h> #include <ctype.h> +#include <limits.h> #include "flash.h" #include "programmer.h" @@ -205,33 +206,62 @@ return -1; } -int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents) +int find_next_included_romentry(unsigned int start) { int i; + unsigned int best_start = UINT_MAX; + int best_entry = -1; - // This function does not save flash write cycles. - // - // Also it does not cope with overlapping rom layout - // sections. - // example: - // 00000000:00008fff gfxrom - // 00009000:0003ffff normal - // 00040000:0007ffff fallback - // 00000000:0007ffff all - // - // If you'd specify -i all the included flag of all other - // sections is still 0, so no changes will be made to the - // flash. Same thing if you specify -i normal -i all only - // normal will be updated and the rest will be kept. - + /* First come, first serve for overlapping regions. */ for (i = 0; i < romimages; i++) { - if (rom_entries[i].included) + if (!rom_entries[i].included) continue; - - memcpy(newcontents + rom_entries[i].start, - oldcontents + rom_entries[i].start, - rom_entries[i].end - rom_entries[i].start + 1); + /* Already past the current entry? */ + if (start > rom_entries[i].end) + continue; + /* Inside the current entry? */ + if (start >= rom_entries[i].start) + return i; + /* Entry begins after start. */ + if (best_start > rom_entries[i].start) { + best_start = rom_entries[i].start; + best_entry = i; + } } + return best_entry; +} + +int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents) +{ + unsigned int start = 0; + int entry; + unsigned int size = flash->total_size * 1024; + /* If no layout file was specified or the layout file was empty, assume + * that the user wants to flash the complete new image. + */ + if (!romimages) + return 0; + /* Non-included romentries are ignored. + * The union of all included romentries is used from the new image. + */ + while (start < size) { + entry = find_next_included_romentry(start); + /* No more romentries for remaining region? */ + if (entry < 0) { + memcpy(newcontents + start, oldcontents + start, + size - start); + break; + } + if (rom_entries[entry].start > start) + memcpy(newcontents + start, oldcontents + start, + rom_entries[entry].start - start); + /* Skip to location after current romentry. */ + start = rom_entries[entry].end + 1; + /* Catch overflow. */ + if (!start) + break; + } + return 0; }

1 0

[patch] print test status in partial write test script
by David Hendricks 02 Nov '10

02 Nov '10

We were a bit too aggressive in trimming superfluous print messages earlier on. This patch adds a line of output for each iteration of the while loops in the test. The intention is to give the user something useful to indicate test progress without just printing a ton of debug spew. Maybe a future patch can redirect more verbose info to a log file. The output looks like this:: localhost ~ # FLASHROM="./flashrom" sh flashrom_partial_write_test.sh testing flashrom binary: ./flashrom Running test in /tmp/tmp.4xPejwaADU ffh pattern written in ff_4k.bin 00h pattern written in 00_4k.bin Reading BIOS image Original image saved as bios.bin aligned region 0 test: passed ... aligned region 15 test: passed unaligned region 0 test: passed ... unaligned region 15 test: passed Result: PASSED restoring original bios image using system's flashrom test files remain in /tmp/tmp.4xPejwaADU Signed-off-by: David Hendricks <dhendrix(a)google.com> -- David Hendricks (dhendrix) Systems Software Engineer, Google Inc.

2 3

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

flashrom November 2010