Print an error message on read errors and abort instead of proceeding anyway. Improve error checking in file write, chip read and chip verify. Refactor the read routines a bit to split reading from file writing.
Parts of this patch were already present in [PATCH] Avoid scary warning if nothing changed
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-read_error_checking/flash.h =================================================================== --- flashrom-read_error_checking/flash.h (Revision 1063) +++ flashrom-read_error_checking/flash.h (Arbeitskopie) @@ -578,7 +578,7 @@ int read_memmapped(struct flashchip *flash, uint8_t *buf, int start, int len); int erase_flash(struct flashchip *flash); struct flashchip *probe_flash(struct flashchip *first_flash, int force); -int read_flash(struct flashchip *flash, char *filename); +int read_flash_to_file(struct flashchip *flash, char *filename); void check_chip_supported(struct flashchip *flash); int check_max_decode(enum chipbustype buses, uint32_t size); int min(int a, int b); Index: flashrom-read_error_checking/cli_classic.c =================================================================== --- flashrom-read_error_checking/cli_classic.c (Revision 1063) +++ flashrom-read_error_checking/cli_classic.c (Arbeitskopie) @@ -418,7 +418,7 @@ exit(1); } printf("Please note that forced reads most likely contain garbage.\n"); - return read_flash(flashes[0], filename); + return read_flash_to_file(flashes[0], filename); } // FIXME: flash writes stay enabled! programmer_shutdown(); Index: flashrom-read_error_checking/flashrom.c =================================================================== --- flashrom-read_error_checking/flashrom.c (Revision 1063) +++ flashrom-read_error_checking/flashrom.c (Arbeitskopie) @@ -673,7 +673,12 @@ starthere = max(start, i * page_size); /* Length of bytes in the range in this page. */ lenhere = min(start + len, (i + 1) * page_size) - starthere; - flash->read(flash, readbuf, starthere, lenhere); + ret = flash->read(flash, readbuf, starthere, lenhere); + if (ret) { + msg_gerr("Verification impossible because read failed " + "at 0x%x (len 0x%x)\n", starthere, lenhere); + break; + } for (j = 0; j < lenhere; j++) { if (cmpbuf[starthere - start + j] != readbuf[j]) { /* Only print the first failure. */ @@ -1024,38 +1029,60 @@ return ret; }
-int read_flash(struct flashchip *flash, char *filename) +int write_buf_to_file(unsigned char *buf, unsigned long size, char *filename) { unsigned long numbytes; FILE *image; - unsigned long size = flash->total_size * 1024; - unsigned char *buf = calloc(size, sizeof(char));
if (!filename) { - msg_gerr("Error: No filename specified.\n"); + msg_gerr("No filename specified.\n"); return 1; } if ((image = fopen(filename, "wb")) == NULL) { perror(filename); - exit(1); + return 1; } - msg_cinfo("Reading flash... "); - if (!flash->read) { - msg_cinfo("FAILED!\n"); - msg_cerr("ERROR: flashrom has no read function for this flash chip.\n"); - return 1; - } else - flash->read(flash, buf, 0, size);
numbytes = fwrite(buf, 1, size, image); fclose(image); - free(buf); - msg_cinfo("%s.\n", numbytes == size ? "done" : "FAILED"); - if (numbytes != size) + if (numbytes != size) { + msg_gerr("File %s could not be written completely.\n", + filename); return 1; + } return 0; }
+int read_flash_to_file(struct flashchip *flash, char *filename) +{ + unsigned long size = flash->total_size * 1024; + unsigned char *buf = calloc(size, sizeof(char)); + int ret = 0; + + msg_cinfo("Reading flash... "); + if (!buf) { + msg_gerr("Memory allocation failed!\n"); + msg_cinfo("FAILED.\n"); + return 1; + } + if (!flash->read) { + msg_cerr("No read function available for this flash chip.\n"); + ret = 1; + goto out_free; + } + if (flash->read(flash, buf, 0, size)) { + msg_cerr("Read operation failed!\n"); + ret = 1; + goto out_free; + } + + ret = write_buf_to_file(buf, flash->total_size * 1024, filename); +out_free: + free(buf); + msg_cinfo("%s.\n", ret ? "done" : "FAILED"); + return ret; +} + /* This function shares a lot of its structure with erase_flash(). * Even if an error is found, the function will keep going and check the rest. */ @@ -1383,7 +1410,7 @@ if (flash->unlock) flash->unlock(flash);
- if (read_flash(flash, filename)) { + if (read_flash_to_file(flash, filename)) { programmer_shutdown(); return 1; }
New version, fixes a logic inversion for status printing.
Print an error message on read errors and abort instead of proceeding anyway. Improve error checking in file write, chip read and chip verify. Refactor the read routines a bit to split reading from file writing.
Parts of this patch were already present in
[PATCH] Avoid scary warning if nothing changed
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-read_error_checking/flash.h =================================================================== --- flashrom-read_error_checking/flash.h (Revision 1078) +++ flashrom-read_error_checking/flash.h (Arbeitskopie) @@ -579,7 +579,7 @@ int read_memmapped(struct flashchip *flash, uint8_t *buf, int start, int len); int erase_flash(struct flashchip *flash); struct flashchip *probe_flash(struct flashchip *first_flash, int force); -int read_flash(struct flashchip *flash, char *filename); +int read_flash_to_file(struct flashchip *flash, char *filename); void check_chip_supported(struct flashchip *flash); int check_max_decode(enum chipbustype buses, uint32_t size); int min(int a, int b); Index: flashrom-read_error_checking/cli_classic.c =================================================================== --- flashrom-read_error_checking/cli_classic.c (Revision 1078) +++ flashrom-read_error_checking/cli_classic.c (Arbeitskopie) @@ -417,7 +417,7 @@ exit(1); } printf("Please note that forced reads most likely contain garbage.\n"); - return read_flash(flashes[0], filename); + return read_flash_to_file(flashes[0], filename); } // FIXME: flash writes stay enabled! programmer_shutdown(); Index: flashrom-read_error_checking/flashrom.c =================================================================== --- flashrom-read_error_checking/flashrom.c (Revision 1078) +++ flashrom-read_error_checking/flashrom.c (Arbeitskopie) @@ -713,7 +713,12 @@ starthere = max(start, i * page_size); /* Length of bytes in the range in this page. */ lenhere = min(start + len, (i + 1) * page_size) - starthere; - flash->read(flash, readbuf, starthere, lenhere); + ret = flash->read(flash, readbuf, starthere, lenhere); + if (ret) { + msg_gerr("Verification impossible because read failed " + "at 0x%x (len 0x%x)\n", starthere, lenhere); + break; + } for (j = 0; j < lenhere; j++) { if (cmpbuf[starthere - start + j] != readbuf[j]) { /* Only print the first failure. */ @@ -1064,38 +1069,60 @@ return ret; }
-int read_flash(struct flashchip *flash, char *filename) +int write_buf_to_file(unsigned char *buf, unsigned long size, char *filename) { unsigned long numbytes; FILE *image; - unsigned long size = flash->total_size * 1024; - unsigned char *buf = calloc(size, sizeof(char));
if (!filename) { - msg_gerr("Error: No filename specified.\n"); + msg_gerr("No filename specified.\n"); return 1; } if ((image = fopen(filename, "wb")) == NULL) { perror(filename); - exit(1); + return 1; } - msg_cinfo("Reading flash... "); - if (!flash->read) { - msg_cinfo("FAILED!\n"); - msg_cerr("ERROR: flashrom has no read function for this flash chip.\n"); - return 1; - } else - flash->read(flash, buf, 0, size);
numbytes = fwrite(buf, 1, size, image); fclose(image); - free(buf); - msg_cinfo("%s.\n", numbytes == size ? "done" : "FAILED"); - if (numbytes != size) + if (numbytes != size) { + msg_gerr("File %s could not be written completely.\n", + filename); return 1; + } return 0; }
+int read_flash_to_file(struct flashchip *flash, char *filename) +{ + unsigned long size = flash->total_size * 1024; + unsigned char *buf = calloc(size, sizeof(char)); + int ret = 0; + + msg_cinfo("Reading flash... "); + if (!buf) { + msg_gerr("Memory allocation failed!\n"); + msg_cinfo("FAILED.\n"); + return 1; + } + if (!flash->read) { + msg_cerr("No read function available for this flash chip.\n"); + ret = 1; + goto out_free; + } + if (flash->read(flash, buf, 0, size)) { + msg_cerr("Read operation failed!\n"); + ret = 1; + goto out_free; + } + + ret = write_buf_to_file(buf, flash->total_size * 1024, filename); +out_free: + free(buf); + msg_cinfo("%s.\n", ret ? "FAILED" : "done"); + return ret; +} + /* This function shares a lot of its structure with erase_flash(). * Even if an error is found, the function will keep going and check the rest. */ @@ -1444,7 +1471,7 @@ if (flash->unlock) flash->unlock(flash);
- if (read_flash(flash, filename)) { + if (read_flash_to_file(flash, filename)) { programmer_shutdown(); return 1; }
Acked-By: Stephen Kou stephen@hyarros.com
Dump to show (expected) failure:
flashrom v0.9.2-r1075 on Linux 2.6.32-23-generic (x86_64), built with libpci 3.0.0, GCC 4.4.3, little endian flashrom is free software, get the source code at http://www.flashrom.org
Calibrating delay loop... OS timer resolution is 1 usecs, 1810M loops per second, 10 myus = 10 us, 100 myus = 100 us, 1000 myus = 992 us, 10000 myus = 9995 us, 4 myus = 4 us, OK. Initializing internal programmer No coreboot table found. DMI string system-manufacturer: " " DMI string system-product-name: " " DMI string system-version: " " DMI string baseboard-manufacturer: "Intel Corporation" DMI string baseboard-product-name: "DX58SO" DMI string baseboard-version: "AAE29331-404" DMI string chassis-type: "Unknown" Found chipset "Intel ICH10R", enabling flash write... chipset PCI ID is 8086:3a16, 0xfff80000/0xffb80000 FWH IDSEL: 0x0 0xfff00000/0xffb00000 FWH IDSEL: 0x0 0xffe80000/0xffa80000 FWH IDSEL: 0x0 0xffe00000/0xffa00000 FWH IDSEL: 0x0 0xffd80000/0xff980000 FWH IDSEL: 0x1 0xffd00000/0xff900000 FWH IDSEL: 0x1 0xffc80000/0xff880000 FWH IDSEL: 0x1 0xffc00000/0xff800000 FWH IDSEL: 0x1 0xff700000/0xff300000 FWH IDSEL: 0x4 0xff600000/0xff200000 FWH IDSEL: 0x5 0xff500000/0xff100000 FWH IDSEL: 0x6 0xff400000/0xff000000 FWH IDSEL: 0x7 0xfff80000/0xffb80000 FWH decode enabled 0xfff00000/0xffb00000 FWH decode enabled 0xffe80000/0xffa80000 FWH decode enabled 0xffe00000/0xffa00000 FWH decode enabled 0xffd80000/0xff980000 FWH decode enabled 0xffd00000/0xff900000 FWH decode enabled 0xffc80000/0xff880000 FWH decode enabled 0xffc00000/0xff800000 FWH decode enabled 0xff700000/0xff300000 FWH decode disabled 0xff600000/0xff200000 FWH decode disabled 0xff500000/0xff100000 FWH decode disabled 0xff400000/0xff000000 FWH decode disabled Maximum FWH chip size: 0x200000 bytes BIOS Lock Enable: enabled, BIOS Write Enable: disabled, BIOS_CNTL is 0xa tried to set 0xdc to 0xb on ICH10R failed (WARNING ONLY)
Root Complex Register Block address = 0xfed1c000 GCS = 0x2e0445: BIOS Interface Lock-Down: enabled, BOOT BIOS Straps: 0x1 (SPI) Top Swap : not enabled SPIBAR = 0xfed1c000 + 0x3800 0x04: 0xe008 (HSFS) FLOCKDN 1, FDV 1, FDOPSS 1, SCIP 0, BERASE 1, AEL 0, FCERR 0, FDONE 0 0x50: 0x00000a0b (FRAP) BMWAG 0x00, BMRAG 0x00, BRWA 0x0a, BRRA 0x0b 0x54: 0x00000000 (FREG0: Flash Descriptor) 0x00000000-0x00000fff is read-only 0x58: 0x01ff0003 (FREG1: BIOS) 0x00003000-0x001fffff is read-write 0x5C: 0x00001fff (FREG2: Management Engine) Management Engine region is unused. 0x60: 0x00020001 (FREG3: Gigabit Ethernet) 0x00001000-0x00002fff is read-write 0x64: 0x00001fff (FREG4: Platform Data) Platform Data region is unused. 0x74: 0x00000000 (PR0) 0x78: 0x00000000 (PR1) 0x7C: 0x00000000 (PR2) 0x80: 0x00000000 (PR3) 0x84: 0x00000000 (PR4) 0x90: 0x00420004 (SSFS, SSFC) 0x94: 0x0006 (PREOP) 0x96: 0x00f0 (OPTYPE) 0x98: 0x2002009f (OPMENU) 0x9C: 0x00000000 (OPMENU+4) 0xA0: 0x00000000 (BBAR) 0xB0: 0x00004000 (FDOC) WARNING: SPI Configuration Lockdown activated. Reading OPCODES... done SPI Read Configuration: prefetching enabled, caching enabled, FAILED! This chipset supports the following protocols: FWH,SPI. Probing for Winbond W25x16, 2048 KB: probe_spi_rdid_generic: id1 0xef, id2 0x3015 Invalid OPCODE 0x05 RDSR failed! Chip status register is ff Found chip "Winbond W25x16" (2048 KB, SPI) at physical address 0xffe00000. === This flash part has status UNTESTED for operations: ERASE WRITE The test status of this chip may have been updated in the latest development version of flashrom. If you are running the latest development version, please email a report to flashrom@flashrom.org if any of the above operations work correctly for you with this flash part. Please include the flashrom output with the additional -V option for all operations you tested (-V, -Vr, -Vw, -VE), and mention which mainboard or programmer you tested. Thanks for your help! === Reading flash... Invalid OPCODE 0x03 Read operation failed! FAILED.
On Wed, 14 Jul 2010 01:13:57 +0200, Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
New version, fixes a logic inversion for status printing.
Print an error message on read errors and abort instead of proceeding anyway. Improve error checking in file write, chip read and chip verify. Refactor the read routines a bit to split reading from file writing.
Parts of this patch were already present in
[PATCH] Avoid scary warning if nothing changed
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-read_error_checking/flash.h
--- flashrom-read_error_checking/flash.h (Revision 1078) +++ flashrom-read_error_checking/flash.h (Arbeitskopie) @@ -579,7 +579,7 @@ int read_memmapped(struct flashchip *flash, uint8_t *buf, int start, int len); int erase_flash(struct flashchip *flash); struct flashchip *probe_flash(struct flashchip *first_flash, int force); -int read_flash(struct flashchip *flash, char *filename); +int read_flash_to_file(struct flashchip *flash, char *filename); void check_chip_supported(struct flashchip *flash); int check_max_decode(enum chipbustype buses, uint32_t size); int min(int a, int b); Index: flashrom-read_error_checking/cli_classic.c =================================================================== --- flashrom-read_error_checking/cli_classic.c (Revision 1078) +++ flashrom-read_error_checking/cli_classic.c (Arbeitskopie) @@ -417,7 +417,7 @@ exit(1); } printf("Please note that forced reads most likely contain garbage.\n");
return read_flash(flashes[0], filename);
} // FIXME: flash writes stay enabled! programmer_shutdown();return read_flash_to_file(flashes[0], filename);
Index: flashrom-read_error_checking/flashrom.c
--- flashrom-read_error_checking/flashrom.c (Revision 1078) +++ flashrom-read_error_checking/flashrom.c (Arbeitskopie) @@ -713,7 +713,12 @@ starthere = max(start, i * page_size); /* Length of bytes in the range in this page. */ lenhere = min(start + len, (i + 1) * page_size) - starthere;
flash->read(flash, readbuf, starthere, lenhere);
ret = flash->read(flash, readbuf, starthere, lenhere);
if (ret) {
msg_gerr("Verification impossible because read failed "
"at 0x%x (len 0x%x)\n", starthere, lenhere);
break;
for (j = 0; j < lenhere; j++) { if (cmpbuf[starthere - start + j] != readbuf[j]) { /* Only print the first failure. */}
@@ -1064,38 +1069,60 @@ return ret; }
-int read_flash(struct flashchip *flash, char *filename) +int write_buf_to_file(unsigned char *buf, unsigned long size, char *filename) { unsigned long numbytes; FILE *image;
unsigned long size = flash->total_size * 1024;
unsigned char *buf = calloc(size, sizeof(char));
if (!filename) {
msg_gerr("Error: No filename specified.\n");
return 1; } if ((image = fopen(filename, "wb")) == NULL) { perror(filename);msg_gerr("No filename specified.\n");
exit(1);
}return 1;
msg_cinfo("Reading flash... ");
if (!flash->read) {
msg_cinfo("FAILED!\n");
msg_cerr("ERROR: flashrom has no read function for this flash chip.\n");
return 1;
} else
flash->read(flash, buf, 0, size);
numbytes = fwrite(buf, 1, size, image); fclose(image);
free(buf);
msg_cinfo("%s.\n", numbytes == size ? "done" : "FAILED");
if (numbytes != size)
- if (numbytes != size) {
msg_gerr("File %s could not be written completely.\n",
return 1;filename);
- } return 0;
}
+int read_flash_to_file(struct flashchip *flash, char *filename) +{
- unsigned long size = flash->total_size * 1024;
- unsigned char *buf = calloc(size, sizeof(char));
- int ret = 0;
- msg_cinfo("Reading flash... ");
- if (!buf) {
msg_gerr("Memory allocation failed!\n");
msg_cinfo("FAILED.\n");
return 1;
- }
- if (!flash->read) {
msg_cerr("No read function available for this flash chip.\n");
ret = 1;
goto out_free;
- }
- if (flash->read(flash, buf, 0, size)) {
msg_cerr("Read operation failed!\n");
ret = 1;
goto out_free;
- }
- ret = write_buf_to_file(buf, flash->total_size * 1024, filename);
+out_free:
- free(buf);
- msg_cinfo("%s.\n", ret ? "FAILED" : "done");
- return ret;
+}
/* This function shares a lot of its structure with erase_flash().
- Even if an error is found, the function will keep going and check
the rest. */ @@ -1444,7 +1471,7 @@ if (flash->unlock) flash->unlock(flash);
if (read_flash(flash, filename)) {
}if (read_flash_to_file(flash, filename)) { programmer_shutdown(); return 1;
On 14.07.2010 01:39, stephen@hyarros.com wrote:
Acked-By: Stephen Kou stephen@hyarros.com
Dump to show (expected) failure:
flashrom v0.9.2-r1075 Calibrating delay loop... OS timer resolution is 1 usecs, 1810M loops per second, 10 myus = 10 us, 100 myus = 100 us, 1000 myus = 992 us, 10000 myus = 9995 us, 4 myus = 4 us, OK. Initializing internal programmer No coreboot table found. DMI string system-manufacturer: " " DMI string system-product-name: " " DMI string system-version: " " DMI string baseboard-manufacturer: "Intel Corporation" DMI string baseboard-product-name: "DX58SO" DMI string baseboard-version: "AAE29331-404" DMI string chassis-type: "Unknown" Found chipset "Intel ICH10R", enabling flash write... chipset PCI ID is 8086:3a16, BIOS Lock Enable: enabled, BIOS Write Enable: disabled, BIOS_CNTL is 0xa tried to set 0xdc to 0xb on ICH10R failed (WARNING ONLY) GCS = 0x2e0445: BIOS Interface Lock-Down: enabled, BOOT BIOS Straps: 0x1 (SPI) WARNING: SPI Configuration Lockdown activated. Reading OPCODES... done SPI Read Configuration: prefetching enabled, caching enabled, FAILED! This chipset supports the following protocols: FWH,SPI. Found chip "Winbond W25x16" (2048 KB, SPI) at physical address 0xffe00000. Reading flash... Invalid OPCODE 0x03 Read operation failed! FAILED.
Thanks, committed in r1079.
Regards, Carl-Daniel