Re: [flashrom] [PATCH] Refactor/fix read-to-file

Acked-By: Stephen Kou

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 [e-mail] 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
 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
> 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;
> }