flashrom

flashrom@flashrom.org

5 participants
9161 discussions

Start a nNew thread

support
by seevkaboorka _ 06 Nov '10

06 Nov '10

subj :)

1 0

[PATCH] Partial write cleanups
by Carl-Daniel Hailfinger 05 Nov '10

05 Nov '10

Avoid two memory leaks in doit() which were unproblematic for flashrom because flashrom terminates after finishing doit(). Rename oldcontents to curconents in erase_and_write_block_helper(). Return write length from get_next_write() instead of filling it as referenced parameter. Thanks to Michael Karcher for pointing out the first two issues. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> diff -u flashrom-partial_write_rolling_erase_write/flashrom.c flashrom-partial_write_rolling_erase_write_cleanup/flashrom.c --- flashrom-partial_write_rolling_erase_write/flashrom.c (Arbeitskopie) +++ flashrom-partial_write_rolling_erase_write_cleanup/flashrom.c (Arbeitskopie) @@ -878,14 +878,11 @@ * 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 *first_start, enum write_granularity gran) { - int result = 0, rel_start = 0; + int result = 0, rel_start = 0, first_len = 0; int i, limit; - /* The passed in variable might be uninitialized. */ - *first_len = 0; switch (gran) { case write_gran_1bit: case write_gran_1byte: @@ -903,14 +900,14 @@ /* First location where have and want * do not differ anymore. */ - *first_len = i - rel_start; + first_len = i - rel_start; break; } } } /* Did the loop terminate without setting first_len? */ - if (result && ! *first_len) - *first_len = i - rel_start; + if (result && ! first_len) + first_len = i - rel_start; break; case write_gran_256bytes: for (i = 0; i < len / 256; i++) { @@ -929,21 +926,21 @@ /* First location where have and want * do not differ anymore. */ - *first_len = i * 256 - rel_start; + 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); + 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; + return first_len; } /* This function generates various test patterns useful for testing controller @@ -1370,7 +1367,7 @@ static int erase_and_write_block_helper(struct flashchip *flash, unsigned int start, unsigned int len, - uint8_t *oldcontents, + uint8_t *curcontents, uint8_t *newcontents, int (*erasefn) (struct flashchip *flash, unsigned int addr, @@ -1383,26 +1380,25 @@ int writecount = 0; enum write_granularity gran = write_gran_256bytes; /* FIXME */ - /* oldcontents and newcontents are opaque to walk_eraseregions, and + /* curcontents and newcontents are opaque to walk_eraseregions, and * need to be adjusted here to keep the impression of proper abstraction */ - oldcontents += start; + curcontents += start; newcontents += start; msg_cdbg(":"); /* FIXME: Assume 256 byte granularity for now to play it safe. */ - if (need_erase(oldcontents, newcontents, len, gran)) { + if (need_erase(curcontents, newcontents, len, gran)) { msg_cdbg("E"); ret = erasefn(flash, start, len); if (ret) return ret; - /* Erase was successful. Adjust oldcontents. */ - memset(oldcontents, 0xff, len); + /* Erase was successful. Adjust curcontents. */ + memset(curcontents, 0xff, len); skip = 0; } - while (get_next_write(oldcontents + starthere, - newcontents + starthere, - len - starthere, &starthere, - &lenhere, gran)) { + while ((lenhere = get_next_write(curcontents + starthere, + newcontents + starthere, + len - starthere, &starthere, gran))) { if (!writecount++) msg_cdbg("W"); /* Needs the partial write function signature. */ @@ -1796,8 +1792,8 @@ if (chip_safety_check(flash, force, filename, read_it, write_it, erase_it, verify_it)) { msg_cerr("Aborting.\n"); - programmer_shutdown(); - return 1; + ret = 1; + goto out_nofree; } /* Given the existence of read locks, we want to unlock for read, @@ -1808,8 +1804,7 @@ if (read_it) { ret = read_flash_to_file(flash, filename); - programmer_shutdown(); - return ret; + goto out_nofree; } oldcontents = (uint8_t *) malloc(size); @@ -1835,14 +1830,13 @@ emergency_help_message(); ret = 1; } - programmer_shutdown(); - return ret; + goto out; } if (write_it || verify_it) { if (read_buf_from_file(newcontents, size, filename)) { - programmer_shutdown(); - return 1; + ret = 1; + goto out; } #if CONFIG_INTERNAL == 1 @@ -1859,8 +1853,8 @@ */ msg_cdbg("Reading old flash chip contents...\n"); if (flash->read(flash, oldcontents, 0, size)) { - programmer_shutdown(); - return 1; + ret = 1; + goto out; } // This should be moved into each flash part's code to do it @@ -1878,13 +1872,13 @@ msg_cinfo("Good. It seems nothing was " "changed.\n"); nonfatal_help_message(); - programmer_shutdown(); - return 1; + ret = 1; + goto out; } } emergency_help_message(); - programmer_shutdown(); - return 1; + ret = 1; + goto out; } } @@ -1901,6 +1895,9 @@ } +out: + free(oldcontents); + free(newcontents); +out_nofree: programmer_shutdown(); - return ret; } -- http://www.hailfinger.org/

2 4

[commit] r1225 - trunk
by repository service 05 Nov '10

05 Nov '10

Author: hailfinger Date: Fri Nov 5 15:51:59 2010 New Revision: 1225 URL: http://flashrom.org/trac/flashrom/changeset/1225 Log: Avoid two memory leaks in doit() which were unproblematic for flashrom because flashrom terminates after finishing doit(). Rename oldcontents to curconents in erase_and_write_block_helper(). Unify the code for all granularities in get_next_write(). Return write length from get_next_write() instead of filling it as referenced parameter. Thanks to Michael Karcher for pointing out the first two issues. Thanks to David Hendricks for pointing out the third issue and suggesting a way to unify that code. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Acked-by: Michael Karcher <flashrom(a)mkarcher.dialup.fu-berlin.de> Modified: trunk/flashrom.c Modified: trunk/flashrom.c ============================================================================== --- trunk/flashrom.c Thu Nov 4 02:04:27 2010 (r1224) +++ trunk/flashrom.c Fri Nov 5 15:51:59 2010 (r1225) @@ -869,81 +869,62 @@ * @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 + * @first_start offset of the first byte which needs to be written (passed in + * value is increased by the offset of the first needed write + * relative to have/want or unchanged if no write is needed) + * @return length of the first contiguous area which needs to be written + * 0 if no write is needed * * 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 *first_start, enum write_granularity gran) { - int result = 0, rel_start = 0; - int i, limit; + int need_write = 0, rel_start = 0, first_len = 0; + int i, limit, stride; - /* 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; + stride = 1; 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); + stride = 256; break; default: msg_cerr("%s: Unsupported granularity! Please report a bug at " "flashrom(a)flashrom.org\n", __func__); + /* Claim that no write was needed. A write with unknown + * granularity is too dangerous to try. + */ + return 0; + } + for (i = 0; i < len / stride; i++) { + limit = min(stride, len - i * stride); + /* Are 'have' and 'want' identical? */ + if (memcmp(have + i * stride, want + i * stride, limit)) { + if (!need_write) { + /* First location where have and want differ. */ + need_write = 1; + rel_start = i * stride; + } + } else { + if (need_write) { + /* First location where have and want + * do not differ anymore. + */ + first_len = i * stride - rel_start; + break; + } + } } + /* Did the loop terminate without setting first_len? */ + if (need_write && ! first_len) + first_len = min(i * stride - rel_start, len); *first_start += rel_start; - return result; + return first_len; } /* This function generates various test patterns useful for testing controller @@ -1370,7 +1351,7 @@ static int erase_and_write_block_helper(struct flashchip *flash, unsigned int start, unsigned int len, - uint8_t *oldcontents, + uint8_t *curcontents, uint8_t *newcontents, int (*erasefn) (struct flashchip *flash, unsigned int addr, @@ -1383,26 +1364,26 @@ int writecount = 0; enum write_granularity gran = write_gran_256bytes; /* FIXME */ - /* oldcontents and newcontents are opaque to walk_eraseregions, and + /* curcontents and newcontents are opaque to walk_eraseregions, and * need to be adjusted here to keep the impression of proper abstraction */ - oldcontents += start; + curcontents += start; newcontents += start; msg_cdbg(":"); /* FIXME: Assume 256 byte granularity for now to play it safe. */ - if (need_erase(oldcontents, newcontents, len, gran)) { + if (need_erase(curcontents, newcontents, len, gran)) { msg_cdbg("E"); ret = erasefn(flash, start, len); if (ret) return ret; - /* Erase was successful. Adjust oldcontents. */ - memset(oldcontents, 0xff, len); + /* Erase was successful. Adjust curcontents. */ + memset(curcontents, 0xff, len); skip = 0; } - while (get_next_write(oldcontents + starthere, - newcontents + starthere, - len - starthere, &starthere, - &lenhere, gran)) { + /* get_next_write() sets starthere to a new value after the call. */ + while ((lenhere = get_next_write(curcontents + starthere, + newcontents + starthere, + len - starthere, &starthere, gran))) { if (!writecount++) msg_cdbg("W"); /* Needs the partial write function signature. */ @@ -1796,8 +1777,8 @@ if (chip_safety_check(flash, force, filename, read_it, write_it, erase_it, verify_it)) { msg_cerr("Aborting.\n"); - programmer_shutdown(); - return 1; + ret = 1; + goto out_nofree; } /* Given the existence of read locks, we want to unlock for read, @@ -1808,8 +1789,7 @@ if (read_it) { ret = read_flash_to_file(flash, filename); - programmer_shutdown(); - return ret; + goto out_nofree; } oldcontents = (uint8_t *) malloc(size); @@ -1835,14 +1815,13 @@ emergency_help_message(); ret = 1; } - programmer_shutdown(); - return ret; + goto out; } if (write_it || verify_it) { if (read_buf_from_file(newcontents, size, filename)) { - programmer_shutdown(); - return 1; + ret = 1; + goto out; } #if CONFIG_INTERNAL == 1 @@ -1859,8 +1838,8 @@ */ msg_cdbg("Reading old flash chip contents...\n"); if (flash->read(flash, oldcontents, 0, size)) { - programmer_shutdown(); - return 1; + ret = 1; + goto out; } // This should be moved into each flash part's code to do it @@ -1878,13 +1857,13 @@ msg_cinfo("Good. It seems nothing was " "changed.\n"); nonfatal_help_message(); - programmer_shutdown(); - return 1; + ret = 1; + goto out; } } emergency_help_message(); - programmer_shutdown(); - return 1; + ret = 1; + goto out; } } @@ -1900,7 +1879,10 @@ emergency_help_message(); } +out: + free(oldcontents); + free(newcontents); +out_nofree: programmer_shutdown(); - return ret; }

1 0

info test run for socket 939 board mATX MSI 6100-939
by M Capoferri 05 Nov '10

05 Nov '10

Hi, I installed flashrom last night and ran the plain command and one with -V just to see what it dumped. I'm a non-pro and was forced into learning linux after having all of my machines infected by an eeprom based rootkit four years ago. It infected eight new motherboards and a few old machines before I gave up (read went broke!) and mutilple peripherals. I was unable to connect to the net between Aug 2006 and Jan 2009. I was finally able to reconnect after getting OpenSUSE 10.3 to sucessfully install to a hard drive (coincidently, the same time the new linux module, Policykit started trials I believe). Failed after 4 weeks but was able to maintain connection using live distros. I remain infected to this day (only partial root control) as some kind of minimal kernal or control framework using busybox is still in my eeprom and hooks control of all hardware. This prevented me from reflashing my BIOS on any of the boards and believe me, i tried multiple times. I'm hoping to try and reflash this or one of my three Asus boards that are on the Coreboot approved list. I'll let you know if i succeed, if I can ever figure out what I'm doing. I'm 56, so it's a little tougher especially as I was never in IT until I started building my own machines a couple of years before the malware hit. I've attached the terminal out as both pdf and txt file (8859-1), and I guess I don't have to warn you to scan them, though i am using gmail. Cheers, Mark Capoferri Ardmore, Pennsylvania US

2 1

flashrom for Win Ent MB-7305
by Anish Patel 05 Nov '10

05 Nov '10

http://paste.flashrom.org/view.php?id=186 http://paste.flashrom.org/view.php?id=187 http://paste.flashrom.org/view.php?id=188 http://paste.flashrom.org/view.php?id=189

1 0

Asus M2N
by Miguel Angel GSM 04 Nov '10

04 Nov '10

I hope that this helps. I have one question. Is it safe for me to write the much newer version 0808 of the BIOS (that I downloaded from ASUS) with my actual settings? I ask because of the following sentence while running flashrom: "Found chipset "NVIDIA MCP61", enabling flash write... chipset PCI ID is 10de:03e0, This chipset is not really supported yet. Guesswork..." Thanks in advance for your answer, Miguel A non-text attachment has been stripped: name="flashrom-r.0408.rom" It is stored at http://paste.flashrom.org/view.php?id=184 A non-text attachment has been stripped: name="asus0408.rom" It is stored at http://paste.flashrom.org/view.php?id=185

2 1

[PATCH] Real partial writes
by Carl-Daniel Hailfinger 04 Nov '10

04 Nov '10

If you're going to review this, make sure you keep a stack of bananas (quickly mobilized carbohydrates for your brain), a bucket of ice (to prevent brain overheating) and a bottle of aspirin handy. If any code is unclear, please tell me and I'll try to add comments to improve readability. If you're going to test this, please be advised that I have not tested execution, only compilation. Testing erase (and checking with a separate readback that erase actually worked) and write (same test with separate readback) would be highly appreciated. Verbose logs are even more appreciated. 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. Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006(a)gmx.net> Index: flashrom-partial_write_rolling_erase_write/flashrom.c =================================================================== --- flashrom-partial_write_rolling_erase_write/flashrom.c (Revision 1215) +++ flashrom-partial_write_rolling_erase_write/flashrom.c (Arbeitskopie) @@ -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 */ @@ -846,10 +849,102 @@ 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; + int i, j, 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; + *first_start = i; + } + } else { + if (result) { + /* First location where have and want + * do not differ anymore. + */ + *first_len = i - *first_start; + break; + } + } + } + /* Did the loop terminate without setting first_len? */ + if (result && ! *first_len) + *first_len = i - *first_start; + break; + case write_gran_256bytes: + for (j = 0; j < len / 256; j++) { + limit = min(256, len - j * 256); + /* Are 'have' and 'want' identical? */ + if (memcmp(have + j * 256, want + j * 256, limit)) { + if (!result) { + /* First location where have and want + * differ. + */ + result = 1; + *first_start = j * 256; + } + } else { + if (result) { + /* First location where have and want + * do not differ anymore. + */ + *first_len = j * 256 - *first_start; + break; + } + } + } + /* Did the loop terminate without setting first_len? */ + if (result && ! *first_len) + *first_len = min(j * 256 - *first_start, len); + break; + default: + msg_cerr("%s: Unsupported granularity! Please report a bug at " + "flashrom(a)flashrom.org\n", __func__); + } + return result; +} + /* This function generates various test patterns useful for testing controller * and chip communication as well as chip behaviour. * @@ -1203,7 +1298,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 +1367,58 @@ 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; + 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; + /* FIXME: Assume 256 byte granularity for now to play it safe. */ + if (need_erase(oldcontents, + newcontents, + len, gran)) { + ret = erasefn(flash, start, len); + if (ret) + return ret; + /* Erase was successful. Adjust oldcontents. */ + memset(oldcontents + start, 0xff, len); + } + while (get_next_write(oldcontents + starthere, + newcontents + starthere, + len - starthere, &starthere, + &lenhere, gran)) { + /* Needs the partial write function signature. */ + ret = flash->write(flash, newcontents + starthere, start + starthere, lenhere); + if (ret) + break; + starthere += lenhere; + } + 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; @@ -1288,7 +1432,7 @@ for (j = 0; j < eraser.eraseblocks[i].count; j++) { 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)) return 1; start += len; } @@ -1296,11 +1440,11 @@ 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; - msg_cinfo("Erasing flash chip... "); + msg_cinfo("Erasing and writing flash chip... "); for (k = 0; k < NUM_ERASEFUNCTIONS; k++) { struct block_eraser eraser = flash->block_erasers[k]; @@ -1324,7 +1468,7 @@ } found = 1; msg_cdbg("trying... "); - ret = walk_eraseregions(flash, k, eraser.block_erase); + ret = walk_eraseregions(flash, k, &erase_and_write_block_helper, oldcontents, newcontents); msg_cdbg("\n"); /* If everything is OK, don't try another erase function. */ if (!ret) @@ -1637,6 +1781,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 +1801,13 @@ * 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; } } - newcontents = (uint8_t *) calloc(size, sizeof(char)); - if (write_it || verify_it) { if (read_buf_from_file(newcontents, size, filename)) { programmer_shutdown(); @@ -1665,8 +1820,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,9 +1839,9 @@ // //////////////////////////////////////////////////////////// if (write_it) { - if (erase_flash(flash)) { - msg_cerr("Uh oh. Erase 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 " @@ -1701,24 +1854,6 @@ 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 (!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; } else { msg_cinfo("COMPLETE.\n"); } -- http://www.hailfinger.org/

7 30

[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 Иван Касенков 04 Nov '10

04 Nov '10

my output of "flashrom -V"

1 0

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

04 Nov '10

my output of "flashrom -V"

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

flashrom