Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev, Peter Marheine.
Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email )
Change subject: erasure_layout: Fix get_flash_region bug ......................................................................
Patch Set 15:
(2 comments)
Patchset:
PS15: I added tests to run write operation over the chip with protected region(s).
Few things to note:
1) FLASHROM_FLAG_SKIP_UNREADABLE_REGIONS needed to be set for write operation, otherwise write failed. I am glad to discover that flags work, but now I want one more test, or actually two tests which set write and read flags false and make sure write fails Not necessarily in this patch
2) Now we need one more buf, I called it `written_protected_buf`, the reason is, in case of protected regions the content which is asked to be written is not what the end state of the chip is: because protected regions are not touched.
File erasure_layout.c:
https://review.coreboot.org/c/flashrom/+/82393/comment/145e00d1_d89b8974?usp... : PS13, Line 368: for (unsigned int addr = region_start; addr <= region_end; addr += len) {
I made CB:82723 to illustrate, the issue repro on head […]
I think this specific issue is outside of scope of the patch: mainly because it is not relevant to protected regions. Also, it seems to be an optimisation issue: the memory state is correct, and eraseblocks are good but not perfect. Since I have a repro case on the test case without protected regions in CB:82723 , I think we can move the discussion there. This patch should not be blocked by it.
Do you agree?
-
Anastasia Klimchuk (Code Review)