May 2024 - flashrom-gerrit - mail.coreboot.org

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Anastasia Klimchuk (Code Review) 31 May '24

31 May '24

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 14: (1 comment) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/82393/comment/fecea642_135db866?us… : PS13, Line 368: for (unsigned int addr = region_start; addr <= region_end; addr += len) { > I have changed tests, and looked into debugging messages, and now I have a piece of mind that the bu […] I made CB:82723 to illustrate, the issue repro on head Now the question is, how urgent the issue is. The memory is erased correctly, but not in an optimal way. Peter what do you think? (also for this latest patchset, Jenkins is not adding a comment, but the results are here https://qa.coreboot.org/job/flashrom_gerrit/11678/console) -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 14 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 31 May 2024 11:55:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: [WIP] Erase blocks are selected smaller than expected
by Anastasia Klimchuk (Code Review) 31 May '24

31 May '24

Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/82723?usp=email ) Change subject: [WIP] Erase blocks are selected smaller than expected ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Running this on head, erase blocks are more granular than expected, specifically: 1017 [ RUN ] Erase test case #6 1018 Creating layout ... done 1019 Dummyflasher initialising with param=""... done 1020 Erase test case #6 started. 1021 Eraser called with blockaddr=0xf, blocklen=0x1 1022 read_chip called with start=0xf, len=0x1 1023 Eraser called with blockaddr=0xe, blocklen=0x1 1024 read_chip called with start=0xe, len=0x1 1025 Eraser called with blockaddr=0x6, blocklen=0x2 1026 read_chip called with start=0x6, len=0x2 1027 Eraser called with blockaddr=0xc, blocklen=0x2 1028 read_chip called with start=0xc, len=0x2 1029 Eraser called with blockaddr=0x8, blocklen=0x4 1030 read_chip called with start=0x8, len=0x4 1031 Eraser called with blockaddr=0x2, blocklen=0x2 1032 read_chip called with start=0x2, len=0x2 1033 Eraser called with blockaddr=0x4, blocklen=0x2 1034 read_chip called with start=0x4, len=0x2 1035 Eraser called with blockaddr=0x0, blocklen=0x2 1036 read_chip called with start=0x0, len=0x2 1037 Erase test case #6 returned 0. 1038 Erased chip memory state for Erase test case #6 is CORRECT 1039 Eraseblocks order of invocation for Erase test case #6 is WRONG 1040 Eraseblocks number of invocations for Erase test case #6 is WRONG, expected 4 actual 8 1021 Eraser called with blockaddr=0xf, blocklen=0x1 1023 Eraser called with blockaddr=0xe, blocklen=0x1 -> this could be one eraseblock of size 2 1031 Eraser called with blockaddr=0x2, blocklen=0x2 1033 Eraser called with blockaddr=0x4, blocklen=0x2 -> this could be one eraseblock of size 4 -- To view, visit https://review.coreboot.org/c/flashrom/+/82723?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I47851d2b6106075111babf8c3535e5394cbbe0f9 Gerrit-Change-Number: 82723 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 31 May 2024 11:51:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in flashrom[main]: [WIP] Erase blocks are selected smaller than expected
by Anastasia Klimchuk (Code Review) 31 May '24

31 May '24

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/82723?usp=email ) Change subject: [WIP] Erase blocks are selected smaller than expected ...................................................................... [WIP] Erase blocks are selected smaller than expected Change-Id: I47851d2b6106075111babf8c3535e5394cbbe0f9 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M tests/erase_func_algo.c 1 file changed, 5 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/82723/1 diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index b0969c2..83f27b6 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -415,11 +415,11 @@ * Test case #6 * * Initial vs written: 7 bytes the same, 1 bytes different, 8 bytes the same. - * One layout region covering the whole chip. + * Layout regions of size 2+4+10b. * Chip with eraseblocks 1, 2, 4, 8, 16. */ .chip = &chip_1_2_4_8_16, - .regions = {{0, MIN_REAL_CHIP_SIZE - 1, "whole chip"}}, + .regions = {{0, 1, "reg1"}, {2, 5, "reg2"}, {6, 15, "reg3"}, {15, MIN_REAL_CHIP_SIZE - 1, "tail"}}, .initial_buf = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x1d, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, @@ -428,8 +428,9 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0xdd, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, - .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, - .eraseblocks_expected_ind = 1, + .eraseblocks_expected = {{0x6, 0x8, TEST_ERASE_INJECTOR_4}, {0xe, 0x2, TEST_ERASE_INJECTOR_2}, + {0x2, 0x4, TEST_ERASE_INJECTOR_3}, {0x0, 0x2, TEST_ERASE_INJECTOR_2}}, + .eraseblocks_expected_ind = 4, .erase_test_name = "Erase test case #6", .write_test_name = "Write test case #6", }, { -- To view, visit https://review.coreboot.org/c/flashrom/+/82723?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I47851d2b6106075111babf8c3535e5394cbbe0f9 Gerrit-Change-Number: 82723 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Anastasia Klimchuk (Code Review) 31 May '24

31 May '24

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 14: (2 comments) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/82393/comment/6598daea_c861aedb?us… : PS13, Line 368: for (unsigned int addr = region_start; addr <= region_end; addr += len) { > I looked at this loop and got a bit suspicious. […] I have changed tests, and looked into debugging messages, and now I have a piece of mind that the bug that I suspected is not present :) Happy ending! However after modifying tests I found something else suspicious: now "Erase protected region test case #3" behaves not as I expect. Specifically, there is a 4byte layout region, outside of protected area. I expect it to be erased by 4byte eraser, instead it is erased by 2b + 2b two erasers. The end state of memory is correct, but the behaviour is not optimal. Maybe that's a double happy ending, if I found a different bug :D I uploaded the patchset 14 to have a history of changes, but now I will try to debug "Erase protected region test case #3". File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/82393/comment/4df28d74_823d2acc?us… : 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, somethin […] Done (protected region spans bytes 6-13) -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 14 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 31 May 2024 10:09:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Anastasia Klimchuk (Code Review) 31 May '24

31 May '24

Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has uploaded a new patch set (#14) to the change originally created by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email ) The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: erasure_layout: Fix get_flash_region bug ...................................................................... erasure_layout: Fix get_flash_region bug When flash regions are protected, erase could incorrectly erase regions which were meant to be protected by requesting the correct size but using an erase opcode with coarser granularity than desired (for instance using a 16-byte erase command while attempting to erase only 8 bytes). This fixes that by exchanging the nesting of the loops over erase blocks and flash regions. Old: - Select erasefns - Loop over blocks to erase for each selected erasefn - Loop over programmer flash regions within erase block - Erase regions (may fail since selected erasefn will be too big if flash region is smaller than erase block) New: - Loop over programmer flash regions within erase block - Select erasefns within programer flash region - Loop over blocks to erase for each selected erasefn - Erase regions Eraser selection and erasing has also been factored out into a helper function to manage nesting depth. TEST=New test cases pass, whereas some of them fail without the changes to erasure_layout.c BUG=https://ticket.coreboot.org/issues/525 Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Co-authored-by: Nikolai Artemiev <nartemiev(a)google.com> Co-authored-by: Anastasia Klimchuk <aklm(a)flashrom.org> Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> --- M erasure_layout.c M tests/erase_func_algo.c M tests/tests.c M tests/tests.h 4 files changed, 495 insertions(+), 96 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/82393/14 -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 14 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>

1 0

[S] Change in flashrom[main]: Correct get_flash_region() to use inclusive upper bounds
by Peter Marheine (Code Review) 31 May '24

31 May '24

Peter Marheine has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82496?usp=email ) Change subject: Correct get_flash_region() to use inclusive upper bounds ...................................................................... Correct get_flash_region() to use inclusive upper bounds get_flash_region() emits a struct flash_region, which uses chipoff_t for the start and end addresses of a region. chipoff_t is defined as a valid flash address, so it was wrong to be setting the end address to start + len; this is clearly wrong in the case where there is a single region because setting end to the flash size generates an address that is beyond the end of the chip (due to zero-indexing). This changes the one actual implementation of .get_region in ichspi.c to use inclusive upper bounds, and corrects all callers of get_flash_region() to treat the upper bounds as inclusive. Overall this reduces complexity slightly by removing more downward adjustments by 1 than it needs to add upward adjustments. TEST=on yaviks, `flashrom -V -x` prints equivalent messages about "x region (0xZZZZ..0xZZZZ) is readable" before and after this patch. Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82496 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Hsuan-ting Chen <roccochen(a)google.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M erasure_layout.c M flashrom.c M ichspi.c 3 files changed, 22 insertions(+), 22 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved Hsuan-ting Chen: Looks good to me, but someone else must approve diff --git a/erasure_layout.c b/erasure_layout.c index e9bfa86..d15594d 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -318,13 +318,13 @@ struct flash_region region; get_flash_region(flashctx, addr, &region); - len = min(start_addr + block_len, region.end) - addr; + len = min(start_addr + block_len, region.end + 1) - addr; if (region.write_prot) { msg_gdbg("%s: cannot erase inside %s " "region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n", __func__, region.name, - region.start, region.end - 1, + region.start, region.end, addr, addr + len - 1); free(region.name); continue; @@ -333,7 +333,7 @@ msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is " "writable, erasing range (%#08x..%#08x).\n", __func__, region.name, - region.start, region.end - 1, + region.start, region.end, addr, addr + len - 1); free(region.name); diff --git a/flashrom.c b/flashrom.c index a056929..dbf06cf 100644 --- a/flashrom.c +++ b/flashrom.c @@ -379,7 +379,7 @@ } else { region->name = strdup(""); region->start = 0; - region->end = flashrom_flash_getsize(flash); + region->end = flashrom_flash_getsize(flash) - 1; region->read_prot = false; region->write_prot = false; } @@ -388,12 +388,12 @@ int check_for_unwritable_regions(const struct flashctx *flash, unsigned int start, unsigned int len) { struct flash_region region; - for (unsigned int addr = start; addr < start + len; addr = region.end) { + for (unsigned int addr = start; addr < start + len; addr = region.end + 1) { get_flash_region(flash, addr, &region); if (region.write_prot) { msg_gerr("%s: cannot write/erase inside %s region (%#08"PRIx32"..%#08"PRIx32").\n", - __func__, region.name, region.start, region.end - 1); + __func__, region.name, region.start, region.end); free(region.name); return -1; } @@ -596,14 +596,14 @@ struct flash_region region; get_flash_region(flash, addr, &region); - read_len = min(start + len, region.end) - addr; + read_len = min(start + len, region.end + 1) - addr; uint8_t *rbuf = buf + addr - start; if (region.read_prot) { if (flash->flags.skip_unreadable_regions) { msg_gdbg("%s: cannot read inside %s region (%#08"PRIx32"..%#08"PRIx32"), " "filling (%#08x..%#08x) with erased value instead.\n", - __func__, region.name, region.start, region.end - 1, + __func__, region.name, region.start, region.end, addr, addr + read_len - 1); free(region.name); @@ -612,12 +612,12 @@ } msg_gerr("%s: cannot read inside %s region (%#08"PRIx32"..%#08"PRIx32").\n", - __func__, region.name, region.start, region.end - 1); + __func__, region.name, region.start, region.end); free(region.name); return -1; } msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is readable, reading range (%#08x..%#08x).\n", - __func__, region.name, region.start, region.end - 1, addr, addr + read_len - 1); + __func__, region.name, region.start, region.end, addr, addr + read_len - 1); free(region.name); read_func_t *read_func = lookup_read_func_ptr(flash->chip); @@ -665,25 +665,25 @@ for (size_t addr = start; addr < start + len; addr += read_len) { struct flash_region region; get_flash_region(flash, addr, &region); - read_len = min(start + len, region.end) - addr; + read_len = min(start + len, region.end + 1) - addr; if ((region.write_prot && flash->flags.skip_unwritable_regions) || (region.read_prot && flash->flags.skip_unreadable_regions)) { msg_gdbg("%s: Skipping verification of %s region (%#08"PRIx32"..%#08"PRIx32")\n", - __func__, region.name, region.start, region.end - 1); + __func__, region.name, region.start, region.end); free(region.name); continue; } if (region.read_prot) { msg_gerr("%s: Verification imposible because %s region (%#08"PRIx32"..%#08"PRIx32") is unreadable.\n", - __func__, region.name, region.start, region.end - 1); + __func__, region.name, region.start, region.end); free(region.name); goto out_free; } msg_gdbg("%s: Verifying %s region (%#08"PRIx32"..%#08"PRIx32")\n", - __func__, region.name, region.start, region.end - 1); + __func__, region.name, region.start, region.end); free(region.name); ret = read_flash(flash, readbuf, addr, read_len); @@ -1046,18 +1046,18 @@ struct flash_region region; get_flash_region(flash, addr, &region); - write_len = min(start + len, region.end) - addr; + write_len = min(start + len, region.end + 1) - addr; const uint8_t *rbuf = buf + addr - start; if (region.write_prot) { msg_gdbg("%s: cannot write inside %s region (%#08"PRIx32"..%#08"PRIx32"), skipping (%#08x..%#08x).\n", - __func__, region.name, region.start, region.end - 1, addr, addr + write_len - 1); + __func__, region.name, region.start, region.end, addr, addr + write_len - 1); free(region.name); continue; } msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is writable, writing range (%#08x..%#08x).\n", - __func__, region.name, region.start, region.end - 1, addr, addr + write_len - 1); + __func__, region.name, region.start, region.end, addr, addr + write_len - 1); write_func_t *write_func = lookup_write_func_ptr(flash->chip); int ret = write_func(flash, rbuf, addr, write_len); @@ -1511,17 +1511,17 @@ struct flash_region region; get_flash_region(flashctx, addr, &region); - len = min(info->erase_start + erase_len, region.end) - addr; + len = min(info->erase_start + erase_len, region.end + 1) - addr; if (region.write_prot) { msg_gdbg("%s: cannot erase inside %s region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n", - __func__, region.name, region.start, region.end - 1, addr, addr + len - 1); + __func__, region.name, region.start, region.end, addr, addr + len - 1); free(region.name); continue; } msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is writable, erasing range (%#08x..%#08x).\n", - __func__, region.name, region.start, region.end - 1, addr, addr + len - 1); + __func__, region.name, region.start, region.end, addr, addr + len - 1); free(region.name); if (erasefn(flashctx, addr, len)) diff --git a/ichspi.c b/ichspi.c index bef43b9..cc56e0c 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1436,7 +1436,7 @@ region->read_prot = false; region->write_prot = false; region->start = 0; - region->end = flashrom_flash_getsize(flash); + region->end = flashrom_flash_getsize(flash) - 1; for (ssize_t i = 0; i < nr; i++) { uint32_t base = fd_regions[i].base; @@ -1459,7 +1459,7 @@ /* fd_regions[i] contains addr, copy to *region. */ name = fd_regions[i].name; region->start = base; - region->end = limit + 1; + region->end = limit; region->read_prot = (level == LOCKED) || (level == READ_PROT); region->write_prot = (level == LOCKED) || (level == WRITE_PROT); break; -- To view, visit https://review.coreboot.org/c/flashrom/+/82496?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia0ca867aef33b598c2697fc264807fa5e29683ec Gerrit-Change-Number: 82496 Gerrit-PatchSet: 8 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in flashrom[main]: ch347_spi: Add driver support for CH347F packaging
by Anastasia Klimchuk (Code Review) 31 May '24

31 May '24

Attention is currently required from: Paul Menzel, ZhiYuanNJ. Anastasia Klimchuk has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email ) Change subject: ch347_spi: Add driver support for CH347F packaging ...................................................................... Patch Set 13: (1 comment) Patchset: PS13: > My SUBMIT button is in an unreachable state, prompting:You don't have permission to submit change 82 […] Yes, this is normal, there is a limited group of core maintainers who can submit patches. You can read more on our guidelines: https://flashrom.org/dev_guide/development_guide.html#merging-patches In this case, you are all good, your patch is ready. It will be submitted in few days. Thank you for your work! -- To view, visit https://review.coreboot.org/c/flashrom/+/82193?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Gerrit-Change-Number: 82193 Gerrit-PatchSet: 13 Gerrit-Owner: ZhiYuanNJ Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: ZhiYuanNJ Gerrit-Comment-Date: Fri, 31 May 2024 02:44:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: ZhiYuanNJ

1 0

[M] Change in flashrom[main]: ch347_spi: Add driver support for CH347F packaging
by ZhiYuanNJ (Code Review) 31 May '24

31 May '24

Attention is currently required from: Paul Menzel. ZhiYuanNJ has posted comments on this change by ZhiYuanNJ. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email ) Change subject: ch347_spi: Add driver support for CH347F packaging ...................................................................... Patch Set 13: (1 comment) Patchset: PS13: My SUBMIT button is in an unreachable state, prompting:You don't have permission to submit change 82193 Is this a normal phenomenon? -- To view, visit https://review.coreboot.org/c/flashrom/+/82193?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Gerrit-Change-Number: 82193 Gerrit-PatchSet: 13 Gerrit-Owner: ZhiYuanNJ Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Fri, 31 May 2024 02:29:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in flashrom[main]: Add clarification to struct flash_region on chipoff_t
by Peter Marheine (Code Review) 31 May '24

31 May '24

Peter Marheine has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82677?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: Add clarification to struct flash_region on chipoff_t ...................................................................... Add clarification to struct flash_region on chipoff_t Although chipoff_t is fairly clearly documented on its own, it seems fairly frequent that developers will treat the end address of a flash region as an exclusive upper bound rather than the inclusive one it should be; for example CB:82496 fixes an incorrect use that affected multiple sites, and CB:73571 stemmed from a similar cause. Add a clarifying comment to call attention to this, to help programmers avoid making similar mistakes in the future. Change-Id: I80b61a87ca31bd5a116224aadb4e211ee6841e1f Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82677 Reviewed-by: Hsuan-ting Chen <roccochen(a)google.com> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M include/layout.h 1 file changed, 5 insertions(+), 0 deletions(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified Hsuan-ting Chen: Looks good to me, but someone else must approve diff --git a/include/layout.h b/include/layout.h index 5614bea..d03a15b 100644 --- a/include/layout.h +++ b/include/layout.h @@ -37,6 +37,11 @@ struct flash_region { char *name; + /* + * Note that because start and end are chipoff_t, end is an inclusive upper + * bound: the length of a region is (end - start + 1) bytes and it is + * impossible to represent a region with zero length. + */ chipoff_t start; chipoff_t end; bool read_prot; -- To view, visit https://review.coreboot.org/c/flashrom/+/82677?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I80b61a87ca31bd5a116224aadb4e211ee6841e1f Gerrit-Change-Number: 82677 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[M] Change in flashrom[main]: tests/erase: record the opcode for each erase
by Peter Marheine (Code Review) 31 May '24

31 May '24

Peter Marheine has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82251?usp=email ) Change subject: tests/erase: record the opcode for each erase ...................................................................... tests/erase: record the opcode for each erase This allows tests to verify that the correct opcode is used when erasing, which is required to unit-test the fix to issue #525 where in some situations an incorrect erase opcode will be used. BUG=https://ticket.coreboot.org/issues/525 Change-Id: I3983fe42c2e7f06668a1bd20d2db7fafa93b8043 Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82251 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashrom.c M include/flash.h M tests/chip.c M tests/erase_func_algo.c M tests/meson.build 5 files changed, 157 insertions(+), 58 deletions(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified diff --git a/flashrom.c b/flashrom.c index a365303..a056929 100644 --- a/flashrom.c +++ b/flashrom.c @@ -402,8 +402,10 @@ return 0; } -/* special unit-test hook */ -erasefunc_t *g_test_erase_injector; +#ifdef FLASHROM_TEST +/* special unit-test hooks */ +erasefunc_t *g_test_erase_injector[NUM_TEST_ERASE_INJECTORS]; +#endif erasefunc_t *lookup_erase_func_ptr(const struct block_eraser *const eraser) { @@ -442,7 +444,14 @@ case ERASE_SECTOR_49LFXXXC: return &erase_sector_49lfxxxc; case STM50_SECTOR_ERASE: return &erase_sector_stm50; // TODO rename to &stm50_sector_erase; case EDI_CHIP_BLOCK_ERASE: return &edi_chip_block_erase; - case TEST_ERASE_INJECTOR: return g_test_erase_injector; +#ifdef FLASHROM_TEST + case TEST_ERASE_INJECTOR_1: + case TEST_ERASE_INJECTOR_2: + case TEST_ERASE_INJECTOR_3: + case TEST_ERASE_INJECTOR_4: + case TEST_ERASE_INJECTOR_5: + return g_test_erase_injector[eraser->block_erase - TEST_ERASE_INJECTOR_1]; +#endif /* default: total function, 0 indicates no erase function set. * We explicitly do not want a default catch-all case in the switch * to ensure unhandled enum's are compiler warnings. @@ -540,8 +549,10 @@ return ret; } +#ifdef FLASHROM_TEST /* special unit-test hook */ read_func_t *g_test_read_injector; +#endif static read_func_t *lookup_read_func_ptr(const struct flashchip *chip) { @@ -552,7 +563,9 @@ case EDI_CHIP_READ: return &edi_chip_read; case SPI_READ_AT45DB: return spi_read_at45db; case SPI_READ_AT45DB_E8: return spi_read_at45db_e8; +#ifdef FLASHROM_TEST case TEST_READ_INJECTOR: return g_test_read_injector; +#endif /* default: total function, 0 indicates no read function set. * We explicitly do not want a default catch-all case in the switch * to ensure unhandled enum's are compiler warnings. @@ -976,7 +989,9 @@ } /* special unit-test hook */ +#ifdef FLASHROM_TEST write_func_t *g_test_write_injector; +#endif static write_func_t *lookup_write_func_ptr(const struct flashchip *chip) { @@ -992,7 +1007,9 @@ case WRITE_82802AB: return &write_82802ab; case WRITE_EN29LV640B: return &write_en29lv640b; case EDI_CHIP_WRITE: return &edi_chip_write; +#ifdef FLASHROM_TEST case TEST_WRITE_INJECTOR: return g_test_write_injector; +#endif /* default: total function, 0 indicates no write 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/include/flash.h b/include/flash.h index 284461e..aa89409 100644 --- a/include/flash.h +++ b/include/flash.h @@ -266,10 +266,16 @@ WRITE_82802AB, WRITE_EN29LV640B, EDI_CHIP_WRITE, +#ifdef FLASHROM_TEST TEST_WRITE_INJECTOR, /* special case must come last. */ +#endif }; typedef int (write_func_t)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); +#ifdef FLASHROM_TEST +extern write_func_t *g_test_write_injector; +#endif + enum read_func { NO_READ_FUNC = 0, /* 0 indicates no read function set. */ SPI_CHIP_READ = 1, @@ -278,11 +284,17 @@ EDI_CHIP_READ, SPI_READ_AT45DB, SPI_READ_AT45DB_E8, +#ifdef FLASHROM_TEST TEST_READ_INJECTOR, /* special case must come last. */ +#endif }; typedef int (read_func_t)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int read_flash(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); +#ifdef FLASHROM_TEST +extern read_func_t *g_test_read_injector; +#endif + enum block_erase_func { NO_BLOCK_ERASE_FUNC = 0, /* 0 indicates no block erase function set. */ SPI_BLOCK_ERASE_EMULATION = 1, @@ -319,9 +331,21 @@ ERASE_SECTOR_49LFXXXC, STM50_SECTOR_ERASE, EDI_CHIP_BLOCK_ERASE, - TEST_ERASE_INJECTOR, /* special case must come last. */ +#ifdef FLASHROM_TEST + /* special cases must come last. */ + TEST_ERASE_INJECTOR_1, + TEST_ERASE_INJECTOR_2, + TEST_ERASE_INJECTOR_3, + TEST_ERASE_INJECTOR_4, + TEST_ERASE_INJECTOR_5, +#endif }; +#ifdef FLASHROM_TEST +#define NUM_TEST_ERASE_INJECTORS 5 +extern erasefunc_t *g_test_erase_injector[NUM_TEST_ERASE_INJECTORS]; +#endif + enum blockprotect_func { NO_BLOCKPROTECT_FUNC = 0, /* 0 indicates no unlock function set. */ SPI_DISABLE_BLOCKPROTECT, diff --git a/tests/chip.c b/tests/chip.c index 96be7b1..3e1ae3b 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -118,10 +118,6 @@ io_mock_register(NULL); } -extern write_func_t *g_test_write_injector; -extern read_func_t *g_test_read_injector; -extern erasefunc_t *g_test_erase_injector; - static const struct flashchip chip_8MiB = { .vendor = "aklm", .total_size = MOCK_CHIP_SIZE / KiB, @@ -132,7 +128,7 @@ {{ /* All blocks within total size of the chip. */ .eraseblocks = { {2 * MiB, 4} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_1, }}, }; @@ -179,7 +175,7 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + g_test_erase_injector[0] = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -238,7 +234,7 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + g_test_erase_injector[0] = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -313,7 +309,7 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + g_test_erase_injector[0] = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; @@ -504,7 +500,7 @@ g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + g_test_erase_injector[0] = block_erase_chip; struct flashrom_flashctx flashctx = { 0 }; struct flashrom_layout *layout; struct flashchip mock_chip = chip_8MiB; diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index ed9eb04..b0969c2 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -39,6 +39,7 @@ struct erase_invoke { unsigned int blockaddr; unsigned int blocklen; + enum block_erase_func erase_func; }; struct test_case { @@ -82,16 +83,17 @@ return 0; } -static int block_erase_chip(struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen) +static int block_erase_chip_tagged(struct flashctx *flash, enum block_erase_func erase_func, unsigned int blockaddr, unsigned int blocklen) { if (blockaddr + blocklen <= MOCK_CHIP_SIZE) { LOG_ERASE_FUNC; /* Register eraseblock invocation. */ - g_state.eraseblocks_actual[g_state.eraseblocks_actual_ind].blocklen - = blocklen; - g_state.eraseblocks_actual[g_state.eraseblocks_actual_ind].blockaddr - = blockaddr; + g_state.eraseblocks_actual[g_state.eraseblocks_actual_ind] = (struct erase_invoke){ + .blocklen = blocklen, + .blockaddr = blockaddr, + .erase_func = erase_func, + }; g_state.eraseblocks_actual_ind++; } @@ -101,10 +103,6 @@ return 0; } -extern write_func_t *g_test_write_injector; -extern read_func_t *g_test_read_injector; -extern erasefunc_t *g_test_erase_injector; - static struct flashchip chip_1_2_4_8_16 = { .vendor = "aklm", /* @@ -127,19 +125,19 @@ { { .eraseblocks = { {1, MIN_REAL_CHIP_SIZE} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_1, }, { .eraseblocks = { {2, MIN_REAL_CHIP_SIZE / 2} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_2, }, { .eraseblocks = { {4, MIN_REAL_CHIP_SIZE / 4} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_3, }, { .eraseblocks = { {8, MIN_REAL_CHIP_SIZE / 8} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_4, }, { .eraseblocks = { {16, MIN_REAL_CHIP_SIZE / 16} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_5, } }, }; @@ -156,13 +154,13 @@ { { .eraseblocks = { {1, MIN_REAL_CHIP_SIZE} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_1, }, { .eraseblocks = { {8, MIN_REAL_CHIP_SIZE / 8} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_4, }, { .eraseblocks = { {16, MIN_REAL_CHIP_SIZE / 16} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_5, } }, }; @@ -179,20 +177,41 @@ { { .eraseblocks = { {8, MIN_REAL_CHIP_SIZE / 8} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_4, }, { .eraseblocks = { {16, MIN_REAL_CHIP_SIZE / 16} }, - .block_erase = TEST_ERASE_INJECTOR, + .block_erase = TEST_ERASE_INJECTOR_5, } }, }; +#define BLOCK_ERASE_FUNC(n) \ + static int block_erase_chip_ ## n (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen) { \ + return block_erase_chip_tagged(flash, TEST_ERASE_INJECTOR_ ## n, blockaddr, blocklen); \ + } +BLOCK_ERASE_FUNC(1) +BLOCK_ERASE_FUNC(2) +BLOCK_ERASE_FUNC(3) +BLOCK_ERASE_FUNC(4) +BLOCK_ERASE_FUNC(5) + static void setup_chip(struct flashrom_flashctx *flashctx, struct flashrom_layout **layout, const char *programmer_param, struct test_case *current_test_case) { g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + /* Each erasefunc corresponds to an operation that erases a block of + * the chip with a particular size in bytes. */ + memcpy(g_test_erase_injector, + (erasefunc_t *const[]){ + block_erase_chip_1, // 1 byte + block_erase_chip_2, // 2 bytes + block_erase_chip_3, // 4 bytes + block_erase_chip_4, // 8 bytes + block_erase_chip_5, // 16 bytes + }, + sizeof(g_test_erase_injector) + ); /* First MOCK_CHIP_SIZE bytes have a meaning and set with given values for this test case. */ memcpy(g_state.buf, current_test_case->initial_buf, MOCK_CHIP_SIZE); @@ -271,7 +290,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f}, - .eraseblocks_expected = {{0x0, 0x10}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #0", .write_test_name = "Write test case #0", @@ -293,7 +312,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, 0xf8, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f}, - .eraseblocks_expected = {{0x8, 0x8}, {0x0, 0x8}}, + .eraseblocks_expected = {{0x8, 0x8, TEST_ERASE_INJECTOR_4}, {0x0, 0x8, TEST_ERASE_INJECTOR_4}}, .eraseblocks_expected_ind = 2, .erase_test_name = "Erase test case #1", .write_test_name = "Write test case #1", @@ -315,7 +334,13 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0xff, 0xff, 0x0, 0xff, 0x0, 0xff, 0x20, 0x2f, 0x20, 0x2f, 0x0, 0xff, 0xff, 0xff, 0x2f, 0x2f}, - .eraseblocks_expected = {{0xb, 0x1}, {0xc, 0x4}, {0xa, 0x1}, {0x8, 0x2}, {0x0, 0x8}}, + .eraseblocks_expected = { + {0xb, 0x1, TEST_ERASE_INJECTOR_1}, + {0xc, 0x4, TEST_ERASE_INJECTOR_3}, + {0xa, 0x1, TEST_ERASE_INJECTOR_1}, + {0x8, 0x2, TEST_ERASE_INJECTOR_2}, + {0x0, 0x8, TEST_ERASE_INJECTOR_4} + }, .eraseblocks_expected_ind = 5, .erase_test_name = "Erase test case #2", .write_test_name = "Write test case #2", @@ -337,7 +362,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0xff, 0xff, 0xff, 0xff, 0x1, 0x2, 0x3, 0x4, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, - .eraseblocks_expected = {{0x0, 0x10}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #3", .write_test_name = "Write test case #3", @@ -359,7 +384,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x11, 0x22, 0x33, 0x44, 0xff, 0xff, 0xff, 0xff, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, - .eraseblocks_expected = {{0x0, 0x10}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #4", .write_test_name = "Write test case #4", @@ -381,7 +406,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0xff, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, - .eraseblocks_expected = {{0x0, 0x10}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #5", .write_test_name = "Write test case #5", @@ -403,7 +428,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0xdd, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, - .eraseblocks_expected = {{0x0, 0x10}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #6", .write_test_name = "Write test case #6", @@ -425,8 +450,16 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f}, - .eraseblocks_expected = {{0xf, 0x1}, {0xe, 0x1}, {0xc, 0x2}, {0x8, 0x4}, - {0x3, 0x1}, {0x4, 0x4}, {0x2, 0x1}, {0x0, 0x2}}, + .eraseblocks_expected = { + {0xf, 0x1, TEST_ERASE_INJECTOR_1}, + {0xe, 0x1, TEST_ERASE_INJECTOR_1}, + {0xc, 0x2, TEST_ERASE_INJECTOR_2}, + {0x8, 0x4, TEST_ERASE_INJECTOR_3}, + {0x3, 0x1, TEST_ERASE_INJECTOR_1}, + {0x4, 0x4, TEST_ERASE_INJECTOR_3}, + {0x2, 0x1, TEST_ERASE_INJECTOR_1}, + {0x0, 0x2, TEST_ERASE_INJECTOR_2} + }, .eraseblocks_expected_ind = 8, .erase_test_name = "Erase test case #7", .write_test_name = "Write test case #7", @@ -448,7 +481,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f}, - .eraseblocks_expected = {{0x0, 0x10}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #8", .write_test_name = "Write test case #8", @@ -471,7 +504,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, 0xf8, 0x29, 0x2a, 0x2b, 0x2c, 0x2d, 0x2e, 0x2f}, - .eraseblocks_expected = {{0x8, 0x8}, {0x0, 0x8}}, + .eraseblocks_expected = {{0x8, 0x8, TEST_ERASE_INJECTOR_4}, {0x0, 0x8, TEST_ERASE_INJECTOR_4}}, .eraseblocks_expected_ind = 2, .erase_test_name = "Erase test case #9", .write_test_name = "Write test case #9", @@ -493,8 +526,17 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0xff, 0xff, 0x0, 0xff, 0x0, 0xff, 0x20, 0x2f, 0x20, 0x2f, 0x0, 0xff, 0xff, 0xff, 0x2f, 0x2f}, - .eraseblocks_expected = {{0xb, 0x1}, {0xc, 0x1}, {0xd, 0x1}, {0xe, 0x1}, - {0xf, 0x1}, {0x8, 0x1}, {0x9, 0x1}, {0xa, 0x1}, {0x0, 0x8}}, + .eraseblocks_expected = { + {0xb, 0x1, TEST_ERASE_INJECTOR_1}, + {0xc, 0x1, TEST_ERASE_INJECTOR_1}, + {0xd, 0x1, TEST_ERASE_INJECTOR_1}, + {0xe, 0x1, TEST_ERASE_INJECTOR_1}, + {0xf, 0x1, TEST_ERASE_INJECTOR_1}, + {0x8, 0x1, TEST_ERASE_INJECTOR_1}, + {0x9, 0x1, TEST_ERASE_INJECTOR_1}, + {0xa, 0x1, TEST_ERASE_INJECTOR_1}, + {0x0, 0x8, TEST_ERASE_INJECTOR_4} + }, .eraseblocks_expected_ind = 9, .erase_test_name = "Erase test case #10", .write_test_name = "Write test case #10", @@ -516,7 +558,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0xff, 0xff, 0xff, 0xff, 0x1, 0x2, 0x3, 0x4, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff}, - .eraseblocks_expected = {{0x0, 0x10}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #11", .write_test_name = "Write test case #11", @@ -538,7 +580,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x11, 0x22, 0x33, 0x44, 0xff, 0xff, 0xff, 0xff, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, - .eraseblocks_expected = {{0x0, 0x10}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #12", .write_test_name = "Write test case #12", @@ -560,7 +602,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0xff, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, - .eraseblocks_expected = {{0x0, 0x10}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #13", .write_test_name = "Write test case #13", @@ -582,7 +624,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0xdd, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, 0xa8}, - .eraseblocks_expected = {{0x0, 0x10}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #14", .write_test_name = "Write test case #14", @@ -605,10 +647,24 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x1a, 0x1b, 0x1c, 0x1d, 0x1e, 0x1f}, - .eraseblocks_expected = {{0xf, 0x1}, {0x8, 0x1}, {0x9, 0x1}, {0xa, 0x1}, - {0xb, 0x1}, {0xc, 0x1}, {0xd, 0x1}, {0xe, 0x1}, - {0x3, 0x1}, {0x4, 0x1}, {0x5, 0x1}, {0x6, 0x1}, - {0x7, 0x1}, {0x0, 0x1}, {0x1, 0x1}, {0x2, 0x1}}, + .eraseblocks_expected = { + {0xf, 0x1, TEST_ERASE_INJECTOR_1}, + {0x8, 0x1, TEST_ERASE_INJECTOR_1}, + {0x9, 0x1, TEST_ERASE_INJECTOR_1}, + {0xa, 0x1, TEST_ERASE_INJECTOR_1}, + {0xb, 0x1, TEST_ERASE_INJECTOR_1}, + {0xc, 0x1, TEST_ERASE_INJECTOR_1}, + {0xd, 0x1, TEST_ERASE_INJECTOR_1}, + {0xe, 0x1, TEST_ERASE_INJECTOR_1}, + {0x3, 0x1, TEST_ERASE_INJECTOR_1}, + {0x4, 0x1, TEST_ERASE_INJECTOR_1}, + {0x5, 0x1, TEST_ERASE_INJECTOR_1}, + {0x6, 0x1, TEST_ERASE_INJECTOR_1}, + {0x7, 0x1, TEST_ERASE_INJECTOR_1}, + {0x0, 0x1, TEST_ERASE_INJECTOR_1}, + {0x1, 0x1, TEST_ERASE_INJECTOR_1}, + {0x2, 0x1, TEST_ERASE_INJECTOR_1}, + }, .eraseblocks_expected_ind = 16, .erase_test_name = "Erase test case #15", .write_test_name = "Write test case #15", @@ -631,7 +687,12 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x14, 0x14, 0x15, 0x15, 0x15, 0x15, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x17}, - .eraseblocks_expected = {{0x8, 0x8}, {0x0, 0x10}, {0x0, 0x8}, {0x0, 0x8}}, + .eraseblocks_expected = { + {0x8, 0x8, TEST_ERASE_INJECTOR_4}, + {0x0, 0x10, TEST_ERASE_INJECTOR_5}, + {0x0, 0x8, TEST_ERASE_INJECTOR_4}, + {0x0, 0x8, TEST_ERASE_INJECTOR_4}, + }, .eraseblocks_expected_ind = 4, .erase_test_name = "Erase test case #16", .write_test_name = "Write test case #16", @@ -653,7 +714,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x14, 0x14, 0x14, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16}, - .eraseblocks_expected = {{0x0, 0x10}, {0x0, 0x8}}, + .eraseblocks_expected = {{0x0, 0x10, TEST_ERASE_INJECTOR_5}, {0x0, 0x8, TEST_ERASE_INJECTOR_4}}, .eraseblocks_expected_ind = 2, .erase_test_name = "Erase test case #17", .write_test_name = "Write test case #17", @@ -675,7 +736,7 @@ ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE}, .written_buf = {0x14, 0x14, 0x14, 0x14, 0x14, 0x14, 0x14, 0x14, 0x14, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16, 0x16}, - .eraseblocks_expected = {{0x8, 0x8}, {0x0, 0x10}}, + .eraseblocks_expected = {{0x8, 0x8, TEST_ERASE_INJECTOR_4}, {0x0, 0x10, TEST_ERASE_INJECTOR_5}}, .eraseblocks_expected_ind = 2, .erase_test_name = "Erase test case #18", .write_test_name = "Write test case #18", @@ -696,7 +757,7 @@ 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, .written_buf = {0x14, 0x14, 0x14, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, - .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected = {{0x0, 0x8, TEST_ERASE_INJECTOR_4}}, .eraseblocks_expected_ind = 1, .erase_test_name = "Erase test case #19", .write_test_name = "Write test case #19", diff --git a/tests/meson.build b/tests/meson.build index e8de4b7..e788b80 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -124,6 +124,7 @@ '-ffunction-sections', '-fdata-sections', '-U_FORTIFY_SOURCE', + '-DFLASHROM_TEST', ], export_dynamic : true, link_args : mocks + link_args, -- To view, visit https://review.coreboot.org/c/flashrom/+/82251?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I3983fe42c2e7f06668a1bd20d2db7fafa93b8043 Gerrit-Change-Number: 82251 Gerrit-PatchSet: 9 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2024