Erase should respect --noverify option

This commit adds check for the flag `verify_after_write` after
erase operation and does not perfom verification if the flag
disabled.
From command line, this is set by --noverify option.
libflashrom flag is FLASHROM_FLAG_VERIFY_AFTER_WRITE

Default stays the same, by default verification is performed.

This commit also adds test for the scenario.

Previously, erase operation ignored --noverify option and always
performed verification.

For more details, see
Ticket: https://ticket.coreboot.org/issues/520

Change-Id: I9f6cb7210f4dcdc32870f9096657a08b12e77c7f
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/88734
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Carly Zlabek <carlyzlabek@gmail.com>
Reviewed-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-by: Vincent Fazio <vfazio@gmail.com>

doc/classic_cli_manpage.rst

@@ -72,14 +72,14 @@ All operations involving any chip access (probe/read/write/...) require the ``-p
 
 
 **-n, --noverify**
-        Skip the automatic verification of flash ROM contents after writing. Using this option is **not** recommended,
+        Skip the automatic verification of flash ROM contents after writing or erasing. Using this option is **not** recommended,
         you should only use it if you know what you are doing and if you feel that the time for verification takes too long.
 
         Typical usage is::
 
                 flashrom -p prog -n -w <file>
 
-        This option is only useful in combination with ``--write``.
+        This option is only useful in combination with ``--write`` or ``--erase``.
 
 
 **-N, --noverify-all**

erasure_layout.c

@@ -301,7 +301,8 @@ static int erase_write_helper(struct flashctx *const flashctx, chipoff_t region_
 			if (erasefn(flashctx, start_addr, block_len)) {
 				return -1;
 			}
-			if (check_erased_range(flashctx, start_addr, block_len)) {
+			if (flashctx->flags.verify_after_write
+				&& check_erased_range(flashctx, start_addr, block_len)) {
 				msg_cerr("ERASE FAILED!\n");
 				return -1;
 			}

tests/erase_func_algo.c

@@ -1194,6 +1194,7 @@ static void test_erase_fails_for_unwritable_region(void **);
 static void test_write_with_sacrifice_ratio50(void **);
 static void erase_unwritable_regions_skipflag_on_test_success(void **);
 static void write_unwritable_regions_skipflag_on_test_success(void **);
+static void test_erase_with_noverify(void **);
 
 /*
  * Creates the array of tests for each test case in test_cases_protected_region[].
@@ -1201,7 +1202,7 @@ static void write_unwritable_regions_skipflag_on_test_success(void **);
  */
 struct CMUnitTest *get_erase_protected_region_algo_tests(size_t *num_tests) {
 	const size_t num_parameterized = ARRAY_SIZE(test_cases_protected_region);
-	const size_t num_unparameterized = 2;
+	const size_t num_unparameterized = 3;
 	// Twice the number of parameterized test cases, because each test case is run twice:
 	// for erase and write.
 	const size_t num_cases = num_parameterized * 2 + num_unparameterized;
@@ -1237,6 +1238,10 @@ struct CMUnitTest *get_erase_protected_region_algo_tests(size_t *num_tests) {
 				.name = "write with sacrifice ratio 50",
 				.test_func = test_write_with_sacrifice_ratio50,
 			},
+			(const struct CMUnitTest) {
+				.name = "erase with noverify",
+				.test_func = test_erase_with_noverify,
+			},
 		},
 		sizeof(*all_cases) * num_unparameterized
 	);
@@ -1783,3 +1788,79 @@ static void test_write_with_sacrifice_ratio50(void **state) {
 
 	assert_int_equal(0, all_write_test_result);
 }
+
+static void test_erase_with_noverify(void **state)
+{
+	/* The exact test case doesn't matter that much, because we only want to test
+	 * that verification was NOT invoked. */
+	struct test_case* current_test_case = &test_cases[2];
+
+	int all_erase_tests_result = 0;
+	struct flashrom_flashctx flashctx = { 0 };
+	const char *param = ""; /* Default values for all params. */
+
+	struct flashrom_layout *layout;
+
+	const chipoff_t verify_end_boundary = setup_chip(&flashctx, &layout, param, current_test_case);
+
+	/* We want to test that verify NOT performed. */
+	flashrom_flag_set(&flashctx, FLASHROM_FLAG_VERIFY_AFTER_WRITE, false);
+
+	printf("%s started.\n", current_test_case->erase_test_name);
+	int ret = flashrom_flash_erase(&flashctx);
+	printf("%s returned %d.\n", current_test_case->erase_test_name, ret);
+
+	int chip_erased = !memcmp(g_state.buf, current_test_case->erased_buf, MOCK_CHIP_SIZE);
+
+	int eraseblocks_in_order = !memcmp(g_state.eraseblocks_actual,
+					current_test_case->eraseblocks_expected,
+					current_test_case->eraseblocks_expected_ind * sizeof(struct erase_invoke));
+
+	int eraseblocks_invocations = (g_state.eraseblocks_actual_ind ==
+					current_test_case->eraseblocks_expected_ind);
+
+	int chip_verified = 0;
+	for (unsigned int i = 0; i <= verify_end_boundary; i++)
+		if (g_state.was_modified[i] && g_state.was_verified[i]) {
+			chip_verified = 1; /* byte was verified, but we disabled verification! */
+			printf("Error: byte 0x%x was verified, but verification is disabled\n", i);
+		}
+
+	if (chip_erased)
+		printf("Erased chip memory state for %s is CORRECT\n",
+			current_test_case->erase_test_name);
+	else
+		printf("Erased chip memory state for %s is WRONG\n",
+			current_test_case->erase_test_name);
+
+	if (eraseblocks_in_order)
+		printf("Eraseblocks order of invocation for %s is CORRECT\n",
+			current_test_case->erase_test_name);
+	else
+		printf("Eraseblocks order of invocation for %s is WRONG\n",
+			current_test_case->erase_test_name);
+
+	if (eraseblocks_invocations)
+		printf("Eraseblocks number of invocations for %s is CORRECT\n",
+			current_test_case->erase_test_name);
+	else
+		printf("Eraseblocks number of invocations for %s is WRONG, expected %d actual %d\n",
+			current_test_case->erase_test_name,
+			current_test_case->eraseblocks_expected_ind,
+			g_state.eraseblocks_actual_ind);
+
+	if (!chip_verified)
+		printf("Verification was not performed, as it was disabled.\n");
+	else
+		printf("ERROR: verify operation was disabled, but it was performed.\n");
+
+	all_erase_tests_result |= ret;
+	all_erase_tests_result |= !chip_erased;
+	all_erase_tests_result |= !eraseblocks_in_order;
+	all_erase_tests_result |= !eraseblocks_invocations;
+	all_erase_tests_result |= chip_verified;
+
+	teardown_chip(&layout);
+
+	assert_int_equal(0, all_erase_tests_result);
+}