Peter Marheine submitted this change.
erasure_layout: Fix get_flash_region bug
When flash regions are protected, erase could incorrectly erase regions
which were meant to be protected by requesting the correct size but
using an erase opcode with coarser granularity than desired (for
instance using a 16-byte erase command while attempting to erase only 8
bytes).
This fixes that by exchanging the nesting of the loops over erase blocks
and flash regions.
Old:
- Select erasefns
- Loop over blocks to erase for each selected erasefn
- Loop over programmer flash regions within erase block
- Erase regions (may fail since selected erasefn will be
too big if flash region is smaller than erase block)
New:
- Loop over programmer flash regions within erase block
- Select erasefns within programer flash region
- Loop over blocks to erase for each selected erasefn
- Erase regions
Eraser selection and erasing has also been factored out into a helper
function to manage nesting depth.
TEST=New test cases pass, whereas some of them fail without the changes
to erasure_layout.c
BUG=https://ticket.coreboot.org/issues/525
Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9
Co-authored-by: Nikolai Artemiev <nartemiev@google.com>
Co-authored-by: Anastasia Klimchuk <aklm@flashrom.org>
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/82393
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
---
M erasure_layout.c
M tests/erase_func_algo.c
M tests/tests.c
M tests/tests.h
4 files changed, 602 insertions(+), 95 deletions(-)
diff --git a/erasure_layout.c b/erasure_layout.c
index d15594d..ab54c12 100644
--- a/erasure_layout.c
+++ b/erasure_layout.c
@@ -238,6 +238,80 @@
}
}
+static int erase_write_helper(struct flashctx *const flashctx, chipoff_t region_start, chipoff_t region_end,
+ uint8_t *curcontents, uint8_t *newcontents,
+ struct erase_layout *erase_layout, bool *all_skipped)
+{
+ const size_t erasefn_count = count_usable_erasers(flashctx);
+
+ // select erase functions
+ for (size_t i = 0; i < erase_layout[erasefn_count - 1].block_count; i++) {
+ if (erase_layout[erasefn_count - 1].layout_list[i].start_addr <= region_end &&
+ region_start <= erase_layout[erasefn_count - 1].layout_list[i].end_addr)
+ select_erase_functions(flashctx, erase_layout,
+ erasefn_count - 1, i,
+ curcontents, newcontents,
+ region_start, region_end);
+ }
+
+ // erase
+ for (size_t i = 0; i < erasefn_count; i++) {
+ for (size_t j = 0; j < erase_layout[i].block_count; j++) {
+ if (!erase_layout[i].layout_list[j].selected)
+ continue;
+
+ chipoff_t start_addr = erase_layout[i].layout_list[j].start_addr;
+ unsigned int block_len = erase_layout[i].layout_list[j].end_addr - start_addr + 1;
+ const uint8_t erased_value = ERASED_VALUE(flashctx);
+ // execute erase
+ erasefunc_t *erasefn = lookup_erase_func_ptr(erase_layout[i].eraser);
+
+
+ if (erasefn(flashctx, start_addr, block_len)) {
+ return -1;
+ }
+ if (check_erased_range(flashctx, start_addr, block_len)) {
+ return -1;
+ msg_cerr("ERASE FAILED!\n");
+ }
+
+ // adjust curcontents
+ memset(curcontents+start_addr, erased_value, block_len);
+ // after erase make it unselected again
+ erase_layout[i].layout_list[j].selected = false;
+ msg_cdbg("E(%"PRIx32":%"PRIx32")", start_addr, start_addr + block_len - 1);
+
+ *all_skipped = false;
+ }
+ }
+
+ // write
+ unsigned int start_here = 0, len_here = 0, erase_len = region_end - region_start + 1;
+ while ((len_here = get_next_write(curcontents + region_start + start_here,
+ newcontents + region_start + start_here,
+ erase_len - start_here, &start_here,
+ flashctx->chip->gran))) {
+ // execute write
+ int ret = write_flash(flashctx,
+ newcontents + region_start + start_here,
+ region_start + start_here, len_here);
+ if (ret) {
+ msg_cerr("Write failed at %#x, Abort.\n", region_start + start_here);
+ return -1;
+ }
+
+ // adjust curcontents
+ memcpy(curcontents + region_start + start_here,
+ newcontents + region_start + start_here, len_here);
+ msg_cdbg("W(%"PRIx32":%"PRIx32")", region_start + start_here, region_start + start_here + len_here - 1);
+
+ *all_skipped = false;
+ }
+
+ return 0;
+}
+
+
/*
* @brief wrapper to use the erase algorithm
*
@@ -253,12 +327,15 @@
uint8_t *curcontents, uint8_t *newcontents,
struct erase_layout *erase_layout, bool *all_skipped)
{
- const size_t erasefn_count = count_usable_erasers(flashctx);
int ret = 0;
- size_t i;
chipoff_t old_start = region_start, old_end = region_end;
align_region(erase_layout, flashctx, ®ion_start, ®ion_end);
+ if (!flashctx->flags.skip_unwritable_regions) {
+ if (check_for_unwritable_regions(flashctx, region_start, region_end - region_start + 1))
+ return -1;
+ }
+
uint8_t *old_start_buf = NULL, *old_end_buf = NULL;
const size_t start_buf_len = old_start - region_start;
const size_t end_buf_len = region_end - old_end;
@@ -287,101 +364,34 @@
memcpy(newcontents + end_offset, curcontents + end_offset, end_buf_len);
}
- // select erase functions
- for (i = 0; i < erase_layout[erasefn_count - 1].block_count; i++) {
- if (erase_layout[erasefn_count - 1].layout_list[i].start_addr <= region_end &&
- region_start <= erase_layout[erasefn_count - 1].layout_list[i].end_addr)
- select_erase_functions(flashctx, erase_layout,
- erasefn_count - 1, i,
- curcontents, newcontents,
- region_start, region_end);
- }
+ unsigned int len;
+ for (unsigned int addr = region_start; addr <= region_end; addr += len) {
+ struct flash_region region;
+ get_flash_region(flashctx, addr, ®ion);
+ len = min(region_end, region.end) - addr + 1;
- for (i = 0; i < erasefn_count; i++) {
- for (size_t j = 0; j < erase_layout[i].block_count; j++) {
- if (!erase_layout[i].layout_list[j].selected) continue;
-
- // erase
- chipoff_t start_addr = erase_layout[i].layout_list[j].start_addr;
- unsigned int block_len = erase_layout[i].layout_list[j].end_addr - start_addr + 1;
- const uint8_t erased_value = ERASED_VALUE(flashctx);
- // execute erase
- erasefunc_t *erasefn = lookup_erase_func_ptr(erase_layout[i].eraser);
-
- if (!flashctx->flags.skip_unwritable_regions) {
- if (check_for_unwritable_regions(flashctx, start_addr, block_len))
- goto _end;
- }
-
- unsigned int len;
- for (unsigned int addr = start_addr; addr < start_addr + block_len; addr += len) {
- struct flash_region region;
- get_flash_region(flashctx, addr, ®ion);
-
- len = min(start_addr + block_len, region.end + 1) - addr;
-
- if (region.write_prot) {
- msg_gdbg("%s: cannot erase inside %s "
- "region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n",
- __func__, region.name,
- region.start, region.end,
- addr, addr + len - 1);
- free(region.name);
- continue;
- }
-
- msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is "
- "writable, erasing range (%#08x..%#08x).\n",
- __func__, region.name,
- region.start, region.end,
- addr, addr + len - 1);
- free(region.name);
-
- if (erasefn(flashctx, addr, len)) {
- ret = -1;
- goto _end;
- }
- if (check_erased_range(flashctx, addr, len)) {
- ret = - 1;
- msg_cerr("ERASE FAILED!\n");
- goto _end;
- }
- }
-
- // adjust curcontents
- memset(curcontents+start_addr, erased_value, block_len);
- // after erase make it unselected again
- erase_layout[i].layout_list[j].selected = false;
- msg_cdbg("E(%"PRIx32":%"PRIx32")", start_addr, start_addr + block_len - 1);
-
- *all_skipped = false;
+ if (region.write_prot) {
+ msg_gdbg("%s: cannot erase inside %s "
+ "region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n",
+ __func__, region.name,
+ region.start, region.end,
+ addr, addr + len - 1);
+ free(region.name);
+ continue;
}
- }
- // write
- unsigned int start_here = 0, len_here = 0, erase_len = region_end - region_start + 1;
- while ((len_here = get_next_write(curcontents + region_start + start_here,
- newcontents + region_start + start_here,
- erase_len - start_here, &start_here,
- flashctx->chip->gran))) {
- // execute write
- ret = write_flash(flashctx,
- newcontents + region_start + start_here,
- region_start + start_here, len_here);
- if (ret) {
- msg_cerr("Write failed at %#zx, Abort.\n", i);
- ret = -1;
+ msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is "
+ "writable, erasing range (%#08x..%#08x).\n",
+ __func__, region.name,
+ region.start, region.end,
+ addr, addr + len - 1);
+ free(region.name);
+
+
+ ret = erase_write_helper(flashctx, addr, addr + len - 1, curcontents, newcontents, erase_layout, all_skipped);
+ if (ret)
goto _end;
- }
-
- // adjust curcontents
- memcpy(curcontents + region_start + start_here,
- newcontents + region_start + start_here, len_here);
- msg_cdbg("W(%"PRIx32":%"PRIx32")", region_start + start_here, region_start + start_here + len_here - 1);
-
- *all_skipped = false;
}
-
_end:
if (old_start_buf) {
memcpy(newcontents + region_start, old_start_buf, start_buf_len);
diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c
index b0969c2..576923f 100644
--- a/tests/erase_func_algo.c
+++ b/tests/erase_func_algo.c
@@ -22,7 +22,7 @@
#include "libflashrom.h"
#include "programmer.h"
-#define LOG_ERASE_FUNC printf("Eraser called with blockaddr=0x%x, blocklen=0x%x\n", blockaddr, blocklen)
+#define 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)
#define LOG_READ_WRITE_FUNC printf("%s called with start=0x%x, len=0x%x\n", __func__, start, len)
#define ERASE_VALUE 0xff
@@ -48,10 +48,11 @@
uint8_t initial_buf[MOCK_CHIP_SIZE]; /* Initial state of chip memory. */
uint8_t erased_buf[MOCK_CHIP_SIZE]; /* Expected content after erase. */
uint8_t written_buf[MOCK_CHIP_SIZE]; /* Expected content after write. */
+ uint8_t written_protected_buf[MOCK_CHIP_SIZE]; /* Expected content after write with protected region. */
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 {
@@ -764,6 +765,181 @@
},
};
+
+#define START_PROTECTED_REGION 6
+#define END_PROTECTED_REGION 13
+
+/*
+ * 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, 0x6, 0x7,
+ 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, ERASE_VALUE, ERASE_VALUE},
+ .written_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7,
+ 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff},
+ .written_protected_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0x6, 0x7,
+ 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xfe, 0xff},
+ .eraseblocks_expected = {{0x4, 0x2, TEST_ERASE_INJECTOR_2}, {0x0, 0x4, TEST_ERASE_INJECTOR_3},
+ {0xe, 0x2, TEST_ERASE_INJECTOR_2}},
+ .eraseblocks_expected_ind = 3,
+ .erase_test_name = "Erase protected region test case #0",
+ .write_test_name = "Write protected region test case #0",
+ }, {
+ /*
+ * Test case #1
+ *
+ * Initial vs written: all 16 bytes are 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, 0xf6, 0xf7,
+ 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, ERASE_VALUE, ERASE_VALUE},
+ .written_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7,
+ 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf},
+ .written_protected_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xf6, 0xf7,
+ 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xae, 0xaf},
+ .eraseblocks_expected = {{0xe, 0x2, TEST_ERASE_INJECTOR_2}, {0x4, 0x2, TEST_ERASE_INJECTOR_2},
+ {0x0, 0x4, TEST_ERASE_INJECTOR_3}},
+ .eraseblocks_expected_ind = 3,
+ .erase_test_name = "Erase protected region test case #1",
+ .write_test_name = "Write protected region test case #1",
+ }, {
+ /*
+ * Test case #2
+ *
+ * Initial vs written: all 16 bytes are different.
+ * Three layout regions 8+4+4b
+ * Chip with eraseblocks 1, 2, 4, 8, 16.
+ */
+ .chip = &chip_1_2_4_8_16,
+ .regions = {{0, 7, "odd1"}, {8, 11, "odd2"}, {12, 15, "odd3"},
+ {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, 0x0, 0xff,
+ 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, ERASE_VALUE, ERASE_VALUE},
+ .written_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7,
+ 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf},
+ .written_protected_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0x0, 0xff,
+ 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, 0xae, 0xaf},
+ .eraseblocks_expected = {{0xe, 0x2, TEST_ERASE_INJECTOR_2}, {0x4, 0x2, TEST_ERASE_INJECTOR_2},
+ {0x0, 0x4, TEST_ERASE_INJECTOR_3}},
+ .eraseblocks_expected_ind = 3,
+ .erase_test_name = "Erase protected region test case #2",
+ .write_test_name = "Write 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, 2, 4, 8, 16.
+ */
+ .chip = &chip_1_2_4_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, 0x6, 0x6,
+ 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, ERASE_VALUE, ERASE_VALUE},
+ .written_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7,
+ 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf},
+ .written_protected_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0x6, 0x6,
+ 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0xae, 0xaf},
+ .eraseblocks_expected = {{0xf, 0x1, TEST_ERASE_INJECTOR_1}, {0xe, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x2, 0x2, TEST_ERASE_INJECTOR_2}, {0x4, 0x2, TEST_ERASE_INJECTOR_2},
+ {0x0, 0x2, TEST_ERASE_INJECTOR_2}},
+ .eraseblocks_expected_ind = 5,
+ .erase_test_name = "Erase protected region test case #3",
+ .write_test_name = "Write 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, 0x6, 0x6,
+ 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, ERASE_VALUE, ERASE_VALUE},
+ .written_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7,
+ 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf},
+ .written_protected_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0x6, 0x6,
+ 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0xae, 0xaf},
+ .eraseblocks_expected = {
+ {0x3, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x4, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x5, 0x1, TEST_ERASE_INJECTOR_1},
+ {0xe, 0x1, TEST_ERASE_INJECTOR_1},
+ {0xf, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x0, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x1, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x2, 0x1, TEST_ERASE_INJECTOR_1},
+ },
+ .eraseblocks_expected_ind = 8,
+ .erase_test_name = "Erase protected region test case #4",
+ .write_test_name = "Write 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 1, 8, 16.
+ */
+ .chip = &chip_1_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, 0x4, 0x4,
+ 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, ERASE_VALUE, ERASE_VALUE},
+ .written_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7,
+ 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf},
+ .written_protected_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0x4, 0x4,
+ 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, 0xae, 0xaf},
+ .eraseblocks_expected = {
+ {0xe, 0x1, TEST_ERASE_INJECTOR_1},
+ {0xf, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x0, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x1, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x2, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x3, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x4, 0x1, TEST_ERASE_INJECTOR_1},
+ {0x5, 0x1, TEST_ERASE_INJECTOR_1},
+ },
+ .eraseblocks_expected_ind = 8,
+ .erase_test_name = "Erase protected region test case #5",
+ .write_test_name = "Write protected region test case #5",
+ },
+};
+
static int setup(void **state) {
struct test_case *current_test_case = *state;
g_state.current_test_case = current_test_case;
@@ -806,6 +982,55 @@
return all_cases;
}
+static void test_erase_fails_for_unwritable_region(void **);
+static void erase_unwritable_regions_skipflag_on_test_success(void **);
+static void write_unwritable_regions_skipflag_on_test_success(void **);
+
+/*
+ * 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 num_parameterized = ARRAY_SIZE(test_cases_protected_region);
+ const size_t num_unparameterized = 1;
+ // 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;
+
+ struct CMUnitTest *all_cases = calloc(num_cases, sizeof(struct CMUnitTest));
+ *num_tests = num_cases;
+
+ for (size_t i = 0; i < num_parameterized; 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,
+ };
+ all_cases[i + num_parameterized] = (struct CMUnitTest) {
+ .name = test_cases_protected_region[i].write_test_name,
+ .setup_func = setup,
+ .teardown_func = teardown,
+ .initial_state = &test_cases_protected_region[i],
+ .test_func = write_unwritable_regions_skipflag_on_test_success,
+ };
+ }
+
+ memcpy(
+ &all_cases[num_parameterized * 2],
+ (const struct CMUnitTest[]){
+ (const struct CMUnitTest) {
+ .name = "erase failure for unskipped unwritable regions",
+ .test_func = test_erase_fails_for_unwritable_region,
+ }
+ },
+ sizeof(*all_cases) * num_unparameterized
+ );
+
+ return all_cases;
+}
+
/*
* This function is invoked for every test case in test_cases[],
* current test case is passed as an argument.
@@ -906,3 +1131,264 @@
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_protected_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 - 1;
+ 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 = END_PROTECTED_REGION + 1;
+ region->end = flashrom_flash_getsize(flash) - 1;
+ region->read_prot = false;
+ region->write_prot = false;
+ }
+}
+
+static int block_erase_chip_with_protected_region(struct flashctx *flash, enum block_erase_func erase_func, 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] = (struct erase_invoke){
+ .erase_func = erase_func,
+ .blocklen = blocklen,
+ .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.
+ 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)
+ || (blockaddr < START_PROTECTED_REGION
+ && blockaddr + blocklen + 1 > END_PROTECTED_REGION)) {
+ printf("Error: block with start=%d, len=%d 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 BLOCK_ERASE_PROTECTED_FUNC(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(flash, TEST_ERASE_INJECTOR_ ## i, blockaddr, blocklen); \
+ }
+BLOCK_ERASE_PROTECTED_FUNC(1)
+BLOCK_ERASE_PROTECTED_FUNC(2)
+BLOCK_ERASE_PROTECTED_FUNC(3)
+BLOCK_ERASE_PROTECTED_FUNC(4)
+BLOCK_ERASE_PROTECTED_FUNC(5)
+
+/*
+ * Runs the test cases that use protected flash regions (regions returned from
+ * get_flash_region() where write_prot is true) when the runtime flag to avoid
+ * writing to those regions is enabled.
+ *
+ * These tests verify that no protected region is erased, and that the erase
+ * commands used match the expected erase size (ensuring for example that a
+ * command erasing 16 bytes is not used when only 8 should be erased).
+ */
+static 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.
+ 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);
+
+ // 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);
+}
+
+/*
+ * Runs the test cases that use protected flash regions (regions returned from
+ * get_flash_region() where write_prot is true) when the runtime flag to avoid
+ * writing to those regions is enabled.
+ *
+ * These tests verify that no protected region is written, i.e. protected region
+ * memory state stays untouched.
+ */
+static void write_unwritable_regions_skipflag_on_test_success(void **state) {
+ struct test_case* current_test_case = *state;
+
+ int all_write_tests_result = 0;
+ struct flashrom_flashctx flashctx = { 0 };
+ uint8_t newcontents[MIN_BUF_SIZE];
+ const char *param = ""; /* Default values for all params. */
+
+ struct flashrom_layout *layout;
+
+ setup_chip(&flashctx, &layout, param, current_test_case);
+ memcpy(&newcontents, current_test_case->written_buf, MOCK_CHIP_SIZE);
+
+ // This test needs special block erase to emulate protected regions.
+ 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);
+ flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNREADABLE_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->write_test_name);
+ int ret = flashrom_image_write(&flashctx, &newcontents, MIN_BUF_SIZE, NULL);
+ printf("%s returned %d.\n", current_test_case->write_test_name, ret);
+
+ int chip_written = !memcmp(g_state.buf, current_test_case->written_protected_buf, MOCK_CHIP_SIZE);
+
+ if (chip_written)
+ printf("Written chip memory state for %s is CORRECT\n",
+ current_test_case->write_test_name);
+ else
+ printf("Written chip memory state for %s is WRONG\n",
+ current_test_case->write_test_name);
+
+ all_write_tests_result |= ret;
+ all_write_tests_result |= !chip_written;
+
+ teardown_chip(&layout);
+
+ assert_int_equal(0, all_write_tests_result);
+}
+
+static void test_erase_fails_for_unwritable_region(void **state) {
+ struct flashrom_flashctx flashctx = {
+ .chip = &chip_1_2_4_8_16,
+ };
+ assert_int_equal(0, programmer_init(&programmer_dummy, ""));
+ flashctx.mst = ®istered_masters[0];
+
+ flashctx.mst->spi.get_region = &get_protected_region;
+ flashctx.mst->opaque.get_region = &get_protected_region;
+ flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS, false);
+
+ /* Ask to erase one byte at the end of the unprotected region and one byte
+ * at the beginning of the protected one. If the check for unwritable regions
+ * wrongly treats the upper bound as exclusive, it will incorrectly try
+ * to erase inside the protected region. */
+ struct flashrom_layout *layout;
+ flashrom_layout_new(&layout);
+ flashrom_layout_add_region(layout, 7, 8, "protected");
+ flashrom_layout_include_region(layout, "protected");
+ flashrom_layout_set(&flashctx, layout);
+
+ int ret = flashrom_flash_erase(&flashctx);
+
+ assert_int_equal(0, programmer_shutdown());
+ flashrom_layout_release(layout);
+
+ assert_int_not_equal(ret, 0);
+}
diff --git a/tests/tests.c b/tests/tests.c
index aa8353a..165390a 100644
--- a/tests/tests.c
+++ b/tests/tests.c
@@ -514,6 +514,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 c22da12..8cf8be0 100644
--- a/tests/tests.h
+++ b/tests/tests.h
@@ -107,6 +107,7 @@
/* 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);
To view, visit change 82393. To unsubscribe, or for help writing mail filters, visit settings.