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

[S] Change in flashrom[main]: doc: Fix index files in Supported HW section
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Anastasia Klimchuk, Sydney, Thomas Heijligen. Hello Peter Marheine, Sydney, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/82196?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by Sydney, Verified+1 by build bot (Jenkins) Change subject: doc: Fix index files in Supported HW section ...................................................................... doc: Fix index files in Supported HW section By default toctree in the index file displays full tree of docs with all the nested levels, and it's too much detail. Besides, left side menu displays the tree anyway, so duplication is not needed. Supported hardware section has the deepest nesting out of all other docs. This patch changes high-level index files to only display flat list of next level subtree. On deeper level, full index is displayed. Change-Id: Ia15e9766cce6f19be1e69fbb1236a327ae3d57b3 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/supported_hw/index.rst M doc/supported_hw/supported_prog/index.rst 2 files changed, 11 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/82196/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82196?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia15e9766cce6f19be1e69fbb1236a327ae3d57b3 Gerrit-Change-Number: 82196 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Sydney <git(a)funkeleinhorn.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sydney <git(a)funkeleinhorn.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[main]: doc: Add doc for supported chipsets
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Thomas Heijligen. Hello Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/82198?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: doc: Add doc for supported chipsets ...................................................................... doc: Add doc for supported chipsets Change-Id: I9c9edc7deeeb7a783e2ba2fc6b372edb9c61609e Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/supported_hw/index.rst A doc/supported_hw/supported_chipsets.rst 2 files changed, 22 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/98/82198/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82198?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9c9edc7deeeb7a783e2ba2fc6b372edb9c61609e Gerrit-Change-Number: 82198 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[main]: doc: Add doc for supported flash chips
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Alexander Goncharov, Anastasia Klimchuk, Thomas Heijligen, Victor Lim. Hello Alexander Goncharov, Peter Marheine, Thomas Heijligen, Victor Lim, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/82197?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: doc: Add doc for supported flash chips ...................................................................... doc: Add doc for supported flash chips Change-Id: I05fb60a4caf2cfb30586fa482687b10638996395 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/supported_hw/index.rst A doc/supported_hw/supported_flashchips.rst 2 files changed, 26 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/97/82197/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82197?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I05fb60a4caf2cfb30586fa482687b10638996395 Gerrit-Change-Number: 82197 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Victor Lim <victorswlim(a)yahoo.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[main]: ichspi.c: Add support for region 9 and beyond in Meteor Lake
by Hsuan-ting Chen (Code Review) 10 May '24

10 May '24

Attention is currently required from: Anastasia Klimchuk, Hsuan Ting Chen, Subrata Banik. Hsuan-ting Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81357?usp=email ) Change subject: ichspi.c: Add support for region 9 and beyond in Meteor Lake ...................................................................... Patch Set 12: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/81357/comment/c6175aab_81cead12 : PS12, Line 26: TEST=On MTL, use flashrom -VV to see correct FREG9 access : TEST=On ADL, use flashrom -VV to see not break anything : TEST=On APL, use flashrom -VV to see not break anything > Would you mind running the test scenarious again on the latest version of code? Thank you! Yes, I checked it on MTL and APL again when I submitted the code. File ichspi.c: https://review.coreboot.org/c/flashrom/+/81357/comment/bb41b5a9_7be97da4 : PS10, Line 1853: mmio_readw > Thanks as usual for giving all details! appreciate it. […] ICH_BRRA and ICH_BRWA are designed to be at most 8 bits, also we have a mask: ``` #define ICH_BRWA(x) ((x >> 8) & 0xff) #define ICH_BRRA(x) ((x >> 0) & 0xff) ``` So they should always fit in 8 bits. (uint8_t) https://review.coreboot.org/c/flashrom/+/81357/comment/10ad523a_d8ae2cdc : PS10, Line 1876: inline > I am thinking from the other side: are there any reasons we should explicitly say it needs to be inl […] Yes, and I believe that this short static would possibly inline it by default. -- To view, visit https://review.coreboot.org/c/flashrom/+/81357?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Gerrit-Change-Number: 81357 Gerrit-PatchSet: 12 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-CC: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 10 May 2024 07:42:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: [WIP] tests: Add tests for erasing chip with protected region
by Peter Marheine (Code Review) 10 May '24

10 May '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81664?usp=email ) Change subject: [WIP] tests: Add tests for erasing chip with protected region ...................................................................... Patch Set 1: (1 comment) File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/81664/comment/869bfab8_44cff11b : PS1, Line 130: TEST_ERASE_INJECTOR > I've prototyped this in CB:82251 but can't tell which (if any) of the new test cases should be faili […] Correction: "without CB:81385". -- To view, visit https://review.coreboot.org/c/flashrom/+/81664?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e7cfd027d37437c917c054f30f3be59b76b0069 Gerrit-Change-Number: 81664 Gerrit-PatchSet: 1 Gerrit-Owner: 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-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 10 May 2024 07:12:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: [WIP] tests: Add tests for erasing chip with protected region
by Peter Marheine (Code Review) 10 May '24

10 May '24

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81664?usp=email ) Change subject: [WIP] tests: Add tests for erasing chip with protected region ...................................................................... Patch Set 1: (1 comment) File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/81664/comment/bc040c5a_db9123c7 : PS1, Line 130: TEST_ERASE_INJECTOR > One thing I haven't figured yet how to do in this test: […] I've prototyped this in CB:82251 but can't tell which (if any) of the new test cases should be failing without CB:82251. I want to create tests that show the current failure, then rebase the fix onto the tests and get them all to pass. -- To view, visit https://review.coreboot.org/c/flashrom/+/81664?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e7cfd027d37437c917c054f30f3be59b76b0069 Gerrit-Change-Number: 81664 Gerrit-PatchSet: 1 Gerrit-Owner: 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-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 10 May 2024 07:11:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

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

10 May '24

Peter Marheine has uploaded this change for review. ( 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 the test to verify that the correct opcode is used when there are protected regions, causing it to fail because that is currently buggy (and intended to be fixed by CB:81385). Change-Id: I3983fe42c2e7f06668a1bd20d2db7fafa93b8043 Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> --- M flashrom.c M include/flash.h M tests/chip.c M tests/erase_func_algo.c 4 files changed, 87 insertions(+), 37 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/82251/1 diff --git a/flashrom.c b/flashrom.c index 630c69d..46bf12a 100644 --- a/flashrom.c +++ b/flashrom.c @@ -402,8 +402,8 @@ return 0; } -/* special unit-test hook */ -erasefunc_t *g_test_erase_injector; +/* special unit-test hooks */ +erasefunc_t *g_test_erase_injector[5] = {NULL}; erasefunc_t *lookup_erase_func_ptr(const struct block_eraser *const eraser) { @@ -442,7 +442,12 @@ 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; + 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]; /* 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. diff --git a/include/flash.h b/include/flash.h index 284461e..fe7e018 100644 --- a/include/flash.h +++ b/include/flash.h @@ -319,7 +319,13 @@ ERASE_SECTOR_49LFXXXC, STM50_SECTOR_ERASE, EDI_CHIP_BLOCK_ERASE, - TEST_ERASE_INJECTOR, /* special case must come last. */ + /* special cases must come last. */ + TEST_ERASE_INJECTOR_1, + TEST_ERASE_INJECTOR = TEST_ERASE_INJECTOR_1, + TEST_ERASE_INJECTOR_2, + TEST_ERASE_INJECTOR_3, + TEST_ERASE_INJECTOR_4, + TEST_ERASE_INJECTOR_5, }; enum blockprotect_func { diff --git a/tests/chip.c b/tests/chip.c index 96be7b1..22a5e1c 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -120,7 +120,7 @@ extern write_func_t *g_test_write_injector; extern read_func_t *g_test_read_injector; -extern erasefunc_t *g_test_erase_injector; +extern erasefunc_t *g_test_erase_injector[5]; static const struct flashchip chip_8MiB = { .vendor = "aklm", @@ -179,7 +179,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 +238,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 +313,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 +504,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 a600de2..c98d873 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 { @@ -88,10 +89,10 @@ 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, + }; g_state.eraseblocks_actual_ind++; } @@ -103,7 +104,7 @@ extern write_func_t *g_test_write_injector; extern read_func_t *g_test_read_injector; -extern erasefunc_t *g_test_erase_injector; +extern erasefunc_t *g_test_erase_injector[5]; static struct flashchip chip_1_2_4_8_16 = { .vendor = "aklm", @@ -127,19 +128,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 +157,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,10 +180,10 @@ { { .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, } }, }; @@ -192,7 +193,12 @@ { g_test_write_injector = write_chip; g_test_read_injector = read_chip; - g_test_erase_injector = block_erase_chip; + memcpy(g_test_erase_injector, + (erasefunc_t *const[]){ + block_erase_chip, block_erase_chip, block_erase_chip, block_erase_chip, block_erase_chip + }, + 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); @@ -800,7 +806,11 @@ .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, - .eraseblocks_expected = {{0x0, 0x8}, {0x0, 0x8}}, + .eraseblocks_expected = { + // TODO: I think this is supposed to fail? + {0x0, 0x8, 5}, + {0x0, 0x8, 4} + }, .eraseblocks_expected_ind = 2, .erase_test_name = "Erase protected region test case #4", }, { @@ -818,9 +828,9 @@ .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, - .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected = {{0x0, 0x8, 5}}, .eraseblocks_expected_ind = 1, - .erase_test_name = "Erase test case #5", + .erase_test_name = "Erase protected region test case #5", }, }; @@ -1016,16 +1026,17 @@ } } -static int block_erase_chip_with_protected_region(struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen) +static int block_erase_chip_with_protected_region_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; + printf("Eraser called with blockaddr=0x%x, blocklen=0x%x, erase_func=%d\n", blockaddr, blocklen, erase_func - TEST_ERASE_INJECTOR_1 + 1); /* 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){ + .erase_func = erase_func, + .blocklen = blocklen, + .blockaddr = blockaddr, + }; g_state.eraseblocks_actual_ind++; } @@ -1037,15 +1048,34 @@ // // Note: returning 1 instead of assert, so that the flow goes back to erasure code // to clean up the memory after failed erase. Memory leaks are also tested by unit tests. + const unsigned int erase_op_size = 1 << (erase_func - TEST_ERASE_INJECTOR_1); + if (blocklen < erase_op_size) { + printf("Error: block length %d is smaller than erase_func length %d\n", blocklen, erase_op_size); + return 1; + } + if ((blockaddr >= START_PROTECTED_REGION && blockaddr < END_PROTECTED_REGION) || (blockaddr + blocklen - 1 >= START_PROTECTED_REGION - && blockaddr + blocklen - 1 < END_PROTECTED_REGION)) - return 1; + && blockaddr + blocklen - 1 < END_PROTECTED_REGION)) { + printf("Error: block with start=%d, len=%d is overlaps protected region %d-%d\n", + blockaddr, blocklen, START_PROTECTED_REGION, END_PROTECTED_REGION); + return 1; + } memset(&g_state.buf[blockaddr], ERASE_VALUE, blocklen); return 0; } +#define ERASE_PROTECTED(i) static int block_erase_chip_with_protected_region_ ## i \ + (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen) { \ + return block_erase_chip_with_protected_region_tagged(flash, TEST_ERASE_INJECTOR_ ## i, blockaddr, blocklen); \ + } +ERASE_PROTECTED(1) +ERASE_PROTECTED(2) +ERASE_PROTECTED(3) +ERASE_PROTECTED(4) +ERASE_PROTECTED(5) + void erase_unwritable_regions_skipflag_on_test_success(void **state) { struct test_case* current_test_case = *state; @@ -1059,7 +1089,16 @@ setup_chip(&flashctx, &layout, param, current_test_case); // This test needs special block erase to emulate protected regions. - g_test_erase_injector = block_erase_chip_with_protected_region; + memcpy(g_test_erase_injector, + (erasefunc_t *const[]){ + block_erase_chip_with_protected_region_1, + block_erase_chip_with_protected_region_2, + block_erase_chip_with_protected_region_3, + block_erase_chip_with_protected_region_4, + block_erase_chip_with_protected_region_5, + }, + sizeof(g_test_erase_injector) + ); flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS, true); -- 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 Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I3983fe42c2e7f06668a1bd20d2db7fafa93b8043 Gerrit-Change-Number: 82251 Gerrit-PatchSet: 1 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[main]: ichspi.c: Add support for region 9 and beyond in Meteor Lake
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Subrata Banik. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81357?usp=email ) Change subject: ichspi.c: Add support for region 9 and beyond in Meteor Lake ...................................................................... Patch Set 12: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/81357/comment/e7205458_afc84bff : PS12, Line 26: TEST=On MTL, use flashrom -VV to see correct FREG9 access : TEST=On ADL, use flashrom -VV to see not break anything : TEST=On APL, use flashrom -VV to see not break anything Would you mind running the test scenarious again on the latest version of code? Thank you! Patchset: PS12: Subrata, just wanted to check, do you have interest to review the latest version of the patch? I know you reviewed, but maybe you want to have the last look with all the changes made. File ichspi.c: https://review.coreboot.org/c/flashrom/+/81357/comment/af31708c_ee193798 : PS10, Line 1853: mmio_readw > You could see the document of Intel Core Ultra: […] Thanks as usual for giving all details! appreciate it. I was staring on the code for some time, but it looks like it should work. I was mostly looking at conversion to smaller size *region_read_access = (uint16_t)ICH_BRRA(tmp); *region_write_access = (uint16_t)ICH_BRWA(tmp); while previously `ICH_BRRA(tmp)` and `ICH_BRWA(tmp)` were used as is, without conversion to smaller size. But, as you pointed in docs, only 2 bytes are needed... https://review.coreboot.org/c/flashrom/+/81357/comment/998aaaf2_094d074e : PS10, Line 1876: inline > Done […] I am thinking from the other side: are there any reasons we should explicitly say it needs to be inline, and if there are no reasons, better leave this to compiler. With just one callsite, I think the function will be inlined anyway. -- To view, visit https://review.coreboot.org/c/flashrom/+/81357?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1e06e7b3d470423a6014e623826d9234fdebfbf9 Gerrit-Change-Number: 81357 Gerrit-PatchSet: 12 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-CC: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Comment-Date: Fri, 10 May 2024 06:04:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: doc: Add user doc with links to ChromeOS documents
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82181?usp=email ) Change subject: doc: Add user doc with links to ChromeOS documents ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > Sounds good! So this is now submitted, you add for yourself an entry in MAINTAINERS file for this file `doc/user_docs/chromebooks.rst` . Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/82181?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: If7b06c077b34f73bc6c33f617332dfc32b982c12 Gerrit-Change-Number: 82181 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Fri, 10 May 2024 03:35:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashrom_udev.rules: Add rule for CH347
by Anastasia Klimchuk (Code Review) 10 May '24

10 May '24

Attention is currently required from: Nicholas Chin. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82162?usp=email ) Change subject: flashrom_udev.rules: Add rule for CH347 ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82162?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ia83fa08f6d7c2f449b1a5c0c387c6d4368b99e3a Gerrit-Change-Number: 82162 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Fri, 10 May 2024 03:30:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit May 2024