Nikolai Artemiev has uploaded this change for review.

View Change

WIP: erasure_layout: Fix get_flash_region bug

This effectively exchanges 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=builds
BUG=none
BRANCH=none

Change-Id: I5d8492351a1ac5832227cbdbe40dea7b9ac3abd1
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
---
M erasure_layout.c
1 file changed, 102 insertions(+), 92 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/85/81385/1
diff --git a/erasure_layout.c b/erasure_layout.c
index 1dd6b60..3fe0e4f 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, &region_start, &region_end);

+ if (!flashctx->flags.skip_unwritable_regions) {
+ if (check_for_unwritable_regions(flashctx, region_start, region_end - region_start))
+ return -1;
+ }
+
uint8_t *old_start_buf = NULL, *old_end_buf = NULL;
old_start_buf = (uint8_t *)malloc(old_start - region_start);
if (!old_start_buf) {
@@ -285,101 +362,34 @@
memcpy(newcontents + end_offset, curcontents + end_offset, region_end - old_end);
}

- // 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, &region);
+ len = min(region_end, region.end) - addr;

- 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, &region);
-
- len = min(start_addr + block_len, region.end) - 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 - 1,
- 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 - 1,
- 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 - 1,
+ 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 - 1,
+ addr, addr + len - 1);
+ free(region.name);
+
+
+ ret = erase_write_helper(flashctx, addr, len, 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:
memcpy(newcontents + region_start, old_start_buf, old_start - region_start);
memcpy(newcontents + old_end, old_end_buf, region_end - old_end);

To view, visit change 81385. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I5d8492351a1ac5832227cbdbe40dea7b9ac3abd1
Gerrit-Change-Number: 81385
Gerrit-PatchSet: 1
Gerrit-Owner: Nikolai Artemiev <nartemiev@google.com>
Gerrit-MessageType: newchange