diff --git a/doc/classic_cli_manpage.rst b/doc/classic_cli_manpage.rst index f0fd6812c..f0879e472 100644 --- a/doc/classic_cli_manpage.rst +++ b/doc/classic_cli_manpage.rst @@ -317,7 +317,7 @@ All operations involving any chip access (probe/read/write/...) require the ``-p **--progress** - [Experimental feature] Show progress percentage of operations on the standard output. + Show progress percentage of operations on the standard output. **--sacrifice-ratio ** Fraction (as a percentage, 0-50) of an erase block that may be erased even if unmodified. diff --git a/flashrom.c b/flashrom.c index 88fa5d9ef..4ae939058 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1243,6 +1243,56 @@ static void setup_progress_from_layout(struct flashctx *flashctx, init_progress(flashctx, stage, total); } + +static void setup_progress_from_layout_and_diff(struct flashctx *flashctx, + const void *have, + const void *want, + enum flashrom_progress_stage stage) +{ + if (!flashctx->progress_callback) + return; + + const struct flashrom_layout *flash_layout = get_layout(flashctx); + const size_t page_size = flashctx->chip->page_size; + + size_t total = 0; + + const struct romentry *entry = NULL; + while ((entry = layout_next_included(flash_layout, entry))) { + const struct flash_region *region = &entry->region; + + if (stage == FLASHROM_PROGRESS_ERASE) { + size_t offset; + for (offset = region->start; offset <= region->end; offset += page_size) { + const size_t len = min(page_size, region->end + 1 - offset); + + if (need_erase(have, want, len, flashctx->chip->gran, ERASED_VALUE(flashctx))) + total += len; + } + } + + if (stage == FLASHROM_PROGRESS_WRITE) { + unsigned int start = region->start; + unsigned int len; + while ((len = get_next_write(have + start, want + start, + region->end + 1 - start, &start, flashctx->chip->gran))) { + start += len; + total += len; + } + + if (flashctx->chip->feature_bits & FEATURE_NO_ERASE) + /* For chips with FEATURE_NO_ERASE erase op is running as write under the hood. + * So typical write, which usually consists of erasing and then writing, + * would be writing and then writing again. The planned total length for the + * progress indicator for write is double. */ + total *= 2; + } + } + + init_progress(flashctx, stage, total); +} + + /** * @brief Reads the included layout regions into a buffer. * @@ -1374,7 +1424,7 @@ static int erase_by_layout(struct flashctx *const flashctx) memset(newcontents, ERASED_VALUE(flashctx), flash_size); setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_READ); - setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_ERASE); + setup_progress_from_layout_and_diff(flashctx, curcontents, newcontents, FLASHROM_PROGRESS_ERASE); const struct flashrom_layout *const flash_layout = get_layout(flashctx); const struct romentry *entry = NULL; @@ -1413,8 +1463,8 @@ static int write_by_layout(struct flashctx *const flashctx, } setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_READ); - setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_WRITE); - setup_progress_from_layout(flashctx, FLASHROM_PROGRESS_ERASE); + setup_progress_from_layout_and_diff(flashctx, curcontents, newcontents, FLASHROM_PROGRESS_WRITE); + setup_progress_from_layout_and_diff(flashctx, curcontents, newcontents, FLASHROM_PROGRESS_ERASE); const struct romentry *entry = NULL; while ((entry = layout_next_included(flash_layout, entry))) { diff --git a/tests/chip.c b/tests/chip.c index ac606639f..25bb8db76 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -47,7 +47,8 @@ static struct { }; struct progress_user_data { - size_t last_seen; /* % of progress last reported, to be asserted in the progress callback. */ + /* % of progress last reported for each operation, to be asserted in the progress callback. */ + size_t last_seen[FLASHROM_PROGRESS_NR]; }; static int read_chip(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) @@ -86,13 +87,14 @@ static void progress_callback(struct flashctx *flash) { printf("Progress started for stage %d, initial callback call\n", flash->progress_state->stage); } else { /* Progress cannot go backwards. */ - assert_true(flash->progress_state->current >= progress_user_data->last_seen); + assert_true(flash->progress_state->current >= progress_user_data->last_seen[flash->progress_state->stage]); } - if (flash->progress_state->current >= flash->progress_state->total) - printf("Progress complete for stage %d, final callback call\n", flash->progress_state->stage); + if (flash->progress_state->current >= flash->progress_state->total - 1) + printf("Progress almost complete for stage %d, current %ld, total %ld\n", + flash->progress_state->stage, flash->progress_state->current, flash->progress_state->total); - progress_user_data->last_seen = flash->progress_state->current; + progress_user_data->last_seen[flash->progress_state->stage] = flash->progress_state->current; } static void setup_chip(struct flashrom_flashctx *flashctx, struct flashrom_layout **layout, @@ -144,6 +146,7 @@ static const struct flashchip chip_8MiB = { .tested = TEST_OK_PREW, .read = TEST_READ_INJECTOR, .write = TEST_WRITE_INJECTOR, + .page_size = 256, .block_erasers = {{ /* All blocks within total size of the chip. */ @@ -586,6 +589,55 @@ void write_chip_feature_no_erase(void **state) free(newcontents); } +void write_chip_feature_no_erase_with_progress(void **state) +{ + (void) state; /* unused */ + + static struct io_mock_fallback_open_state data = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock chip_io = { + .fallback_open_state = &data, + }; + + struct flashrom_flashctx flashctx = { 0 }; + struct flashrom_layout *layout; + + /* + * Tricking the dummyflasher by asking to emulate W25Q128FV but giving to it + * mock chip with FEATURE_NO_ERASE. + * As long as chip size is the same, this is fine. + */ + struct flashchip mock_chip = chip_no_erase; + const char *param_dup = "bus=spi,emulate=W25Q128FV"; + + setup_chip(&flashctx, &layout, &mock_chip, param_dup, &chip_io); + + /* See comment in write_chip_test_success */ + const char *const filename = "-"; + unsigned long size = mock_chip.total_size * 1024; + uint8_t *const newcontents = malloc(size); + assert_non_null(newcontents); + + struct progress_user_data progress_user_data = {0}; + struct flashrom_progress progress_state = { + .user_data = &progress_user_data + }; + flashrom_set_progress_callback(&flashctx, progress_callback, &progress_state); + + printf("Write chip operation started.\n"); + assert_int_equal(0, read_buf_from_file(newcontents, size, filename)); + assert_int_equal(0, flashrom_image_write(&flashctx, newcontents, size, NULL)); + assert_int_equal(0, flashrom_image_verify(&flashctx, newcontents, size)); + printf("Write chip operation done.\n"); + + teardown(&layout); + + free(newcontents); +} + + void write_nonaligned_region_with_dummyflasher_test_success(void **state) { (void) state; /* unused */ diff --git a/tests/tests.c b/tests/tests.c index 00b46656d..72b4868da 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -504,6 +504,7 @@ int main(int argc, char *argv[]) cmocka_unit_test(write_chip_with_progress), cmocka_unit_test(write_chip_with_dummyflasher_test_success), cmocka_unit_test(write_chip_feature_no_erase), + cmocka_unit_test(write_chip_feature_no_erase_with_progress), cmocka_unit_test(write_nonaligned_region_with_dummyflasher_test_success), cmocka_unit_test(verify_chip_test_success), cmocka_unit_test(verify_chip_with_dummyflasher_test_success), diff --git a/tests/tests.h b/tests/tests.h index dba33fbdf..85c4f52c7 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -91,6 +91,7 @@ void write_chip_test_success(void **state); void write_chip_with_progress(void **state); void write_chip_with_dummyflasher_test_success(void **state); void write_chip_feature_no_erase(void **state); +void write_chip_feature_no_erase_with_progress(void **state); void write_nonaligned_region_with_dummyflasher_test_success(void **state); void verify_chip_test_success(void **state); void verify_chip_with_dummyflasher_test_success(void **state);