Attention is currently required from: Brian Norris, Edward O'Callaghan.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75991?usp=email )
Change subject: flashrom: only unlock for write/erase operations ......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I couldn't figure out whether this got here for a good reason. […]
I checked the unlock functions, it turns out one of them actually does unlock for reads (last bullet).
Most unlock functions call `spi_disable_blockprotect_generic()`, which only tries to clear bits in SR1. I've never seen a chip with SR1-controlled read protection, chips that do have read protection usually control it via special registers or command sequences. So these are probably safe.
There are four unlock functions that don't call `spi_disable_blockprotect_generic()`:
- `UNPROTECT_28SF040` - used by SST28SF040A - I don't have datasheet, it looks like a very old parallel flash. I would guess it doesn't have read protection.
- `UNLOCK_SST_FWHUB` - used by several chips - the SST49LF0008A datasheet doesn't indicate it has read protection.
- `SPI_DISABLE_BLOCKPROTECT_AT45DB` - used by several chips - the AT45DB321E datasheet doesn't indicate it has read protection.
- `SPI_DISABLE_BLOCKPROTECT_SST26_GLOBAL_UNPROTECT` - used by several chips - according to the SST26VF032B datasheet, there is sector based read and write protection that can be globally cleared with a ULBPR (98h) instruciton, which is what the unlock function does.
Maybe we can work it around with something like a `FEATURE_UNLOCK_FOR_READ` chip flag?
-
Nikolai Artemiev (Code Review)