Peter Marheine has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/82251?usp=email )
Change subject: tests/erase: record the opcode for each erase ......................................................................
tests/erase: record the opcode for each erase
This allows the test to verify that the correct opcode is used when there are protected regions, causing it to fail because that is currently buggy (and intended to be fixed by CB:81385).
Change-Id: I3983fe42c2e7f06668a1bd20d2db7fafa93b8043 Signed-off-by: Peter Marheine pmarheine@chromium.org --- M flashrom.c M include/flash.h M tests/chip.c M tests/erase_func_algo.c 4 files changed, 87 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/82251/1
diff --git a/flashrom.c b/flashrom.c index 630c69d..46bf12a 100644 --- a/flashrom.c +++ b/flashrom.c @@ -402,8 +402,8 @@ return 0; }
-/* special unit-test hook */ -erasefunc_t *g_test_erase_injector; +/* special unit-test hooks */ +erasefunc_t *g_test_erase_injector[5] = {NULL};
erasefunc_t *lookup_erase_func_ptr(const struct block_eraser *const eraser) { @@ -442,7 +442,12 @@ case ERASE_SECTOR_49LFXXXC: return &erase_sector_49lfxxxc; case STM50_SECTOR_ERASE: return &erase_sector_stm50; // TODO rename to &stm50_sector_erase; case EDI_CHIP_BLOCK_ERASE: return &edi_chip_block_erase; - case TEST_ERASE_INJECTOR: return g_test_erase_injector; + case TEST_ERASE_INJECTOR_1: + case TEST_ERASE_INJECTOR_2: + case TEST_ERASE_INJECTOR_3: + case TEST_ERASE_INJECTOR_4: + case TEST_ERASE_INJECTOR_5: + return g_test_erase_injector[eraser->block_erase - TEST_ERASE_INJECTOR_1]; /* default: total function, 0 indicates no erase function set. * We explicitly do not want a default catch-all case in the switch * to ensure unhandled enum's are compiler warnings. diff --git a/include/flash.h b/include/flash.h index 284461e..fe7e018 100644 --- a/include/flash.h +++ b/include/flash.h @@ -319,7 +319,13 @@ ERASE_SECTOR_49LFXXXC, STM50_SECTOR_ERASE, EDI_CHIP_BLOCK_ERASE, - TEST_ERASE_INJECTOR, /* special case must come last. */ + /* special cases must come last. */ + TEST_ERASE_INJECTOR_1, + TEST_ERASE_INJECTOR = TEST_ERASE_INJECTOR_1, + TEST_ERASE_INJECTOR_2, + TEST_ERASE_INJECTOR_3, + TEST_ERASE_INJECTOR_4, + TEST_ERASE_INJECTOR_5, };
enum blockprotect_func { diff --git a/tests/chip.c b/tests/chip.c index 96be7b1..22a5e1c 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -120,7 +120,7 @@
extern write_func_t *g_test_write_injector; extern read_func_t *g_test_read_injector; -extern erasefunc_t *g_test_erase_injector; +extern erasefunc_t *g_test_erase_injector[5];
static const struct flashchip chip_8MiB = { .vendor = "aklm", @@ -179,7 +179,7 @@
g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + g_test_erase_injector[0] = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -238,7 +238,7 @@
g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + g_test_erase_injector[0] = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -313,7 +313,7 @@
g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + g_test_erase_injector[0] = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -504,7 +504,7 @@
g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + g_test_erase_injector[0] = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index a600de2..c98d873 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -39,6 +39,7 @@ struct erase_invoke { unsigned int blockaddr; unsigned int blocklen; + enum block_erase_func erase_func; };
struct test_case { @@ -88,10 +89,10 @@ 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[g_state.eraseblocks_actual_ind] = (struct erase_invoke){ + .blocklen = blocklen, + .blockaddr = blockaddr, + }; g_state.eraseblocks_actual_ind++; }
@@ -103,7 +104,7 @@
extern write_func_t *g_test_write_injector; extern read_func_t *g_test_read_injector; -extern erasefunc_t *g_test_erase_injector; +extern erasefunc_t *g_test_erase_injector[5];
static struct flashchip chip_1_2_4_8_16 = { .vendor = "aklm", @@ -127,19 +128,19 @@ { { .eraseblocks = { {1, MIN_REAL_CHIP_SIZE} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_1, }, { .eraseblocks = { {2, MIN_REAL_CHIP_SIZE / 2} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_2, }, { .eraseblocks = { {4, MIN_REAL_CHIP_SIZE / 4} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_3, }, { .eraseblocks = { {8, MIN_REAL_CHIP_SIZE / 8} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_4, }, { .eraseblocks = { {16, MIN_REAL_CHIP_SIZE / 16} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_5, } }, }; @@ -156,13 +157,13 @@ { { .eraseblocks = { {1, MIN_REAL_CHIP_SIZE} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_1, }, { .eraseblocks = { {8, MIN_REAL_CHIP_SIZE / 8} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_4, }, { .eraseblocks = { {16, MIN_REAL_CHIP_SIZE / 16} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_5, } }, }; @@ -179,10 +180,10 @@ { { .eraseblocks = { {8, MIN_REAL_CHIP_SIZE / 8} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_4, }, { .eraseblocks = { {16, MIN_REAL_CHIP_SIZE / 16} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_5, } }, }; @@ -192,7 +193,12 @@ { g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + memcpy(g_test_erase_injector, + (erasefunc_t *const[]){ + block_erase_chip, block_erase_chip, block_erase_chip, block_erase_chip, block_erase_chip + }, + sizeof(g_test_erase_injector) + );
/* First MOCK_CHIP_SIZE bytes have a meaning and set with given values for this test case. */ memcpy(g_state.buf, current_test_case->initial_buf, MOCK_CHIP_SIZE); @@ -800,7 +806,11 @@ .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 = {{0x0, 0x8}, {0x0, 0x8}}, + .eraseblocks_expected = { + // TODO: I think this is supposed to fail? + {0x0, 0x8, 5}, + {0x0, 0x8, 4} + }, .eraseblocks_expected_ind = 2, .erase_test_name = "Erase protected region test case #4", }, { @@ -818,9 +828,9 @@ .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 = {{0x0, 0x8, 5}}, .eraseblocks_expected_ind = 1, - .erase_test_name = "Erase test case #5", + .erase_test_name = "Erase protected region test case #5", }, };
@@ -1016,16 +1026,17 @@ } }
-static int block_erase_chip_with_protected_region(struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen) +static int block_erase_chip_with_protected_region_tagged(struct flashctx *flash, enum block_erase_func erase_func, unsigned int blockaddr, unsigned int blocklen) { if (blockaddr + blocklen <= MOCK_CHIP_SIZE) { - LOG_ERASE_FUNC; + printf("Eraser called with blockaddr=0x%x, blocklen=0x%x, erase_func=%d\n", blockaddr, blocklen, erase_func - TEST_ERASE_INJECTOR_1 + 1);
/* 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[g_state.eraseblocks_actual_ind] = (struct erase_invoke){ + .erase_func = erase_func, + .blocklen = blocklen, + .blockaddr = blockaddr, + }; g_state.eraseblocks_actual_ind++; }
@@ -1037,15 +1048,34 @@ // // 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. + const unsigned int erase_op_size = 1 << (erase_func - TEST_ERASE_INJECTOR_1); + if (blocklen < erase_op_size) { + printf("Error: block length %d is smaller than erase_func length %d\n", blocklen, erase_op_size); + return 1; + } + if ((blockaddr >= START_PROTECTED_REGION && blockaddr < END_PROTECTED_REGION) || (blockaddr + blocklen - 1 >= START_PROTECTED_REGION - && blockaddr + blocklen - 1 < END_PROTECTED_REGION)) - return 1; + && blockaddr + blocklen - 1 < END_PROTECTED_REGION)) { + printf("Error: block with start=%d, len=%d is overlaps protected region %d-%d\n", + blockaddr, blocklen, START_PROTECTED_REGION, END_PROTECTED_REGION); + return 1; + }
memset(&g_state.buf[blockaddr], ERASE_VALUE, blocklen); return 0; }
+#define ERASE_PROTECTED(i) static int block_erase_chip_with_protected_region_ ## i \ + (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen) { \ + return block_erase_chip_with_protected_region_tagged(flash, TEST_ERASE_INJECTOR_ ## i, blockaddr, blocklen); \ + } +ERASE_PROTECTED(1) +ERASE_PROTECTED(2) +ERASE_PROTECTED(3) +ERASE_PROTECTED(4) +ERASE_PROTECTED(5) + void erase_unwritable_regions_skipflag_on_test_success(void **state) { struct test_case* current_test_case = *state; @@ -1059,7 +1089,16 @@ 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; + memcpy(g_test_erase_injector, + (erasefunc_t *const[]){ + block_erase_chip_with_protected_region_1, + block_erase_chip_with_protected_region_2, + block_erase_chip_with_protected_region_3, + block_erase_chip_with_protected_region_4, + block_erase_chip_with_protected_region_5, + }, + sizeof(g_test_erase_injector) + );
flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS, true);
-
Peter Marheine (Code Review)