Display progress for what is actually erased/written

The patch updates calculation of total length for the operation
which is displayed with progress.

The reason is: even if, for example the whole chip erase or write
was requested, the actual length of bytes modified can be less than
whole chip size (areas which already have expected content,
are skipped).

Change-Id: I88ac4d40f1b6ccc1636b1efb690d8d68bdebec08
Co-developed-by: Anastasia Klimchuk <aklm@flashrom.org>
Co-developed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84439
Reviewed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

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 <ratio>**
 	Fraction (as a percentage, 0-50) of an erase block that may be erased even if unmodified.

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))) {

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 */

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),

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);