mirror of
https://review.coreboot.org/flashrom.git
synced 2025-07-01 06:01:16 +02:00
flashrom: only perform WP unlock for write/erase operations
Don't unlock using WP for read/verify operations because WP will only disable write locks. Most chips don't have read locks anyway, but some do, so we still call the chip's unlock function for read/verify operations. Unconditionally unlocking using WP slows down flashrom significantly with some programmers, particularly linux_mtd due to inefficiency in the current kernel MTD interface. BUG=b:283779258 BRANCH=none TEST=`ninja test` TEST=`flashrom -{r,w,E,v}` on strongbad TEST=`flashrom --wp-enable; flashrom -{w,E}` on strongbad Change-Id: I5dc66474a0b7969b51b86ac9f5daa2c95ae968f1 Signed-off-by: Nikolai Artemiev <nartemiev@google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/75991 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
This commit is contained in:
52
flashrom.c
52
flashrom.c
@ -2052,24 +2052,49 @@ static int save_initial_flash_wp(struct flashctx *const flash)
|
||||
return 0;
|
||||
}
|
||||
|
||||
static int unlock_flash_wp(struct flashctx *const flash)
|
||||
static int unlock_flash_wp(struct flashctx *const flash,
|
||||
const bool write_it, const bool erase_it)
|
||||
|
||||
{
|
||||
/* Save original WP state to be restored later */
|
||||
if (save_initial_flash_wp(flash))
|
||||
int ret = 0;
|
||||
|
||||
/* WP only disables write protection, so only use WP to unlock
|
||||
* for write/erase operations.
|
||||
*
|
||||
* For read/verify operations, we still call the chip's unlock
|
||||
* function, which may disable read locks if the chip has them.
|
||||
*/
|
||||
if (!write_it && !erase_it) {
|
||||
msg_cdbg("Skipping writeprotect-based unlocking for read/verify operations.\n");
|
||||
return -1;
|
||||
}
|
||||
|
||||
/* Save original WP state to be restored later */
|
||||
if (save_initial_flash_wp(flash)) {
|
||||
ret = -1;
|
||||
goto warn_out;
|
||||
}
|
||||
|
||||
/* Disable WP */
|
||||
struct flashrom_wp_cfg *unlocked_wp_cfg;
|
||||
if (flashrom_wp_cfg_new(&unlocked_wp_cfg) != FLASHROM_WP_OK)
|
||||
return -1;
|
||||
if (flashrom_wp_cfg_new(&unlocked_wp_cfg) != FLASHROM_WP_OK) {
|
||||
ret = -1;
|
||||
goto warn_out;
|
||||
}
|
||||
|
||||
flashrom_wp_set_range(unlocked_wp_cfg, 0, 0);
|
||||
flashrom_wp_set_mode(unlocked_wp_cfg, FLASHROM_WP_MODE_DISABLED);
|
||||
enum flashrom_wp_result ret = flashrom_wp_write_cfg(flash, unlocked_wp_cfg);
|
||||
if (flashrom_wp_write_cfg(flash, unlocked_wp_cfg) != FLASHROM_WP_OK) {
|
||||
ret = -1;
|
||||
}
|
||||
|
||||
flashrom_wp_cfg_release(unlocked_wp_cfg);
|
||||
|
||||
return (ret == FLASHROM_WP_OK) ? 0 : -1;
|
||||
warn_out:
|
||||
if (ret)
|
||||
msg_cerr("Failed to unlock flash status reg with wp support.\n");
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
int prepare_flash_access(struct flashctx *const flash,
|
||||
@ -2092,14 +2117,19 @@ int prepare_flash_access(struct flashctx *const flash,
|
||||
/* Initialize chip_restore_fn_count before chip unlock calls. */
|
||||
flash->chip_restore_fn_count = 0;
|
||||
|
||||
/* Given the existence of read locks, we want to unlock for read, erase and write. */
|
||||
int ret = 1;
|
||||
if (flash->chip->decode_range != NO_DECODE_RANGE_FUNC ||
|
||||
(flash->mst->buses_supported & BUS_PROG && flash->mst->opaque.wp_write_cfg)) {
|
||||
ret = unlock_flash_wp(flash);
|
||||
if (ret)
|
||||
msg_cerr("Failed to unlock flash status reg with wp support.\n");
|
||||
ret = unlock_flash_wp(flash, write_it, erase_it);
|
||||
}
|
||||
/*
|
||||
* Fall back to chip unlock function if we haven't already successfully
|
||||
* unlocked using WP (e.g. WP unlocking failed, chip had no WP support,
|
||||
* WP was skipped for read/verify ops).
|
||||
*
|
||||
* Given the existence of read locks, we want to unlock for read,
|
||||
* erase, write, and verify.
|
||||
*/
|
||||
blockprotect_func_t *bp_func = lookup_blockprotect_func_ptr(flash->chip);
|
||||
if (ret && bp_func)
|
||||
bp_func(flash);
|
||||
|
Reference in New Issue
Block a user