Re: [flashrom] [PATCH] dummyflasher.c: add support for SFDP by adding a new emulator chip: W25Q64CV
Am 20.02.2012 17:07 schrieb Stefan Tauner:
TODO:
- how should the SFDP data be supplied/selected by the user?
- option A (suggested one): add a default table with a legit complete table
Good idea.
and a programmer option to use a binary file instead.
I think having the file+builtin combination is overkill. Builtin should be sufficient, unless you plan to focus more on making flashrom a verification tool for flash vendors.
- Manpage
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
dummyflasher.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 82 insertions(+), 8 deletions(-)
diff --git a/dummyflasher.c b/dummyflasher.c index afe0518..6d8b9a2 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -45,6 +45,7 @@ enum emu_chip { EMULATE_ST_M25P10_RES, EMULATE_SST_SST25VF040_REMS, EMULATE_SST_SST25VF032B,
- EMULATE_WINBOND_W25Q64CV,
}; static enum emu_chip emu_chip = EMULATE_NONE; static char *emu_persistent_image = NULL; @@ -61,6 +62,40 @@ unsigned char spi_blacklist[256]; unsigned char spi_ignorelist[256]; int spi_blacklist_size = 0; int spi_ignorelist_size = 0;
+/* legit intel version */ +/* +static const uint8_t const sfdp_table[256] = {
- 0x53, 0x46, 0x44, 0x50, // @0x00
- 0x00, 0x01, 0x00, 0xFF, // @0x04
- 0x00, 0x00, 0x01, 0x04, // @0x08: len = 4 instead of 9
- 0x14, 0x00, 0x00, 0xFF, // @0x0C: PTP0 = 0x14 instead of 0x80
- 0xFF, 0xFF, 0xFF, 0xFF, // @0x10
- 0xE5, 0x20, 0xF1, 0xFF, // @0x14
- 0xFF, 0xFF, 0xFF, 0x03, // @0x18
- 0x44, 0xEB, 0x08, 0x6B, // @0x1C
- 0x08, 0x3B, 0x80, 0xBB, // @0x20
+}; +*/ +/* legit complete table */ +static const uint8_t const sfdp_table[256] = {
- 0x53, 0x46, 0x44, 0x50, // @0x00
- 0x00, 0x01, 0x00, 0xFF, // @0x04
- 0x00, 0x00, 0x01, 0x09, // @0x08
- 0x14, 0x00, 0x00, 0xFF, // @0x0C: PTP0 = 0x14 instead of 0x80
- 0xFF, 0xFF, 0xFF, 0xFF, // @0x10
- 0xE5, 0x20, 0xF1, 0xFF, // @0x14
- 0xFF, 0xFF, 0xFF, 0x03, // @0x18
- 0x44, 0xEB, 0x08, 0x6B, // @0x1C
- 0x08, 0x3B, 0x80, 0xBB, // @0x20
- 0xEE, 0xFF, 0xFF, 0xFF, // @0x24
- 0xFF, 0xFF, 0x00, 0x00, // @0x28
- 0xFF, 0xFF, 0x00, 0x00, // @0x2C
- 0x0C, 0x20, 0x0F, 0x52, // @0x30
- 0x10, 0xD8, 0x00, 0x00, // @0x34
+};
#endif #endif
@@ -296,6 +331,19 @@ int dummy_init(void) msg_pdbg("Emulating SST SST25VF032B SPI flash chip (RDID, AAI " "write)\n"); }
- if (!strcmp(tmp, "W25Q64CV")) {
emu_chip = EMULATE_WINBOND_W25Q64CV;emu_chip_size = 8 * 1024 * 1024;emu_max_byteprogram_size = 256;emu_max_aai_size = 0;emu_jedec_se_size = 4 * 1024;emu_jedec_be_52_size = 32 * 1024;emu_jedec_be_d8_size = 64 * 1024;emu_jedec_ce_60_size = emu_chip_size;emu_jedec_ce_c7_size = emu_chip_size;msg_pdbg("Emulating Winbond W25Q64CV SPI flash chip (RDID, ""SFDP)\n");- }
#endif if (emu_chip == EMULATE_NONE) { msg_perr("Invalid chip specified for emulation: %s\n", tmp); @@ -471,15 +519,26 @@ static int emulate_spi_chip_response(unsigned int writecnt, readarr[1] = 0x44; break; case JEDEC_RDID:
if (emu_chip != EMULATE_SST_SST25VF032B)
switch (emu_chip) {case EMULATE_SST_SST25VF032B:if (readcnt > 0)readarr[0] = 0xbf;if (readcnt > 1)readarr[1] = 0x25;if (readcnt > 2)readarr[2] = 0x4a; break;
/* Respond with SST_SST25VF032B. */if (readcnt > 0)readarr[0] = 0xbf;if (readcnt > 1)readarr[1] = 0x25;if (readcnt > 2)readarr[2] = 0x4a;
case EMULATE_WINBOND_W25Q64CV:if (readcnt > 0)readarr[0] = 0xef;if (readcnt > 1)readarr[1] = 0x40;if (readcnt > 2)readarr[2] = 0x17;break;default: /* ignore */break;}@@ -629,6 +688,20 @@ static int emulate_spi_chip_response(unsigned int writecnt, /* emu_jedec_ce_c7_size is emu_chip_size. */ memset(flashchip_contents, 0xff, emu_jedec_ce_c7_size); break;
- case JEDEC_SFDP:
if (emu_chip != EMULATE_WINBOND_W25Q64CV)break;offs = writearr[1] << 16 | writearr[2] << 8 | writearr[3];/** FIXME: There is one dummy byte (i.e. 8 clock cycles) to be* transferred after the address. Since we can not observe the* clock, we would need to check for appropriate writecnt and/or* readcnt and recalculate the parameters below.*//* FIXME: this could be more sophisticated. */memcpy(readarr, sfdp_table + offs,min(sizeof(sfdp_table) - offs, readcnt));
That memcpy will segfault if offs>sizeof(sfdp_table). Suggestion: Replace the whole case statement with this (some 80 col reformatting may be needed):
case JEDEC_SFDP: int toread; if (emu_chip != EMULATE_WINBOND_W25Q64CV) break; if (writecnt < 4) break; offs = writearr[1] << 16 | writearr[2] << 8 | writearr[3]; /* The response is shifted if more than 4 bytes are written. */ offs += writecnt - 4; /* The SFDP spec suggests wraparound is allowed. */ offs %= sizeof(sfdp_table); toread = min(sizeof(sfdp_table) - offs, readcnt); memcpy(readarr, sfdp_table + offs, toread); if (toread < readcnt) memcpy(readarr + toread, sfdp_table, min(sizeof(sfdp_table),readcnt - toread)); if (readcnt - toread > sizeof(sfdp_table)) msg_pdbg("Reading more than SFDP table size in one " "chunk, contents may be incomplete.\n");
break;@@ -657,6 +730,7 @@ static int dummy_spi_send_command(struct flashctx *flash, unsigned int writecnt, case EMULATE_ST_M25P10_RES: case EMULATE_SST_SST25VF040_REMS: case EMULATE_SST_SST25VF032B:
- case EMULATE_WINBOND_W25Q64CV: if (emulate_spi_chip_response(writecnt, readcnt, writearr, readarr)) { msg_pdbg("Invalid command sent to flash chip!\n");
Regards, Carl-Daniel
The chip features a complete 1.0 SFDP JEDEC flash parameter table and also a vendor-specific extension table (defining voltages, lock bits etc). NB: the MX25L6436 uses the same RDID as the MX25L6405.
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at --- dummyflasher.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++++++----- flashrom.8 | 2 + 2 files changed, 90 insertions(+), 9 deletions(-)
On Mon, 20 Feb 2012 19:49:38 +0100 Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net wrote:
Am 20.02.2012 17:07 schrieb Stefan Tauner:
TODO:
- how should the SFDP data be supplied/selected by the user?
- option A (suggested one): add a default table with a legit complete table
Good idea.
and a programmer option to use a binary file instead.I think having the file+builtin combination is overkill. Builtin should be sufficient, unless you plan to focus more on making flashrom a verification tool for flash vendors.
some of them would need it. *cough* ;)
[…]
- case JEDEC_SFDP:
if (emu_chip != EMULATE_WINBOND_W25Q64CV)break;offs = writearr[1] << 16 | writearr[2] << 8 | writearr[3];/** FIXME: There is one dummy byte (i.e. 8 clock cycles) to be* transferred after the address. Since we can not observe the* clock, we would need to check for appropriate writecnt and/or* readcnt and recalculate the parameters below.*//* FIXME: this could be more sophisticated. */memcpy(readarr, sfdp_table + offs,min(sizeof(sfdp_table) - offs, readcnt));That memcpy will segfault if offs>sizeof(sfdp_table). Suggestion: Replace the whole case statement with this (some 80 col reformatting may be needed):
case JEDEC_SFDP: int toread; if (emu_chip != EMULATE_WINBOND_W25Q64CV) break; if (writecnt < 4) break; offs = writearr[1] << 16 | writearr[2] << 8 | writearr[3]; /* The response is shifted if more than 4 bytes are written. */ offs += writecnt - 4;
first of all... this breaks the implementation atm, because it uses a writecnt of 5 to include the dummy byte
secondly i am not sure i understand your intention about the shifting. i guess the following: flashrom cant cope with concurrent write and read bytes... so you are specifying that if we write more bytes than needed we miss the bytes read in the beginning.. which seems fair enough.
therefor i have changed 4 to 5 in the writecnt-related lines..
/* The SFDP spec suggests wraparound is allowed. */ offs %= sizeof(sfdp_table); toread = min(sizeof(sfdp_table) - offs, readcnt); memcpy(readarr, sfdp_table + offs, toread); if (toread < readcnt) memcpy(readarr + toread, sfdp_table, min(sizeof(sfdp_table),readcnt - toread)); if (readcnt - toread > sizeof(sfdp_table)) msg_pdbg("Reading more than SFDP table size in one " "chunk, contents may be incomplete.\n");
you are trying to support wrapping around for continous reads here (at least for the first wrap around afaics) while the specs say that this is not allowed at all (whatever that actually means...). so i would rather just read up to the boundary and reply 0xFF after that. makes that patch simpler too :)
break;@@ -657,6 +730,7 @@ static int dummy_spi_send_command(struct flashctx *flash, unsigned int writecnt, case EMULATE_ST_M25P10_RES: case EMULATE_SST_SST25VF040_REMS: case EMULATE_SST_SST25VF032B:
- case EMULATE_WINBOND_W25Q64CV: if (emulate_spi_chip_response(writecnt, readcnt, writearr, readarr)) { msg_pdbg("Invalid command sent to flash chip!\n");
beside that i have also changed the emulated chip to the macronix chip, because it features a second table (that we might want to parse if it is used by other vendors too... one can at least hope :) it helps testing another execution path too...
diff --git a/dummyflasher.c b/dummyflasher.c index afe0518..794a2f6 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -45,6 +45,7 @@ enum emu_chip { EMULATE_ST_M25P10_RES, EMULATE_SST_SST25VF040_REMS, EMULATE_SST_SST25VF032B, + EMULATE_MACRONIX_MX25L6436, }; static enum emu_chip emu_chip = EMULATE_NONE; static char *emu_persistent_image = NULL; @@ -61,6 +62,33 @@ unsigned char spi_blacklist[256]; unsigned char spi_ignorelist[256]; int spi_blacklist_size = 0; int spi_ignorelist_size = 0; + +/* A legit complete SFDP table based on the MX25L6436E (rev. 1.8) datasheet. */ +static const uint8_t const sfdp_table[] = { + 0x53, 0x46, 0x44, 0x50, // @0x00: SFDP signature + 0x00, 0x01, 0x01, 0xFF, // @0x04: revision 1.0, 2 headers + 0x00, 0x00, 0x01, 0x09, // @0x08: JEDEC SFDP header rev. 1.0, 9 DW long + 0x1C, 0x00, 0x00, 0xFF, // @0x0C: PTP0 = 0x1C (instead of 0x30) + 0xC2, 0x00, 0x01, 0x04, // @0x10: Macronix header rev. 1.0, 4 DW long + 0x48, 0x00, 0x00, 0xFF, // @0x14: PTP1 = 0x48 (instead of 0x60) + 0xFF, 0xFF, 0xFF, 0xFF, // @0x18: hole. + 0xE5, 0x20, 0xC9, 0xFF, // @0x1C: SFDP parameter table start + 0xFF, 0xFF, 0xFF, 0x03, // @0x20 + 0x00, 0xFF, 0x08, 0x6B, // @0x24 + 0x08, 0x3B, 0x00, 0xFF, // @0x28 + 0xEE, 0xFF, 0xFF, 0xFF, // @0x2C + 0xFF, 0xFF, 0x00, 0x00, // @0x30 + 0xFF, 0xFF, 0x00, 0xFF, // @0x34 + 0x0C, 0x20, 0x0F, 0x52, // @0x38 + 0x10, 0xD8, 0x00, 0xFF, // @0x3C: SFDP parameter table end + 0xFF, 0xFF, 0xFF, 0xFF, // @0x40: hole. + 0xFF, 0xFF, 0xFF, 0xFF, // @0x44: hole. + 0x00, 0x36, 0x00, 0x27, // @0x48: Macronix parameter table start + 0xF4, 0x4F, 0xFF, 0xFF, // @0x4C + 0xD9, 0xC8, 0xFF, 0xFF, // @0x50 + 0xFF, 0xFF, 0xFF, 0xFF, // @0x54: Macronix parameter table end +}; + #endif #endif
@@ -296,6 +324,19 @@ int dummy_init(void) msg_pdbg("Emulating SST SST25VF032B SPI flash chip (RDID, AAI " "write)\n"); } + if (!strcmp(tmp, "MX25L6436")) { + emu_chip = EMULATE_MACRONIX_MX25L6436; + emu_chip_size = 8 * 1024 * 1024; + emu_max_byteprogram_size = 256; + emu_max_aai_size = 0; + emu_jedec_se_size = 4 * 1024; + emu_jedec_be_52_size = 32 * 1024; + emu_jedec_be_d8_size = 64 * 1024; + emu_jedec_ce_60_size = emu_chip_size; + emu_jedec_ce_c7_size = emu_chip_size; + msg_pdbg("Emulating Macronix MX25L6436 SPI flash chip (RDID, " + "SFDP)\n"); + } #endif if (emu_chip == EMULATE_NONE) { msg_perr("Invalid chip specified for emulation: %s\n", tmp); @@ -471,15 +512,26 @@ static int emulate_spi_chip_response(unsigned int writecnt, readarr[1] = 0x44; break; case JEDEC_RDID: - if (emu_chip != EMULATE_SST_SST25VF032B) + switch (emu_chip) { + case EMULATE_SST_SST25VF032B: + if (readcnt > 0) + readarr[0] = 0xbf; + if (readcnt > 1) + readarr[1] = 0x25; + if (readcnt > 2) + readarr[2] = 0x4a; break; - /* Respond with SST_SST25VF032B. */ - if (readcnt > 0) - readarr[0] = 0xbf; - if (readcnt > 1) - readarr[1] = 0x25; - if (readcnt > 2) - readarr[2] = 0x4a; + case EMULATE_MACRONIX_MX25L6436: + if (readcnt > 0) + readarr[0] = 0xc2; + if (readcnt > 1) + readarr[1] = 0x20; + if (readcnt > 2) + readarr[2] = 0x17; + break; + default: /* ignore */ + break; + } break; case JEDEC_RDSR: memset(readarr, 0, readcnt); @@ -629,7 +681,33 @@ static int emulate_spi_chip_response(unsigned int writecnt, /* emu_jedec_ce_c7_size is emu_chip_size. */ memset(flashchip_contents, 0xff, emu_jedec_ce_c7_size); break; - default: + case JEDEC_SFDP: { + unsigned int toread; + if (emu_chip != EMULATE_MACRONIX_MX25L6436) + break; + if (writecnt < 5) + break; + offs = writearr[1] << 16 | writearr[2] << 8 | writearr[3]; + /* The response is shifted if more than 4 bytes are written. */ + offs += writecnt - 5; + /* The SFDP spec suggests wraparound is allowed as long as it + * does not happen in a single continuous read. */ + if (offs >= sizeof(sfdp_table)) { + msg_pdbg("Wrapping the start address around the SFDP " + "table boundary (using 0x%x instead of 0x%x)." + "\n", + (unsigned int)(offs % sizeof(sfdp_table)), + offs); + offs %= sizeof(sfdp_table); + } + toread = min(sizeof(sfdp_table) - offs, readcnt); + memcpy(readarr, sfdp_table + offs, toread); + if (toread < readcnt) + msg_pdbg("Crossing the SFDP table boundary in a single " + "continuous chunk produces undefined results " + "after that point.\n"); + break; + } default: /* No special response. */ break; } @@ -657,6 +735,7 @@ static int dummy_spi_send_command(struct flashctx *flash, unsigned int writecnt, case EMULATE_ST_M25P10_RES: case EMULATE_SST_SST25VF040_REMS: case EMULATE_SST_SST25VF032B: + case EMULATE_MACRONIX_MX25L6436: if (emulate_spi_chip_response(writecnt, readcnt, writearr, readarr)) { msg_pdbg("Invalid command sent to flash chip!\n"); diff --git a/flashrom.8 b/flashrom.8 index ff8ed9d..fdf1251 100644 --- a/flashrom.8 +++ b/flashrom.8 @@ -432,6 +432,8 @@ vendor): .sp .RB "* SST " SST25VF032B " SPI flash chip (RDID, AAI write)" .sp +.RB "* Macronix " MX25L6436 " SPI flash chip (RDID, SFDP)" +.sp Example: .B "flashrom -p dummy:emulate=SST25VF040.REMS" .TP
-
Carl-Daniel Hailfinger -
Stefan Tauner