January 2024 - flashrom-gerrit

[XS] Change in flashrom[main]: Makefile: Fix version string for non-Git builds
by Anastasia Klimchuk (Code Review) Jan. 15, 2024

Jan. 15, 2024

Attention is currently required from: Anton Samsonov, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79152?usp=email ) Change subject: Makefile: Fix version string for non-Git builds ...................................................................... Patch Set 5: Code-Review+1 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/79152/comment/db59f4a7_f2c0488a : PS5, Line 7: non-Git builds What is an example of non-Git build? I am probably missing something, how do you get the repo without Git? (The answer would probably be a good addition to commit description) -- To view, visit https://review.coreboot.org/c/flashrom/+/79152?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: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed Gerrit-Change-Number: 79152 Gerrit-PatchSet: 5 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: 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-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Mon, 15 Jan 2024 08:55:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: Makefile,meson.build: Add support for Sphinx 3.x
by Anastasia Klimchuk (Code Review) Jan. 15, 2024

Jan. 15, 2024

Attention is currently required from: Anton Samsonov, Thomas Heijligen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77778?usp=email ) Change subject: Makefile,meson.build: Add support for Sphinx 3.x ...................................................................... Patch Set 3: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/77778/comment/6a9d3d8c_aadecc76 : PS3, Line 7: 3 3 or 4? From the patch it seems like 4.x is a big change, not 3? https://review.coreboot.org/c/flashrom/+/77778/comment/eebda22c_e977d6ff : PS3, Line 10: 7996 and 9217 This is very useful info, but you would need to add full links to GitHub issues (so that it's easier for reviewers to follow), thank you! https://review.coreboot.org/c/flashrom/+/77778/comment/9404d46a_8056d70c : PS3, Line 13: but not on `sphinx-build` own level : upon which `meson` build relies I don't fully understand this last part of the sentence, what does it mean "not on sphinx-build own level"? Patchset: PS3: Anton sorry for such a long delay, I want to try get to this patch soon. I added few small questions, but I will do a deep dive later. Thank you for the patch! -- To view, visit https://review.coreboot.org/c/flashrom/+/77778?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: I9cd280551a1ba4d17edb2e857d56f80431b61e1b Gerrit-Change-Number: 77778 Gerrit-PatchSet: 3 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: 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-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Mon, 15 Jan 2024 04:56:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashrom_tester: Fix partial_lock_test on libflashrom
by Anastasia Klimchuk (Code Review) Jan. 15, 2024

Jan. 15, 2024

Attention is currently required from: David Wu, Edward O'Callaghan, Evan Benn, Hsuan Ting Chen, Hsuan-ting Chen. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79304?usp=email ) Change subject: flashrom_tester: Fix partial_lock_test on libflashrom ...................................................................... Patch Set 3: (4 comments) Patchset: PS3: Ed, Evan, thank you so much for reviews! Could you please vote on the patch? PS3: > Hi Anastasia, […] Sure! I looked at this, but we need to try get votes on the patch from Ed and/or Evan, they have knowledge of the stuff. File util/flashrom_tester/Cargo.toml: https://review.coreboot.org/c/flashrom/+/79304/comment/a0f95b87_0d53b186 : PS3, Line 25: libflashrom = { path = "../../bindings/rust/libflashrom" } I am curious: why it was working before, without this line? File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/79304/comment/fe975616_e2c1e92d : PS3, Line 301: // TODO Did you mean to do something here? -- To view, visit https://review.coreboot.org/c/flashrom/+/79304?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: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2 Gerrit-Change-Number: 79304 Gerrit-PatchSet: 3 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Attention: Evan Benn <evanbenn(a)gmail.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 15 Jan 2024 04:18:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: Add Idaville platform into chipset enable list and add update IRC reg...
by Anastasia Klimchuk (Code Review) Jan. 15, 2024

Jan. 15, 2024

Attention is currently required from: Angel Pons, David Hendricks, Kevin Yu. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79340?usp=email ) Change subject: Add Idaville platform into chipset enable list and add update IRC region support ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: David, may I ask your help to review this patch? That would be huge help, thank you so much! -- To view, visit https://review.coreboot.org/c/flashrom/+/79340?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: Icc526b22a9efc8071e972bb1d8cfc51d6a83651b Gerrit-Change-Number: 79340 Gerrit-PatchSet: 6 Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Kevin Yu <ykevin(a)celestica.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 03:23:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: Add Idaville platform into chipset enable list and add update IRC reg...
by Anastasia Klimchuk (Code Review) Jan. 15, 2024

Jan. 15, 2024

Attention is currently required from: Angel Pons, Kevin Yu. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79340?usp=email ) Change subject: Add Idaville platform into chipset enable list and add update IRC region support ...................................................................... Patch Set 6: (4 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/79340/comment/cbf2f644_ad8ed0ae : PS6, Line 7: Add Idaville platform into chipset enable list and add update IRC region support Commit title and message are limited by 72 chars width, so you need to shorten your commit title. For example "Add support for Idaville HCC platform and iRC regions" And then the rest of commit description can be provide full details of the commit. https://review.coreboot.org/c/flashrom/+/79340/comment/2c8bc947_ef80464e : PS6, Line 11: You need to add full testing info into commit message (which platforms you tested on, which commands etc). Patchset: PS6: Hello Kevin! Sorry there was some delay in reply! It's because by coincidence your patch fell into the holidays season here, and most people would be away (between mid-December and mid-January that is). I am looking at it now, and I have few bigger questions, before diving into details: 1) This seems platform-specific feature (Intel), so flashrom.c is probably not the right place for the code, I think the code should live in ichspi.c 2) If you could give me more details on what is IRC region, any docs you can share? I want to understand what that is. There are few other comments that I have, you can have a look. Also I think we need few more reviewers on the patch, but that's fine. Thank you for patch! And let's start working on it :) File flashrom.c: https://review.coreboot.org/c/flashrom/+/79340/comment/c6310cbe_b2b19c43 : PS6, Line 527: }else{ A generic comment to all of your code (applies to all the files): code should follow the coding style which is described in our dev guidelines: https://www.flashrom.org/dev_guide/development_guide.html#coding-style (please don't miss to click the link and open Linux kernel style guide). You need to read the code style first, and then go through your code and make sure all of it follows the style. Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/79340?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: Icc526b22a9efc8071e972bb1d8cfc51d6a83651b Gerrit-Change-Number: 79340 Gerrit-PatchSet: 6 Gerrit-Owner: Kevin Yu <ykevin(a)celestica.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Kevin Yu <ykevin(a)celestica.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 03:16:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashchips: Mark FM25Q16 as probe/read tested
by Stefan Reinauer (Code Review) Jan. 15, 2024

Jan. 15, 2024

Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev. Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79975?usp=email ) Change subject: flashchips: Mark FM25Q16 as probe/read tested ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/79975?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: I8e55cd6eeb693bf33e580d662da08e50fb46809d Gerrit-Change-Number: 79975 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Mon, 15 Jan 2024 03:01:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: flashchips: Mark FM25Q16 as probe/read tested
by Anastasia Klimchuk (Code Review) Jan. 15, 2024

Jan. 15, 2024

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/79975?usp=email ) Change subject: flashchips: Mark FM25Q16 as probe/read tested ...................................................................... flashchips: Mark FM25Q16 as probe/read tested As reported on the mailing list: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/I6H7… Change-Id: I8e55cd6eeb693bf33e580d662da08e50fb46809d Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M flashchips.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/79975/1 diff --git a/flashchips.c b/flashchips.c index 663b01b..f580a36 100644 --- a/flashchips.c +++ b/flashchips.c @@ -6198,7 +6198,7 @@ /* OTP: 1024B total; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ /* QPI enable 0x38, disable 0xFF */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI, - .tested = TEST_UNTESTED, + .tested = TEST_OK_PR, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = { -- To view, visit https://review.coreboot.org/c/flashrom/+/79975?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: I8e55cd6eeb693bf33e580d662da08e50fb46809d Gerrit-Change-Number: 79975 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[main]: tests: Add test cases for erasing/writing unaligned layout regions
by Anastasia Klimchuk (Code Review) Jan. 15, 2024

Jan. 15, 2024

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/78985?usp=email ) ( 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tests: Add test cases for erasing/writing unaligned layout regions ...................................................................... tests: Add test cases for erasing/writing unaligned layout regions The test cases from #16 onwards cover the case when layout region is not aligned with eraseblock region, and therefore either layout start boundary or end boundary needs to be extended to align with eraseblock. Ticket: https://ticket.coreboot.org/issues/494 Change-Id: I726a30b0e47a966e8093ddc19abf4a65fb1d70ce Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/78985 Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M tests/erase_func_algo.c 1 file changed, 118 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index 824a03a..ed9eb04 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -27,8 +27,6 @@ #define ERASE_VALUE 0xff #define MOCK_CHIP_SIZE 16 -/* How many small chip test cases. */ -#define TEST_CASES_NUM 16 #define MIN_BUF_SIZE 1024 /* Minimum buffer size flashrom operates for chip operations. */ #define MIN_REAL_CHIP_SIZE 1024 /* Minimum chip size that can be defined for real chip in flashchips */ @@ -169,6 +167,26 @@ }, }; +static struct flashchip chip_8_16 = { + .vendor = "aklm", + /* See comment on previous chip. */ + .total_size = 1, + .tested = TEST_OK_PREW, + .gran = WRITE_GRAN_1BYTE, + .read = TEST_READ_INJECTOR, + .write = TEST_WRITE_INJECTOR, + .block_erasers = + { + { + .eraseblocks = { {8, MIN_REAL_CHIP_SIZE / 8} }, + .block_erase = TEST_ERASE_INJECTOR, + }, { + .eraseblocks = { {16, MIN_REAL_CHIP_SIZE / 16} }, + .block_erase = TEST_ERASE_INJECTOR, + } + }, +}; + static void setup_chip(struct flashrom_flashctx *flashctx, struct flashrom_layout **layout, const char *programmer_param, struct test_case *current_test_case) { @@ -230,8 +248,9 @@ /* * Setup all test cases. * - * First half of test cases (0 - (size(testcases)/2-1)) set up for a chip with erasers: 1, 2, 4, 8, 16 bytes. - * Second half (size(testcases)/2 - size(testcases)) repeates the same test cases for a chip with erasers: 1, 8, 16 bytes. + * First half of test cases is set up for a chip with erasers: 1, 2, 4, 8, 16 bytes. + * Second half repeates the same test cases for a chip with erasers: 1, 8, 16 bytes. + * Tests from #16 onwards use the chip with erasers: 8, 16 bytes, to test unaligned layout regions. */ static struct test_case test_cases[] = { { @@ -593,6 +612,94 @@ .eraseblocks_expected_ind = 16, .erase_test_name = "Erase test case #15", .write_test_name = "Write test case #15", + }, { + /* + * Test case #16 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned regions 2+4+9+1b which are smaller than the smallest eraseblock. + * Chip with eraseblocks 8, 16. + */ + .chip = &chip_8_16, + .regions = {{0, 1, "reg2"}, {2, 5, "reg4"}, {6, 14, "reg9"}, + {15, MIN_REAL_CHIP_SIZE - 1, "reg1"}}, + .initial_buf = {0x4, 0x4, 0x5, 0x5, 0x5, 0x5, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x7}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 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_ind = 4, + .erase_test_name = "Erase test case #16", + .write_test_name = "Write test case #16", + }, { + /* + * Test case #17 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned region 3+13b which are smaller than the smallest eraseblock. + * Chip with eraseblocks 8, 16. + */ + .chip = &chip_8_16, + .regions = {{0, 2, "reg3"}, {3, MIN_REAL_CHIP_SIZE - 1, "tail"}}, + .initial_buf = {0x4, 0x4, 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 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_ind = 2, + .erase_test_name = "Erase test case #17", + .write_test_name = "Write test case #17", + }, { + /* + * Test case #18 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned region 9+7b. + * Chip with eraseblocks 8, 16. + */ + .chip = &chip_8_16, + .regions = {{0, 8, "reg9"}, {9, MIN_REAL_CHIP_SIZE - 1, "tail"}}, + .initial_buf = {0x4, 0x4, 0x4, 0x4, 0x4, 0x4, 0x4, 0x4, + 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 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_ind = 2, + .erase_test_name = "Erase test case #18", + .write_test_name = "Write test case #18", + }, { + /* + * Test case #19 + * + * Initial vs written: 3 bytes of the logical layout are different, rest is the same. + * Layout with unaligned region 3 bytes. Layout does not cover the whole chip memory. + * Chip memory outside of logical layout is skipped by both erase and write ops. + * Chip with eraseblocks 8, 16. + */ + .chip = &chip_8_16, + .regions = {{0, 2, "reg3"}}, + .initial_buf = {0x4, 0x4, 0x4, 0x0, 0x0, 0x0, 0x0, 0x0, + 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, 0x0, + 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_ind = 1, + .erase_test_name = "Erase test case #19", + .write_test_name = "Write test case #19", }, }; @@ -612,11 +719,13 @@ * The caller needs to free the allocated memory. */ struct CMUnitTest *get_erase_func_algo_tests(size_t *num_tests) { - // Twice the number of test cases, because each test case is run twice: for erase and write. - struct CMUnitTest *all_cases = calloc(TEST_CASES_NUM * 2, sizeof(struct CMUnitTest)); - *num_tests = TEST_CASES_NUM * 2; + const size_t test_cases_num = ARRAY_SIZE(test_cases); - for (size_t i = 0; i < TEST_CASES_NUM; i++) { + // Twice the number of test cases, because each test case is run twice: for erase and write. + struct CMUnitTest *all_cases = calloc(test_cases_num * 2, sizeof(struct CMUnitTest)); + *num_tests = test_cases_num * 2; + + for (size_t i = 0; i < test_cases_num; i++) { all_cases[i] = (struct CMUnitTest) { .name = test_cases[i].erase_test_name, .setup_func = setup, @@ -624,7 +733,7 @@ .initial_state = &test_cases[i], .test_func = erase_function_algo_test_success, }; - all_cases[i + TEST_CASES_NUM] = (struct CMUnitTest) { + all_cases[i + test_cases_num] = (struct CMUnitTest) { .name = test_cases[i].write_test_name, .setup_func = setup, .teardown_func = teardown, -- To view, visit https://review.coreboot.org/c/flashrom/+/78985?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: I726a30b0e47a966e8093ddc19abf4a65fb1d70ce Gerrit-Change-Number: 78985 Gerrit-PatchSet: 6 Gerrit-Owner: Anastasia Klimchuk <aklm(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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix unaligned region end offset by 1
by Anastasia Klimchuk (Code Review) Jan. 15, 2024

Jan. 15, 2024

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/78984?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: erasure_layout: Fix unaligned region end offset by 1 ...................................................................... erasure_layout: Fix unaligned region end offset by 1 In the case when layout region is not aligned with eraseblock, end region boundary is extended to match the eraseblock. There is a special handling of this extended area (between original end of region and extended one). Fix the offset of this extended area by +1 so that it covers the extended area and not the original region. Before the patch, the last byte of the original region was failed to write since it was treated as if an extended area, while it was the last byte of the normal layout region. Ticket: https://ticket.coreboot.org/issues/494 Change-Id: I7f78a0090065cd2a952cba1a5d28179483ba4c55 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/78984 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M erasure_layout.c 1 file changed, 4 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved diff --git a/erasure_layout.c b/erasure_layout.c index c60305f..1dd6b60 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -279,9 +279,10 @@ memcpy(newcontents + region_start, curcontents + region_start, old_start - region_start); } if (region_end - old_end) { - read_flash(flashctx, curcontents + old_end, old_end, region_end - old_end); - memcpy(old_end_buf, newcontents + old_end, region_end - old_end); - memcpy(newcontents + old_end, curcontents + old_end, region_end - old_end); + chipoff_t end_offset = old_end + 1; + read_flash(flashctx, curcontents + end_offset, end_offset, region_end - old_end); + memcpy(old_end_buf, newcontents + end_offset, region_end - old_end); + memcpy(newcontents + end_offset, curcontents + end_offset, region_end - old_end); } // select erase functions -- To view, visit https://review.coreboot.org/c/flashrom/+/78984?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: I7f78a0090065cd2a952cba1a5d28179483ba4c55 Gerrit-Change-Number: 78984 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(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: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix unaligned region end offset by 1
by Anastasia Klimchuk (Code Review) Jan. 15, 2024

Jan. 15, 2024

Attention is currently required from: Aarya, Alexander Goncharov, Carly Zlabek, Vincent Fazio. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78984?usp=email ) Change subject: erasure_layout: Fix unaligned region end offset by 1 ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > After some investigation I came to conclusion that no fix is needed for start boundary extension (se […] Alright, resolving this to unblock the patch. -- To view, visit https://review.coreboot.org/c/flashrom/+/78984?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: I7f78a0090065cd2a952cba1a5d28179483ba4c55 Gerrit-Change-Number: 78984 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Vincent Fazio <vfazio(a)gmail.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Mon, 15 Jan 2024 01:58:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0