Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/83846?usp=email )
Change subject: flashrom.c: Delete legacy erase and write logic ......................................................................
flashrom.c: Delete legacy erase and write logic
Current code path for erase and write has been enabled in the tree since May 2023, which is more than 1 year ago (15 months ago), and legacy path has been disabled since the same time.
Current logic has been officially released in v1.4.0 in July 2024.
Change-Id: I08fd686fecf6a5313eea2d66b368661c664f4800 Signed-off-by: Anastasia Klimchuk aklm@flashrom.org Reviewed-on: https://review.coreboot.org/c/flashrom/+/83846 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Aarya aarya.chaumal@gmail.com Reviewed-by: Peter Marheine pmarheine@chromium.org --- M erasure_layout.c M flashrom.c 2 files changed, 1 insertion(+), 369 deletions(-)
Approvals: Aarya: Looks good to me, approved build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved
diff --git a/erasure_layout.c b/erasure_layout.c index ab54c12..a7eaa2d 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -151,7 +151,7 @@ * @param region_start pointer to start address of the region to align * @param region_end pointer to end address of the region to align * - * This function aligns start and end address of the region (in struct walk_info) + * This function aligns start and end address of the region * to some erase sector boundaries and modify the region start and end addresses * to match nearest erase sector boundaries. This function will be used in the * new algorithm for erase function selection. diff --git a/flashrom.c b/flashrom.c index dbf06cf..453a2bf 100644 --- a/flashrom.c +++ b/flashrom.c @@ -36,8 +36,6 @@ #include "chipdrivers.h" #include "erasure_layout.h"
-static bool use_legacy_erase_path = false; - const char flashrom_version[] = FLASHROM_VERSION;
static const struct programmer_entry *programmer = NULL; @@ -1329,250 +1327,6 @@ return ret; }
-typedef int (*erasefn_t)(struct flashctx *, unsigned int addr, unsigned int len); -/** - * @private - * - * For read-erase-write, `curcontents` and `newcontents` shall point - * to buffers of the chip's size. Both are supposed to be prefilled - * with at least the included layout regions of the current flash - * contents (`curcontents`) and the data to be written to the flash - * (`newcontents`). - * - * For erase, `curcontents` and `newcontents` shall be NULL-pointers. - * - * The `chipoff_t` values are used internally by `walk_by_layout()`. - */ -struct walk_info { - uint8_t *curcontents; - const uint8_t *newcontents; - chipoff_t region_start; - chipoff_t region_end; - chipoff_t erase_start; - chipoff_t erase_end; -}; -/* returns 0 on success, 1 to retry with another erase function, 2 for immediate abort */ -typedef int (*per_blockfn_t)(struct flashctx *, const struct walk_info *, erasefn_t, bool *); - -static int walk_eraseblocks(struct flashctx *const flashctx, - struct walk_info *const info, - const size_t erasefunction, const per_blockfn_t per_blockfn, - bool *all_skipped) -{ - int ret; - size_t i, j; - bool first = true; - struct block_eraser *const eraser = &flashctx->chip->block_erasers[erasefunction]; - - info->erase_start = 0; - for (i = 0; i < NUM_ERASEREGIONS; ++i) { - /* count==0 for all automatically initialized array - members so the loop below won't be executed for them. */ - for (j = 0; j < eraser->eraseblocks[i].count; ++j, info->erase_start = info->erase_end + 1) { - info->erase_end = info->erase_start + eraser->eraseblocks[i].size - 1; - - /* Skip any eraseblock that is completely outside the current region. */ - if (info->erase_end < info->region_start) - continue; - if (info->region_end < info->erase_start) - break; - - /* Print this for every block except the first one. */ - if (first) - first = false; - else - msg_cdbg(", "); - msg_cdbg("0x%06"PRIx32"-0x%06"PRIx32":", info->erase_start, info->erase_end); - - erasefunc_t *erase_func = lookup_erase_func_ptr(eraser); - ret = per_blockfn(flashctx, info, erase_func, all_skipped); - if (ret) - return ret; - } - if (info->region_end < info->erase_start) - break; - } - msg_cdbg("\n"); - return 0; -} - -static int walk_by_layout(struct flashctx *const flashctx, struct walk_info *const info, - const per_blockfn_t per_blockfn, bool *all_skipped) -{ - const struct flashrom_layout *const layout = get_layout(flashctx); - const struct romentry *entry = NULL; - - *all_skipped = true; - msg_cinfo("Erasing and writing flash chip... "); - - while ((entry = layout_next_included(layout, entry))) { - const struct flash_region *region = &entry->region; - info->region_start = region->start; - info->region_end = region->end; - - size_t j; - int error = 1; /* retry as long as it's 1 */ - for (j = 0; j < NUM_ERASEFUNCTIONS; ++j) { - if (j != 0) - msg_cinfo("Looking for another erase function.\n"); - msg_cdbg("Trying erase function %zi... ", j); - if (check_block_eraser(flashctx, j, 1)) - continue; - - error = walk_eraseblocks(flashctx, info, j, per_blockfn, all_skipped); - if (error != 1) - break; - - if (info->curcontents) { - msg_cinfo("Reading current flash chip contents... "); - if (read_by_layout(flashctx, info->curcontents)) { - /* Now we are truly screwed. Read failed as well. */ - msg_cerr("Can't read anymore! Aborting.\n"); - /* We have no idea about the flash chip contents, so - retrying with another erase function is pointless. */ - error = 2; - break; - } - msg_cinfo("done. "); - } - } - if (error == 1) - msg_cinfo("No usable erase functions left.\n"); - if (error) { - msg_cerr("FAILED!\n"); - return 1; - } - } - if (*all_skipped) - msg_cinfo("\nWarning: Chip content is identical to the requested image.\n"); - msg_cinfo("Erase/write done.\n"); - return 0; -} - -static int erase_block(struct flashctx *const flashctx, - const struct walk_info *const info, const erasefn_t erasefn, - bool *all_skipped) -{ - const unsigned int erase_len = info->erase_end + 1 - info->erase_start; - const bool region_unaligned = info->region_start > info->erase_start || - info->erase_end > info->region_end; - uint8_t *backup_contents = NULL, *erased_contents = NULL; - int ret = 2; - - /* - * If the region is not erase-block aligned, merge current flash con- - * tents into a new buffer `backup_contents`. - */ - if (region_unaligned) { - backup_contents = malloc(erase_len); - erased_contents = malloc(erase_len); - if (!backup_contents || !erased_contents) { - msg_cerr("Out of memory!\n"); - ret = 1; - goto _free_ret; - } - memset(backup_contents, ERASED_VALUE(flashctx), erase_len); - memset(erased_contents, ERASED_VALUE(flashctx), erase_len); - - msg_cdbg("R"); - /* Merge data preceding the current region. */ - if (info->region_start > info->erase_start) { - const chipoff_t start = info->erase_start; - const chipsize_t len = info->region_start - info->erase_start; - if (read_flash(flashctx, backup_contents, start, len)) { - msg_cerr("Can't read! Aborting.\n"); - goto _free_ret; - } - } - /* Merge data following the current region. */ - if (info->erase_end > info->region_end) { - const chipoff_t start = info->region_end + 1; - const chipoff_t rel_start = start - info->erase_start; /* within this erase block */ - const chipsize_t len = info->erase_end - info->region_end; - if (read_flash(flashctx, backup_contents + rel_start, start, len)) { - msg_cerr("Can't read! Aborting.\n"); - goto _free_ret; - } - } - } - - ret = 1; - *all_skipped = false; - - msg_cdbg("E"); - - if (!flashctx->flags.skip_unwritable_regions) { - if (check_for_unwritable_regions(flashctx, info->erase_start, erase_len)) - goto _free_ret; - } - - unsigned int len; - for (unsigned int addr = info->erase_start; addr < info->erase_start + erase_len; addr += len) { - struct flash_region region; - get_flash_region(flashctx, addr, ®ion); - - len = min(info->erase_start + erase_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)) - goto _free_ret; - if (check_erased_range(flashctx, addr, len)) { - msg_cerr("ERASE FAILED!\n"); - goto _free_ret; - } - } - - - if (region_unaligned) { - unsigned int starthere = 0, lenhere = 0, writecount = 0; - /* get_next_write() sets starthere to a new value after the call. */ - while ((lenhere = get_next_write(erased_contents + starthere, backup_contents + starthere, - erase_len - starthere, &starthere, flashctx->chip->gran))) { - if (!writecount++) - msg_cdbg("W"); - /* Needs the partial write function signature. */ - if (write_flash(flashctx, backup_contents + starthere, - info->erase_start + starthere, lenhere)) - goto _free_ret; - starthere += lenhere; - } - } - - ret = 0; - -_free_ret: - free(erased_contents); - free(backup_contents); - return ret; -} - -/** - * @brief Erases the included layout regions. - * - * If there is no layout set in the given flash context, the whole chip will - * be erased. - * - * @param flashctx Flash context to be used. - * @return 0 on success, - * 1 if all available erase functions failed. - */ -static int erase_by_layout_legacy(struct flashctx *const flashctx) -{ - struct walk_info info = { 0 }; - bool all_skipped = true; - return walk_by_layout(flashctx, &info, &erase_block, &all_skipped); -} - static int erase_by_layout_new(struct flashctx *const flashctx) { bool all_skipped = true; @@ -1618,129 +1372,9 @@
static int erase_by_layout(struct flashctx *const flashctx) { - if (use_legacy_erase_path) - return erase_by_layout_legacy(flashctx); return erase_by_layout_new(flashctx); }
-static int read_erase_write_block(struct flashctx *const flashctx, - const struct walk_info *const info, const erasefn_t erasefn, - bool *all_skipped) -{ - const chipsize_t erase_len = info->erase_end + 1 - info->erase_start; - const bool region_unaligned = info->region_start > info->erase_start || - info->erase_end > info->region_end; - const uint8_t *newcontents = NULL; - int ret = 2; - - /* - * If the region is not erase-block aligned, merge current flash con- - * tents into `info->curcontents` and a new buffer `newc`. The former - * is necessary since we have no guarantee that the full erase block - * was already read into `info->curcontents`. For the latter a new - * buffer is used since `info->newcontents` might contain data for - * other unaligned regions that touch this erase block too. - */ - if (region_unaligned) { - msg_cdbg("R"); - uint8_t *const newc = malloc(erase_len); - if (!newc) { - msg_cerr("Out of memory!\n"); - return 1; - } - memcpy(newc, info->newcontents + info->erase_start, erase_len); - - /* Merge data preceding the current region. */ - if (info->region_start > info->erase_start) { - const chipoff_t start = info->erase_start; - const chipsize_t len = info->region_start - info->erase_start; - if (read_flash(flashctx, newc, start, len)) { - msg_cerr("Can't read! Aborting.\n"); - goto _free_ret; - } - memcpy(info->curcontents + start, newc, len); - } - /* Merge data following the current region. */ - if (info->erase_end > info->region_end) { - const chipoff_t start = info->region_end + 1; - const chipoff_t rel_start = start - info->erase_start; /* within this erase block */ - const chipsize_t len = info->erase_end - info->region_end; - if (read_flash(flashctx, newc + rel_start, start, len)) { - msg_cerr("Can't read! Aborting.\n"); - goto _free_ret; - } - memcpy(info->curcontents + start, newc + rel_start, len); - } - - newcontents = newc; - } else { - newcontents = info->newcontents + info->erase_start; - } - - ret = 1; - bool skipped = true; - uint8_t *const curcontents = info->curcontents + info->erase_start; - const uint8_t erased_value = ERASED_VALUE(flashctx); - if (!(flashctx->chip->feature_bits & FEATURE_NO_ERASE) && - need_erase(curcontents, newcontents, erase_len, flashctx->chip->gran, erased_value)) { - if (erase_block(flashctx, info, erasefn, all_skipped)) - goto _free_ret; - /* Erase was successful. Adjust curcontents. */ - memset(curcontents, erased_value, erase_len); - skipped = false; - } - - unsigned int starthere = 0, lenhere = 0, writecount = 0; - /* get_next_write() sets starthere to a new value after the call. */ - while ((lenhere = get_next_write(curcontents + starthere, newcontents + starthere, - erase_len - starthere, &starthere, flashctx->chip->gran))) { - if (!writecount++) - msg_cdbg("W"); - /* Needs the partial write function signature. */ - if (write_flash(flashctx, newcontents + starthere, - info->erase_start + starthere, lenhere)) - goto _free_ret; - starthere += lenhere; - skipped = false; - } - if (skipped) - msg_cdbg("S"); - else - *all_skipped = false; - - /* Update curcontents, other regions with overlapping erase blocks - might rely on this. */ - memcpy(curcontents, newcontents, erase_len); - ret = 0; - -_free_ret: - if (region_unaligned) - free((void *)newcontents); - return ret; -} - -/** - * @brief Writes the included layout regions from a given image. - * - * If there is no layout set in the given flash context, the whole image - * will be written. - * - * @param flashctx Flash context to be used. - * @param curcontents A buffer of full chip size with current chip contents of included regions. - * @param newcontents The new image to be written. - * @return 0 on success, - * 1 if anything has gone wrong. - */ -static int write_by_layout_legacy(struct flashctx *const flashctx, - void *const curcontents, const void *const newcontents, - bool *all_skipped) -{ - struct walk_info info; - info.curcontents = curcontents; - info.newcontents = newcontents; - return walk_by_layout(flashctx, &info, read_erase_write_block, all_skipped); -} - static int write_by_layout_new(struct flashctx *const flashctx, void *const curcontents, const void *const newcontents, bool *all_skipped) @@ -1780,8 +1414,6 @@ uint8_t *const curcontents, const uint8_t *const newcontents, bool *all_skipped) { - if (use_legacy_erase_path) - return write_by_layout_legacy(flashctx, curcontents, newcontents, all_skipped); return write_by_layout_new(flashctx, curcontents, newcontents, all_skipped); }
-
Anastasia Klimchuk (Code Review)