Nikolai Artemiev has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/81385?usp=email )
Change subject: WIP: erasure_layout: Fix get_flash_region bug ......................................................................
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, ®ion_start, ®ion_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, ®ion); + 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, ®ion); - - 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);
-
Nikolai Artemiev (Code Review)