[L] Change in flashrom[main]: [WIP] tests: Add tests for erasing chip with protected region
Attention is currently required from: Anastasia Klimchuk.
Hello Anastasia Klimchuk,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/flashrom/+/82392?usp=email
to review the following change.
Change subject: [WIP] tests: Add tests for erasing chip with protected region ......................................................................
[WIP] tests: Add tests for erasing chip with protected region
Change-Id: Ifd472b623dbbc4d11d6ffee7e5b24e38b1fb562a Signed-off-by: Anastasia Klimchuk aklm@flashrom.org Signed-off-by: Peter Marheine pmarheine@chromium.org --- M tests/erase_func_algo.c M tests/tests.c M tests/tests.h 3 files changed, 309 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/92/82392/1
diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index ed9eb04..35f9a16 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -49,8 +49,8 @@ uint8_t written_buf[MOCK_CHIP_SIZE]; /* Expected content after write. */ struct erase_invoke eraseblocks_expected[MOCK_CHIP_SIZE]; /* Expected order of eraseblocks invocations. */ unsigned int eraseblocks_expected_ind; /* Expected number of eraseblocks invocations. */ - char erase_test_name[20]; /* Test case display name for testing erase operation. */ - char write_test_name[20]; /* Test case display name for testing write operation. */ + char erase_test_name[40]; /* Test case display name for testing erase operation. */ + char write_test_name[40]; /* Test case display name for testing write operation. */ };
struct all_state { @@ -703,6 +703,145 @@ }, };
+ +#define START_PROTECTED_REGION 8 +#define END_PROTECTED_REGION 16 + +/* + * Setup all test cases with protected region. + * Protected region is the same for all test cases, between bytes 8 - 15. + */ +static struct test_case test_cases_protected_region[] = { + { + /* + * Test case #0 + * + * Initial vs written: all 16 bytes are different. + * One layout region for the whole chip. + * Chip with eraseblocks 1, 2, 4, 8, 16. + */ + .chip = &chip_1_2_4_8_16, + .regions = {{0, MIN_REAL_CHIP_SIZE - 1, "whole chip"}}, + .initial_buf = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, + 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}, + .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected_ind = 1, + .erase_test_name = "Erase protected region test case #0", + }, { + /* + * Test case #1 + * + * Initial vs written: 9 bytes the same, 7 bytes different. + * Two layout regions each one 8 bytes, which is 1/2 size of chip. + * Chip with eraseblocks 1, 2, 4, 8, 16. + */ + .chip = &chip_1_2_4_8_16, + .regions = {{0, MOCK_CHIP_SIZE/2 - 1, "part1"}, {MOCK_CHIP_SIZE/2, MIN_REAL_CHIP_SIZE - 1, "part2"}}, + .initial_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, + 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff}, + .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected_ind = 1, + .erase_test_name = "Erase protected region test case #1", + }, { + /* + * Test case #2 + * + * Initial vs written: 6 bytes the same, 4 bytes different, 4 bytes the same, 2 bytes different. + * Two layout regions 11 and 5 bytes each. + * Chip with eraseblocks 1, 2, 4, 8, 16. + */ + .chip = &chip_1_2_4_8_16, + .regions = {{0, 10, "odd1"}, {11, 15, "odd2"}, {MOCK_CHIP_SIZE, MIN_REAL_CHIP_SIZE - 1, "longtail"}}, + .initial_buf = {0xff, 0xff, 0x0, 0xff, 0x0, 0xff, 0x0, 0xff, + 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff}, + .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected_ind = 1, + .erase_test_name = "Erase protected region test case #2", + }, { + /* + * Test case #3 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned regions 2+4+9+1b which require use of the 1-byte erase block. + * Chip with eraseblocks 1, 8, 16. + */ + .chip = &chip_1_8_16, + .regions = {{0, 1, "reg2"}, {2, 5, "reg4"}, {6, 14, "reg9"}, + {15, MIN_REAL_CHIP_SIZE - 1, "reg1"}}, + .initial_buf = {0x4, 0x4, 0x5, 0x5, 0x5, 0x5, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x7}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x7}, + .eraseblocks_expected = { + {6, 1, TEST_ERASE_INJECTOR_1}, + {7, 1, TEST_ERASE_INJECTOR_1}, + {2, 1, TEST_ERASE_INJECTOR_1}, + {3, 1, TEST_ERASE_INJECTOR_1}, + {4, 1, TEST_ERASE_INJECTOR_1}, + {5, 1, TEST_ERASE_INJECTOR_1}, + {0, 1, TEST_ERASE_INJECTOR_1}, + {1, 1, TEST_ERASE_INJECTOR_1}, + }, + .eraseblocks_expected_ind = 8, + .erase_test_name = "Erase protected region test case #3", + }, { + /* + * Test case #4 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned region 3+13b which require use of the 1-byte erase block. + * Chip with eraseblocks 1, 8, 16. + */ + .chip = &chip_1_8_16, + .regions = {{0, 2, "reg3"}, {3, MIN_REAL_CHIP_SIZE - 1, "tail"}}, + .initial_buf = {0x4, 0x4, 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .eraseblocks_expected = { + {3, 1, TEST_ERASE_INJECTOR_1}, + {4, 1, TEST_ERASE_INJECTOR_1}, + {5, 1, TEST_ERASE_INJECTOR_1}, + {6, 1, TEST_ERASE_INJECTOR_1}, + {7, 1, TEST_ERASE_INJECTOR_1}, + {0, 1, TEST_ERASE_INJECTOR_1}, + {1, 1, TEST_ERASE_INJECTOR_1}, + {2, 1, TEST_ERASE_INJECTOR_1}, + }, + .eraseblocks_expected_ind = 8, + .erase_test_name = "Erase protected region test case #4", + }, { + /* + * Test case #5 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned region 9+7b. + * Chip with eraseblocks 8, 16. + */ + .chip = &chip_8_16, + .regions = {{0, 8, "reg9"}, {9, MIN_REAL_CHIP_SIZE - 1, "tail"}}, + .initial_buf = {0x4, 0x4, 0x4, 0x4, 0x4, 0x4, 0x4, 0x4, + 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected_ind = 1, + .erase_test_name = "Erase test case #5", + }, +}; + static int setup(void **state) { struct test_case *current_test_case = *state; g_state.current_test_case = current_test_case; @@ -746,6 +885,29 @@ }
/* + * Creates the array of tests for each test case in test_cases_protected_region[]. + * The caller needs to free the allocated memory. + */ +struct CMUnitTest *get_erase_protected_region_algo_tests(size_t *num_tests) { + const size_t test_cases_num = ARRAY_SIZE(test_cases_protected_region); + + struct CMUnitTest *all_cases = calloc(test_cases_num, sizeof(struct CMUnitTest)); + *num_tests = test_cases_num; + + for (size_t i = 0; i < test_cases_num; i++) { + all_cases[i] = (struct CMUnitTest) { + .name = test_cases_protected_region[i].erase_test_name, + .setup_func = setup, + .teardown_func = teardown, + .initial_state = &test_cases_protected_region[i], + .test_func = erase_unwritable_regions_skipflag_on_test_success, + }; + } + + return all_cases; +} + +/* * This function is invoked for every test case in test_cases[], * current test case is passed as an argument. */ @@ -845,3 +1007,135 @@
assert_int_equal(0, all_write_test_result); } + +static void get_protected_region(const struct flashctx *flash, unsigned int addr, struct flash_region *region) +{ + if (addr < 20) + printf("Inside test_get_flash_region for addr=0x%x\n", addr); + + if (addr < START_PROTECTED_REGION) { + region->name = strdup("not protected"); + region->start = 0; + region->end = START_PROTECTED_REGION; + region->read_prot = false; + region->write_prot = false; + } else if (addr < END_PROTECTED_REGION) { + region->name = strdup("protected"); + region->start = START_PROTECTED_REGION; + region->end = END_PROTECTED_REGION; + region->read_prot = true; + region->write_prot = true; + } else { + region->name = strdup("tail"); + region->start = 16; + region->end = flashrom_flash_getsize(flash); + region->read_prot = false; + region->write_prot = false; + } +} + +static int block_erase_chip_with_protected_region(struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen) +{ + if (blockaddr + blocklen <= MOCK_CHIP_SIZE) { + LOG_ERASE_FUNC; + + /* Register eraseblock invocation. */ + g_state.eraseblocks_actual[g_state.eraseblocks_actual_ind].blocklen + = blocklen; + g_state.eraseblocks_actual[g_state.eraseblocks_actual_ind].blockaddr + = blockaddr; + g_state.eraseblocks_actual_ind++; + } + + assert_in_range(blockaddr + blocklen, 0, MIN_REAL_CHIP_SIZE); + + // Check we are not trying to erase protected region. This should not happen, + // because the logic should handle protected regions and never invoke erasefn + // for them. If this happens, means there is a bug in erasure logic, and test fails. + // + // Note: returning 1 instead of assert, so that the flow goes back to erasure code + // to clean up the memory after failed erase. Memory leaks are also tested by unit tests. + if ((blockaddr >= START_PROTECTED_REGION && blockaddr < END_PROTECTED_REGION) + || (blockaddr + blocklen - 1 >= START_PROTECTED_REGION + && blockaddr + blocklen - 1 < END_PROTECTED_REGION)) + return 1; + + memset(&g_state.buf[blockaddr], ERASE_VALUE, blocklen); + return 0; +} + +void erase_unwritable_regions_skipflag_on_test_success(void **state) +{ + struct test_case* current_test_case = *state; + + int all_erase_tests_result = 0; + struct flashrom_flashctx flashctx = { 0 }; + const char *param = ""; /* Default values for all params. */ + + struct flashrom_layout *layout; + + setup_chip(&flashctx, &layout, param, current_test_case); + + // This test needs special block erase to emulate protected regions. + g_test_erase_injector = block_erase_chip_with_protected_region; + + flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS, true); + + // We use dummyflasher programmer in tests, but for this test we need to + // replace dummyflasher's default get_region fn with test one. + // The rest of master struct is fine for this test. + // Note dummyflasher registers multiple masters by default, so replace + // get_region for each of them. + flashctx.mst->spi.get_region = &get_protected_region; + flashctx.mst->opaque.get_region = &get_protected_region; + + 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); + + 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); + + all_erase_tests_result |= ret; + all_erase_tests_result |= !chip_erased; + all_erase_tests_result |= !eraseblocks_in_order; + all_erase_tests_result |= !eraseblocks_invocations; + + teardown_chip(&layout); + + assert_int_equal(0, all_erase_tests_result); +} + +void erase_unwritable_regions_skipflag_off_test_success(void **state) +{ +// flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS, false); +} diff --git a/tests/tests.c b/tests/tests.c index c38a66c..bff24e5 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -509,6 +509,16 @@ ret |= _cmocka_run_group_tests("erase_func_algo.c tests", erase_func_algo_tests, n_erase_tests, NULL, NULL); free(erase_func_algo_tests);
+ size_t n_erase_protected_region_tests; + struct CMUnitTest *erase_protected_region_algo_tests + = get_erase_protected_region_algo_tests(&n_erase_protected_region_tests); + ret |= _cmocka_run_group_tests("erase_func_algo.c protected region tests", + erase_protected_region_algo_tests, + n_erase_protected_region_tests, + NULL, + NULL); + free(erase_protected_region_algo_tests); + // Write-protect group should run last. const struct CMUnitTest chip_wp_tests[] = { cmocka_unit_test(invalid_wp_range_dummyflasher_test_success), diff --git a/tests/tests.h b/tests/tests.h index af5f968..7f8fc3c 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -107,7 +107,10 @@
/* erase_func_algo.c */ struct CMUnitTest *get_erase_func_algo_tests(size_t *num_tests); +struct CMUnitTest *get_erase_protected_region_algo_tests(size_t *num_tests); void erase_function_algo_test_success(void **state); void write_function_algo_test_success(void **state); +void erase_unwritable_regions_skipflag_off_test_success(void **state); +void erase_unwritable_regions_skipflag_on_test_success(void **state);
#endif /* TESTS_H */
-
Peter Marheine (Code Review)