Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/74930 )
Change subject: flashrom: Use WP-based unlocking on opaque masters ......................................................................
flashrom: Use WP-based unlocking on opaque masters
Flashrom only tries to use WP-based unlocking if it detects that WP operations are supported. However WP support was detected in a way that ignored WP operations provided by opaque masters.
This stopped flashrom from automatically unlocking with some opaque masters, particularly linux_mtd.
This commit also deletes part of a test that required the chip unlock function to be called before read/write/erase operations because WP unlocking is now used instead of chip unlocking.
BUG=b:280111380 BRANCH=none TEST=Checked flashrom automatically unlocked flash on strongbad (MTD)
Change-Id: I1774ad64d82ae47cd085df6045e17e283855c01f Signed-off-by: Nikolai Artemiev nartemiev@google.com Reviewed-on: https://review.coreboot.org/c/flashrom/+/74930 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org Reviewed-by: Sergii Dmytruk sergii.dmytruk@3mdeb.com --- M flashrom.c M include/flash.h M spi25_statusreg.c M tests/chip.c 4 files changed, 32 insertions(+), 42 deletions(-)
Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Sergii Dmytruk: Looks good to me, approved
diff --git a/flashrom.c b/flashrom.c index 07ed0df..faffe51 100644 --- a/flashrom.c +++ b/flashrom.c @@ -2093,7 +2093,8 @@
/* 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) { + 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"); diff --git a/include/flash.h b/include/flash.h index 3e9c885..0eace15 100644 --- a/include/flash.h +++ b/include/flash.h @@ -348,7 +348,6 @@ UNLOCK_LH28F008BJT, UNLOCK_SST_FWHUB, UNPROTECT_28SF040, - TEST_UNLOCK_INJECTOR, /* special case must come last. */ };
enum printlock_func { diff --git a/spi25_statusreg.c b/spi25_statusreg.c index 60e0b46..fa276f7 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -914,9 +914,6 @@ return spi_disable_blockprotect_bp2_srwd(flash); }
-/* special unit-test hook */ -blockprotect_func_t *g_test_unlock_injector; - blockprotect_func_t *lookup_blockprotect_func_ptr(const struct flashchip *const chip) { switch (chip->unlock) { @@ -948,7 +945,6 @@ return lookup_82802ab_blockprotect_func_ptr(chip); case UNLOCK_SST_FWHUB: return unlock_sst_fwhub; /* sst_fwhub.c */ case UNPROTECT_28SF040: return unprotect_28sf040; /* sst28sf040.c */ - case TEST_UNLOCK_INJECTOR: return g_test_unlock_injector; /* default: non-total function, 0 indicates no unlock function set. * We explicitly do not want a default catch-all case in the switch * to ensure unhandled enum's are compiler warnings. diff --git a/tests/chip.c b/tests/chip.c index 6a17d86..96be7b1 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -20,7 +20,7 @@ * Example of test: erase_chip_test_success. * * 2) Mock chip operations backed by `dummyflasher` emulation. - * Dummyflasher controls chip state and emulates read/write/unlock/erase. + * Dummyflasher controls chip state and emulates read/write/erase. * `g_chip_state` is NOT used for this type of tests. * Example of test: erase_chip_with_dummyflasher_test_success. */ @@ -40,20 +40,14 @@ #define MOCK_CHIP_CONTENT 0xCC /* 0x00 is a zeroed heap and 0xFF is an erased chip. */
static struct { - unsigned int unlock_calls; /* how many times unlock function was called */ uint8_t buf[MOCK_CHIP_SIZE]; /* buffer of total size of chip, to emulate a chip */ } g_chip_state = { - .unlock_calls = 0, .buf = { 0 }, };
static int read_chip(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len) { printf("Read chip called with start=0x%x, len=0x%x\n", start, len); - if (!g_chip_state.unlock_calls) { - printf("Error while reading chip: unlock was not called.\n"); - return 1; - }
assert_in_range(start + len, 0, MOCK_CHIP_SIZE);
@@ -64,10 +58,6 @@ static int write_chip(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len) { printf("Write chip called with start=0x%x, len=0x%x\n", start, len); - if (!g_chip_state.unlock_calls) { - printf("Error while writing chip: unlock was not called.\n"); - return 1; - }
assert_in_range(start + len, 0, MOCK_CHIP_SIZE);
@@ -75,26 +65,9 @@ return 0; }
-static int unlock_chip(struct flashctx *flash) -{ - printf("Unlock chip called\n"); - g_chip_state.unlock_calls++; - - if (g_chip_state.unlock_calls > 1) { - printf("Error: Unlock called twice\n"); - return -1; - } - - return 0; -} - static int block_erase_chip(struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen) { printf("Block erase called with blockaddr=0x%x, blocklen=0x%x\n", blockaddr, blocklen); - if (!g_chip_state.unlock_calls) { - printf("Error while erasing chip: unlock was not called.\n"); - return 1; - }
assert_in_range(blockaddr + blocklen, 0, MOCK_CHIP_SIZE);
@@ -109,7 +82,6 @@
flashctx->chip = chip;
- g_chip_state.unlock_calls = 0; memset(g_chip_state.buf, MOCK_CHIP_CONTENT, sizeof(g_chip_state.buf));
printf("Creating layout with one included region... "); @@ -149,7 +121,6 @@ extern write_func_t *g_test_write_injector; extern read_func_t *g_test_read_injector; extern erasefunc_t *g_test_erase_injector; -extern blockprotect_func_t *g_test_unlock_injector;
static const struct flashchip chip_8MiB = { .vendor = "aklm", @@ -157,7 +128,6 @@ .tested = TEST_OK_PREW, .read = TEST_READ_INJECTOR, .write = TEST_WRITE_INJECTOR, - .unlock = TEST_UNLOCK_INJECTOR, .block_erasers = {{ /* All blocks within total size of the chip. */ @@ -173,7 +143,6 @@ .tested = TEST_OK_PREW, .read = SPI_CHIP_READ, .write = SPI_CHIP_WRITE256, - .unlock = SPI_DISABLE_BLOCKPROTECT, .page_size = 256, .block_erasers = { @@ -211,7 +180,6 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; g_test_erase_injector = block_erase_chip; - g_test_unlock_injector = unlock_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -271,7 +239,6 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; g_test_erase_injector = block_erase_chip; - g_test_unlock_injector = unlock_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -347,7 +314,6 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; g_test_erase_injector = block_erase_chip; - g_test_unlock_injector = unlock_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -539,7 +505,6 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; g_test_erase_injector = block_erase_chip; - g_test_unlock_injector = unlock_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB;