Namyoon Woo has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
support variable-size SPI chip for dummy programmer
This is designed for firmware updater to pack firmware image preserving some specific partitions in any size.
BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
$ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c M flashchips.h 4 files changed, 159 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/1
diff --git a/chipdrivers.h b/chipdrivers.h index 3c7d146..cf03811 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -196,6 +196,9 @@ int probe_en29lv640b(struct flashctx *flash); int write_en29lv640b(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len);
+/* dummyflasher.c */ +int probe_variable_size(struct flashctx *flash); + /* edi.c */ int edi_chip_block_erase(struct flashctx *flash, unsigned int page, unsigned int size); int edi_chip_write(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); diff --git a/dummyflasher.c b/dummyflasher.c index e1a1d80..4024f2a 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -21,6 +21,7 @@ #include "flash.h" #include "chipdrivers.h" #include "programmer.h" +#include "flashchips.h"
/* Remove the #define below if you don't want SPI flash chip emulation. */ #define EMULATE_SPI_CHIP 1 @@ -33,6 +34,13 @@ #if EMULATE_CHIP #include <sys/types.h> #include <sys/stat.h> + +#if EMULATE_SPI_CHIP +/* The name of variable-size virtual chip. A 4MB flash example: + * flashrom -p dummy:emulate=VARIABLE_SIZE,size=4194304 + */ +#define VARIABLE_SIZE_CHIP_NAME "VARIABLE_SIZE" +#endif #endif
#if EMULATE_CHIP @@ -44,6 +52,7 @@ EMULATE_SST_SST25VF032B, EMULATE_MACRONIX_MX25L6436, EMULATE_WINBOND_W25Q128FV, + EMULATE_VARIABLE_SIZE, }; static enum emu_chip emu_chip = EMULATE_NONE; static char *emu_persistent_image = NULL; @@ -147,6 +156,12 @@ return 0; }
+/* Values for the 'size' parameter */ +enum { + SIZE_UNKNOWN = -1, + SIZE_AUTO = -2, +}; + int dummy_init(void) { char *bustext = NULL; @@ -154,6 +169,7 @@ unsigned int i; #if EMULATE_SPI_CHIP char *status = NULL; + int size = SIZE_UNKNOWN; /* size for VARIOUS_SIZE chip device */ #endif #if EMULATE_CHIP struct stat image_stat; @@ -272,6 +288,31 @@ free(tmp);
#if EMULATE_CHIP +#if EMULATE_SPI_CHIP + tmp = extract_programmer_param("size"); + if (tmp) { + int multiplier = 1; + if (!strcmp(tmp, "auto")) + size = SIZE_AUTO; + else if (strlen(tmp)) { + int remove_last_char = 1; + switch (tmp[strlen(tmp) - 1]) { + case 'k': case 'K': + multiplier = 1024; + break; + case 'm': case 'M': + multiplier = 1024 * 1024; + break; + default: + remove_last_char = 0; + break; + } + if (remove_last_char) tmp[strlen(tmp) - 1] = '\0'; + size = atoi(tmp) * multiplier; + } + } +#endif + tmp = extract_programmer_param("emulate"); if (!tmp) { msg_pdbg("Not emulating any flash chip.\n"); @@ -343,6 +384,42 @@ emu_jedec_ce_c7_size = emu_chip_size; msg_pdbg("Emulating Winbond W25Q128FV SPI flash chip (RDID)\n"); } + + emu_persistent_image = extract_programmer_param("image"); + /* Will be freed by shutdown function if necessary. */ + if (!emu_persistent_image) { + /* Nothing else to do. */ + goto dummy_init_out; + } + + if (!strncmp(tmp, VARIABLE_SIZE_CHIP_NAME, + strlen(VARIABLE_SIZE_CHIP_NAME))) { + if (size == SIZE_UNKNOWN) { + msg_perr("%s: the size parameter is not given.\n", + __func__); + free(tmp); + return 1; + } else if (size == SIZE_AUTO) { + if (stat(emu_persistent_image, &image_stat)) { + msg_perr("%s: no image so cannot use automatic size.\n", + __func__); + free(tmp); + return 1; + } + size = image_stat.st_size; + } + emu_chip = EMULATE_VARIABLE_SIZE; + emu_chip_size = size; + 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 generic SPI flash chip (size=%d bytes)\n", + emu_chip_size); + } #endif if (emu_chip == EMULATE_NONE) { msg_perr("Invalid chip specified for emulation: %s\n", tmp); @@ -376,12 +453,6 @@ msg_pdbg("Filling fake flash chip with 0xff, size %i\n", emu_chip_size); memset(flashchip_contents, 0xff, emu_chip_size);
- /* Will be freed by shutdown function if necessary. */ - emu_persistent_image = extract_programmer_param("image"); - if (!emu_persistent_image) { - /* Nothing else to do. */ - goto dummy_init_out; - } /* We will silently (in default verbosity) ignore the file if it does not exist (yet) or the size does * not match the emulated chip. */ if (!stat(emu_persistent_image, &image_stat)) { @@ -610,6 +681,16 @@ if (readcnt > 2) readarr[2] = 0x18; break; + case EMULATE_VARIABLE_SIZE: + if (readcnt > 0) + readarr[0] = (VARIABLE_SIZE_MANUF_ID >> 8) & 0xff; + if (readcnt > 1) + readarr[1] = VARIABLE_SIZE_MANUF_ID & 0xff; + if (readcnt > 2) + readarr[2] = (VARIABLE_SIZE_DEVICE_ID >> 8) & 0xff; + if (readcnt > 3) + readarr[3] = VARIABLE_SIZE_DEVICE_ID & 0xff; + break; default: /* ignore */ break; } @@ -840,6 +921,7 @@ case EMULATE_SST_SST25VF032B: case EMULATE_MACRONIX_MX25L6436: case EMULATE_WINBOND_W25Q128FV: + case EMULATE_VARIABLE_SIZE: if (emulate_spi_chip_response(writecnt, readcnt, writearr, readarr)) { msg_pdbg("Invalid command sent to flash chip!\n"); @@ -862,3 +944,47 @@ return spi_write_chunked(flash, buf, start, len, spi_write_256_chunksize); } + +#if EMULATE_CHIP && EMULATE_SPI_CHIP +int probe_variable_size(struct flashctx *flash) +{ + int i; + + /* Skip the probing if we don't emulate this chip. */ + if (emu_chip != EMULATE_VARIABLE_SIZE) + return 0; + + /* + * This will break if one day flashctx becomes read-only. + * Once that happens, we need to have special hacks in functions: + * + * erase_and_write_flash() in flashrom.c + * read_flash_to_file() + * handle_romentries() + * ... + * + * Search "total_size * 1024" in code. + */ + if (emu_chip_size % 1024) + msg_perr("%s: emu_chip_size is not multipler of 1024.\n", + __func__); + flash->chip->total_size = emu_chip_size / 1024; + msg_cdbg("%s: set flash->total_size to %dK bytes.\n", __func__, + flash->chip->total_size); + + /* Update eraser count */ + for (i = 0; i < NUM_ERASEFUNCTIONS; i++) { + struct block_eraser *eraser = &flash->chip->block_erasers[i]; + if (eraser->block_erase == NULL) + break; + + eraser->eraseblocks[0].count = emu_chip_size / + eraser->eraseblocks[0].size; + msg_cdbg("%s: eraser.size=%d, .count=%d\n", + __func__, eraser->eraseblocks[0].size, + eraser->eraseblocks[0].count); + } + + return 1; +} +#endif diff --git a/flashchips.c b/flashchips.c index 8b5b5cc..e99073b 100644 --- a/flashchips.c +++ b/flashchips.c @@ -18759,6 +18759,27 @@
{ .vendor = "Generic", + .name = "Variable Size SPI chip", + .bustype = BUS_SPI, + .manufacture_id = VARIABLE_SIZE_MANUF_ID, + .model_id = VARIABLE_SIZE_DEVICE_ID, + .total_size = 64, /* This size is set temporarily */ + .page_size = 256, + .tested = TEST_OK_PREW, + .probe = probe_variable_size, + .block_erasers = + { + { + .eraseblocks = { {64 * 1024, 1} }, + .block_erase = spi_block_erase_d8, + } + }, + .write = spi_chip_write_256, + .read = spi_chip_read, + }, + + { + .vendor = "Generic", .name = "unknown SPI chip (RDID)", .bustype = BUS_SPI, .manufacture_id = GENERIC_MANUF_ID, diff --git a/flashchips.h b/flashchips.h index 5b7937f..85010ea 100644 --- a/flashchips.h +++ b/flashchips.h @@ -36,6 +36,9 @@ #define PROGMANUF_ID 0xFFFE /* dummy ID for opaque chips behind a programmer */ #define PROGDEV_ID 0x01 /* dummy ID for opaque chips behind a programmer */
+#define VARIABLE_SIZE_MANUF_ID 0x3eaf +#define VARIABLE_SIZE_DEVICE_ID 0x10af + #define ALLIANCE_ID 0x52 /* Alliance Semiconductor */ #define ALLIANCE_AS29F002B 0x34 #define ALLIANCE_AS29F002T 0xB0
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 1: Code-Review+1
This is to sync with the one in chromiumos repo. This patch is quoted from two patches: - https://codereview.chromium.org/6791015 - https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+...
Namyoon Woo has removed a vote from this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Removed Code-Review+1 by Namyoon Woo namyoon@google.com
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 1: Code-Review+2
(1 comment)
https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c@395 PS1, Line 395: if (!strncmp(tmp, VARIABLE_SIZE_CHIP_NAME, : strlen(VARIABLE_SIZE_CHIP_NAME))) { I think this is fine to put on the same line for readability.
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/44879
to look at the new patch set (#2).
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
support variable-size SPI chip for dummy programmer
This is designed for firmware updater to pack firmware image preserving some specific partitions in any size.
BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
$ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c M flashchips.h 4 files changed, 154 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/2
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c@395 PS1, Line 395: if (!strncmp(tmp, VARIABLE_SIZE_CHIP_NAME, : strlen(VARIABLE_SIZE_CHIP_NAME))) {
I think this is fine to put on the same line for readability.
Like other cases above, I converted it to use strcmp(), without a name definition.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 2: Code-Review+2
(1 comment)
https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c@395 PS1, Line 395: if (!strncmp(tmp, VARIABLE_SIZE_CHIP_NAME, : strlen(VARIABLE_SIZE_CHIP_NAME))) {
Like other cases above, I converted it to use strcmp(), without a name definition.
Sorry, what I mean is that you can remove the line wrap here.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 2: Code-Review-1
(8 comments)
I don't like the parsing of the command-line parameters at all.
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@a379 PS2, Line 379: /* Will be freed by shutdown function if necessary. */ Why is this gone?
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@288 PS2, Line 288: if (!strcmp(tmp, "auto")) : size = SIZE_AUTO; : This branch of the if should have braces
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@293 PS2, Line 293: case 'k': case 'K': I'd put these two case statements in separate lines:
case 'k': case 'K':
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@303 PS2, Line 303: tmp[strlen(tmp) - 1] = '\0'; This should be on a separate line
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@304 PS2, Line 304: atoi(tmp) What will happen if I specify the following sizes as a parameter?
K 1G 87654321 -512m 1 big_enough 0
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@388 PS2, Line 388: 4MB that's 4 MiB (4 MB would be size=4000000)
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@963 PS2, Line 963: if (emu_chip_size % 1024) : msg_perr("%s: emu_chip_size is not multipler of 1024.\n", : __func__); Please move this to the parsing of the commmandline arguments
https://review.coreboot.org/c/flashrom/+/44879/2/flashchips.h File flashchips.h:
https://review.coreboot.org/c/flashrom/+/44879/2/flashchips.h@39 PS2, Line 39: #define VARIABLE_SIZE_MANUF_ID 0x3eaf : #define VARIABLE_SIZE_DEVICE_ID 0x10af Where do these IDs come from? Why not use PROGMANUF_ID and PROGDEV_ID instead?
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/44879
to look at the new patch set (#3).
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
support variable-size SPI chip for dummy programmer
This is designed for firmware updater to pack firmware image preserving some specific partitions in any size.
BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
$ flashrom -p dummy:image=${TMP_FILE},size=16M, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
$ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c M flashchips.h 4 files changed, 177 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/3
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 3:
(9 comments)
https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/44879/1/dummyflasher.c@395 PS1, Line 395: if (!strncmp(tmp, VARIABLE_SIZE_CHIP_NAME, : strlen(VARIABLE_SIZE_CHIP_NAME))) {
Sorry, what I mean is that you can remove the line wrap here.
Done
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@a379 PS2, Line 379: /* Will be freed by shutdown function if necessary. */
Why is this gone?
This is moved to above line 347, because it needed to stat emu_persistent_image in case that SIZE_AUTO was given in parameter.
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@288 PS2, Line 288: if (!strcmp(tmp, "auto")) : size = SIZE_AUTO; :
This branch of the if should have braces
Done
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@293 PS2, Line 293: case 'k': case 'K':
I'd put these two case statements in separate lines: […]
Done
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@303 PS2, Line 303: tmp[strlen(tmp) - 1] = '\0';
This should be on a separate line
Done
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@304 PS2, Line 304: atoi(tmp)
What will happen if I specify the following sizes as a parameter? […]
Added more parse error handling codes.
K => size shall be 0, and processed as an error at the end. 1G => atoi('1G') shall be 0, and processed as an error at the end. 87654321 => shall be processed as an error since it is not a multiple of 1024. -512m => shall be processed as an error since it is negative. 1 => shall be processed as an error since it is not a multiple of 1024. big_enough => size shall be 0, and processed as an error at the end. 0 => size shall be 0, and processed as an error at the end.
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@388 PS2, Line 388: 4MB
that's 4 MiB (4 MB would be size=4000000)
Done
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@963 PS2, Line 963: if (emu_chip_size % 1024) : msg_perr("%s: emu_chip_size is not multipler of 1024.\n", : __func__);
Please move this to the parsing of the commmandline arguments
Done. Moved to the line 427 in patchset#3.
https://review.coreboot.org/c/flashrom/+/44879/2/flashchips.h File flashchips.h:
https://review.coreboot.org/c/flashrom/+/44879/2/flashchips.h@39 PS2, Line 39: #define VARIABLE_SIZE_MANUF_ID 0x3eaf : #define VARIABLE_SIZE_DEVICE_ID 0x10af
Where do these IDs come from? Why not use PROGMANUF_ID and PROGDEV_ID instead?
Done
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/44879
to look at the new patch set (#4).
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
support variable-size SPI chip for dummy programmer
This is designed for firmware updater to pack firmware image preserving some specific partitions in any size.
BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
$ flashrom -p dummy:image=${TMP_FILE},size=16M, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
$ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c 3 files changed, 174 insertions(+), 6 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/4
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/44879
to look at the new patch set (#5).
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
support variable-size SPI chip for dummy programmer
This is designed for firmware updater to pack firmware image preserving some specific partitions in any size.
BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
$ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c 3 files changed, 114 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/5
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/44879
to look at the new patch set (#6).
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
support variable-size SPI chip for dummy programmer
This is designed for firmware updater to pack firmware image preserving some specific partitions in any size.
BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
$ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c 3 files changed, 113 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/6
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 6:
(1 comment)
Patch Set 2: Code-Review-1
(8 comments)
I don't like the parsing of the command-line parameters at all.
In patchset#6, I removed SIZE_AUTO and 'k|K|m|M' prefix handling in size parameter for simplicity.
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/44879/2/dummyflasher.c@a379 PS2, Line 379: /* Will be freed by shutdown function if necessary. */
This is moved to above line 347, because it needed to stat emu_persistent_image in case that SIZE_AU […]
I am bring this back here.
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 6: Code-Review+2
(3 comments)
https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@917 PS6, Line 917: int unsigned i;
https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@938 PS6, Line 938: pdate eraser count * "Update the first count of each of the block_erasers" or something more detailed why here.
https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@941 PS6, Line 941: eraser->block_erase == NULL `if (!eraser->block_erase)` is canonical
Hello build bot (Jenkins), Edward O'Callaghan, Angel Pons,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/44879
to look at the new patch set (#7).
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
support variable-size SPI chip for dummy programmer
This is designed for firmware updater to pack firmware image preserving some specific partitions in any size.
BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
$ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 --- M chipdrivers.h M dummyflasher.c M flashchips.c 3 files changed, 113 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/44879/7
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 7:
(3 comments)
https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@917 PS6, Line 917: int
unsigned i;
Done
https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@938 PS6, Line 938: pdate eraser count *
"Update the first count of each of the block_erasers" or something more detailed why here.
Done
https://review.coreboot.org/c/flashrom/+/44879/6/dummyflasher.c@941 PS6, Line 941: eraser->block_erase == NULL
`if (!eraser->block_erase)` is canonical
Done
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 7: Code-Review+2
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 7:
ping.
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
support variable-size SPI chip for dummy programmer
This is designed for firmware updater to pack firmware image preserving some specific partitions in any size.
BUG=none TEST=ran the command line below: $ flashrom -p dummy:image=${TMP_FILE},size=16777216, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
$ flashrom -p dummy:image=${TMP_FILE},size=auto, \ emulate=VARIABLE_SIZE -w ${IMG} -V -f
Signed-off-by: Namyoon Woo namyoon@google.com Change-Id: Iff266e151459561b126ecfd1c47420b385be1db2 Reviewed-on: https://review.coreboot.org/c/flashrom/+/44879 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org --- M chipdrivers.h M dummyflasher.c M flashchips.c 3 files changed, 113 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved
diff --git a/chipdrivers.h b/chipdrivers.h index 3c7d146..cf03811 100644 --- a/chipdrivers.h +++ b/chipdrivers.h @@ -196,6 +196,9 @@ int probe_en29lv640b(struct flashctx *flash); int write_en29lv640b(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len);
+/* dummyflasher.c */ +int probe_variable_size(struct flashctx *flash); + /* edi.c */ int edi_chip_block_erase(struct flashctx *flash, unsigned int page, unsigned int size); int edi_chip_write(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); diff --git a/dummyflasher.c b/dummyflasher.c index e1a1d80..b2b3b54 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -21,6 +21,7 @@ #include "flash.h" #include "chipdrivers.h" #include "programmer.h" +#include "flashchips.h"
/* Remove the #define below if you don't want SPI flash chip emulation. */ #define EMULATE_SPI_CHIP 1 @@ -44,6 +45,7 @@ EMULATE_SST_SST25VF032B, EMULATE_MACRONIX_MX25L6436, EMULATE_WINBOND_W25Q128FV, + EMULATE_VARIABLE_SIZE, }; static enum emu_chip emu_chip = EMULATE_NONE; static char *emu_persistent_image = NULL; @@ -154,6 +156,7 @@ unsigned int i; #if EMULATE_SPI_CHIP char *status = NULL; + int size = -1; /* size for VARIOUS_SIZE chip device */ #endif #if EMULATE_CHIP struct stat image_stat; @@ -272,6 +275,23 @@ free(tmp);
#if EMULATE_CHIP +#if EMULATE_SPI_CHIP + tmp = extract_programmer_param("size"); + if (tmp) { + size = atoi(tmp); + if (size <= 0 || (size % 1024 != 0)) { + msg_perr("%s: Chip size is not a multipler of 1024: %s\n", + __func__, tmp); + free(tmp); + return 1; + } + free(tmp); + } else { + msg_perr("%s: the size parameter is not given.\n", __func__); + return 1; + } +#endif + tmp = extract_programmer_param("emulate"); if (!tmp) { msg_pdbg("Not emulating any flash chip.\n"); @@ -343,6 +363,23 @@ emu_jedec_ce_c7_size = emu_chip_size; msg_pdbg("Emulating Winbond W25Q128FV SPI flash chip (RDID)\n"); } + + /* The name of variable-size virtual chip. A 4 MiB flash example: + * flashrom -p dummy:emulate=VARIABLE_SIZE,size=4194304 + */ + if (!strcmp(tmp, "VARIABLE_SIZE")) { + emu_chip = EMULATE_VARIABLE_SIZE; + emu_chip_size = size; + 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 generic SPI flash chip (size=%d bytes)\n", + emu_chip_size); + } #endif if (emu_chip == EMULATE_NONE) { msg_perr("Invalid chip specified for emulation: %s\n", tmp); @@ -610,6 +647,16 @@ if (readcnt > 2) readarr[2] = 0x18; break; + case EMULATE_VARIABLE_SIZE: + if (readcnt > 0) + readarr[0] = (PROGMANUF_ID >> 8) & 0xff; + if (readcnt > 1) + readarr[1] = PROGMANUF_ID & 0xff; + if (readcnt > 2) + readarr[2] = (PROGDEV_ID >> 8) & 0xff; + if (readcnt > 3) + readarr[3] = PROGDEV_ID & 0xff; + break; default: /* ignore */ break; } @@ -840,6 +887,7 @@ case EMULATE_SST_SST25VF032B: case EMULATE_MACRONIX_MX25L6436: case EMULATE_WINBOND_W25Q128FV: + case EMULATE_VARIABLE_SIZE: if (emulate_spi_chip_response(writecnt, readcnt, writearr, readarr)) { msg_pdbg("Invalid command sent to flash chip!\n"); @@ -862,3 +910,44 @@ return spi_write_chunked(flash, buf, start, len, spi_write_256_chunksize); } + +#if EMULATE_CHIP && EMULATE_SPI_CHIP +int probe_variable_size(struct flashctx *flash) +{ + unsigned int i; + + /* Skip the probing if we don't emulate this chip. */ + if (emu_chip != EMULATE_VARIABLE_SIZE) + return 0; + + /* + * This will break if one day flashctx becomes read-only. + * Once that happens, we need to have special hacks in functions: + * + * erase_and_write_flash() in flashrom.c + * read_flash_to_file() + * handle_romentries() + * ... + * + * Search "total_size * 1024" in code. + */ + flash->chip->total_size = emu_chip_size / 1024; + msg_cdbg("%s: set flash->total_size to %dK bytes.\n", __func__, + flash->chip->total_size); + + /* Update the first count of each of the block_erasers. */ + for (i = 0; i < NUM_ERASEFUNCTIONS; i++) { + struct block_eraser *eraser = &flash->chip->block_erasers[i]; + if (!eraser->block_erase) + break; + + eraser->eraseblocks[0].count = emu_chip_size / + eraser->eraseblocks[0].size; + msg_cdbg("%s: eraser.size=%d, .count=%d\n", + __func__, eraser->eraseblocks[0].size, + eraser->eraseblocks[0].count); + } + + return 1; +} +#endif diff --git a/flashchips.c b/flashchips.c index 8b5b5cc..6fced9e 100644 --- a/flashchips.c +++ b/flashchips.c @@ -18759,6 +18759,27 @@
{ .vendor = "Generic", + .name = "Variable Size SPI chip", + .bustype = BUS_SPI, + .manufacture_id = PROGMANUF_ID, + .model_id = PROGDEV_ID, + .total_size = 64, /* This size is set temporarily */ + .page_size = 256, + .tested = TEST_OK_PREW, + .probe = probe_variable_size, + .block_erasers = + { + { + .eraseblocks = { {64 * 1024, 1} }, + .block_erase = spi_block_erase_d8, + } + }, + .write = spi_chip_write_256, + .read = spi_chip_read, + }, + + { + .vendor = "Generic", .name = "unknown SPI chip (RDID)", .bustype = BUS_SPI, .manufacture_id = GENERIC_MANUF_ID,
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 8:
(2 comments)
This doesn't seem like anybody has read this. It breaks the dummyflasher, uses discouraged functions (like atoi()), didn't update the manpage...
https://review.coreboot.org/c/flashrom/+/44879/8/dummyflasher.c File dummyflasher.c:
https://review.coreboot.org/c/flashrom/+/44879/8/dummyflasher.c@159 PS8, Line 159: VARIOUS VARIABLE_SIZE?
https://review.coreboot.org/c/flashrom/+/44879/8/dummyflasher.c@291 PS8, Line 291: return 1; What about all the other emulation targets?
I'll stop reading here.
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 8:
Patch Set 8:
(2 comments)
This doesn't seem like anybody has read this. It breaks the dummyflasher, uses discouraged functions (like atoi()), didn't update the manpage...
Uploaded https://review.coreboot.org/c/flashrom/+/46536 for fixes.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 8:
This doesn't seem like anybody has read this. It breaks the dummyflasher, uses discouraged functions (like atoi()), didn't update the manpage...
Uploaded https://review.coreboot.org/c/flashrom/+/46536 for fixes.
Thanks for the immediate reaction! And sorry for the harsh words. I'm sometimes too tense because I see the community cleaning up behind such things. Really, much appreciate that you care about it after merge :)
Namyoon Woo has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 8:
Patch Set 8:
This doesn't seem like anybody has read this. It breaks the dummyflasher, uses discouraged functions (like atoi()), didn't update the manpage...
Uploaded https://review.coreboot.org/c/flashrom/+/46536 for fixes.
Thanks for the immediate reaction! And sorry for the harsh words. I'm sometimes too tense because I see the community cleaning up behind such things. Really, much appreciate that you care about it after merge :)
No problem. Thanks for your feedback!
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/44879 )
Change subject: support variable-size SPI chip for dummy programmer ......................................................................
Patch Set 8:
(1 comment)
Patchset:
PS8: Broke building with CONFIG_DUMMY=no; probe_variable_size() would have to move to a chip driver. It needs access to programmer internals, though. That's what the opaque master is there for... Seems all borked. It also casts objects of all programmers to the dummy one. Only bails out by chance. Another 3 hours wasted on a Saturday night, please somebody remove this code. We've been patching this for almost a year now and it's still broken.
-
Angel Pons (Code Review) -
Edward O'Callaghan (Code Review)
-
Namyoon Woo (Code Review)
-
Nico Huber (Code Review)