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 13:
(4 comments)
Patchset:
PS13: I got some last moment thoughts...
File erasure_layout.c:
https://review.coreboot.org/c/flashrom/+/82393/comment/afce5561_615cb6d6?usp... : PS13, Line 368: for (unsigned int addr = region_start; addr <= region_end; addr += len) { I looked at this loop and got a bit suspicious. Specifically, will it properly get to the other side of protected region and continue after the end of protected region?
And I really only now thought about it. And now I am realising that current test cases do not cover this case, when protected region somewhere in the middle of the chip (which is always the case in real life). The unit test has protected region which ends right at the end of chip memory :\ To represent what I am thinking about now, it could be e.g. between bytes 6-12, and we want to make sure bytes 13-15 are also processed.
What do you think? I can try modify the test cases, maybe at the end of this week. Or if you want you can?
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/82393/comment/8fa90395_825e150d?usp... : PS9, Line 953: const size_t num_unparameterized = 1;
I don't understand what you're suggesting as an alternative. […]
I forgot what exactly was on my mind when I was asking , sorry :( Probably I was confused because parameterized tests are initialized in a loop, and non-param with a memcpy.
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/82393/comment/4e231d09_e567a183?usp... : PS13, Line 766: : #define START_PROTECTED_REGION 8 : #define END_PROTECTED_REGION 15 This was what I thought in the other comment: have this both in the middle of a chip (5-12, something like that). Which might also need to adjust layout regions. It's easier to have the same protected region, and variations of layout regions in every test case.