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

[S] Change in flashrom[main]: meson: Update CI script to enforce building man pages and docs
by Anastasia Klimchuk (Code Review) 04 Apr '24

04 Apr '24

Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen. Hello Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/81665?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: meson: Update CI script to enforce building man pages and docs ...................................................................... meson: Update CI script to enforce building man pages and docs `test_build.sh` is used by Jenkins, therefore it should build everything. Docker container for Jenkins is expected to have all the dependencies installed, and if some of them are missing, script should fail. Recently we had a situation when docker image was missing sphinx and flashrom Jenkins was silently skipping building man-pages and documentation. This patch prevents this happening again, because building man pages and docs will be enforced. Change-Id: Ib89abddad27d1168cf0a621cf4bdb9f541266165 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/meson.build M test_build.sh 2 files changed, 9 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/81665/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/81665?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: Ib89abddad27d1168cf0a621cf4bdb9f541266165 Gerrit-Change-Number: 81665 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[main]: meson: Update CI script to enforce building man pages and docs
by Anastasia Klimchuk (Code Review) 04 Apr '24

04 Apr '24

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/81665?usp=email ) Change subject: meson: Update CI script to enforce building man pages and docs ...................................................................... meson: Update CI script to enforce building man pages and docs `test_build.sh` is used by Jenkins, therefore it should build everything. Docker container for Jenkins is expected to have all the dependencies installed, and if some of them are missing, script should fail. Recently we had a situation when docker image was missing sphinx and flashrom Jenkins was silently skipping building man-pages and documentation. This patch prevents this happening again, because building man pages and docs will be enforced. Change-Id: Ib89abddad27d1168cf0a621cf4bdb9f541266165 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/meson.build M test_build.sh 2 files changed, 9 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/65/81665/1 diff --git a/doc/meson.build b/doc/meson.build index 5bc57b9..a2dad0e 100644 --- a/doc/meson.build +++ b/doc/meson.build @@ -34,4 +34,12 @@ ) endif +else # sphinx-build not found + if get_option('man-pages').enabled() + error('Cannot build man-pages because sphinx-build is missing.') + endif + + if get_option('documentation').enabled() + error('Cannot build documentation because sphinx-build is missing.') + endif endif diff --git a/test_build.sh b/test_build.sh index 9b490dc..081738b 100755 --- a/test_build.sh +++ b/test_build.sh @@ -51,7 +51,7 @@ build_meson () { build_dir=out - meson_opts="-Dtests=enabled" + meson_opts="-Dtests=enabled -Dman-pages-enabled -Ddocumentation=enabled" ninja_opts="-j $(nproc)" rm -rf ${build_dir} -- To view, visit https://review.coreboot.org/c/flashrom/+/81665?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: Ib89abddad27d1168cf0a621cf4bdb9f541266165 Gerrit-Change-Number: 81665 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[main]: flashchips: Add support for Boya BY25D40
by Anastasia Klimchuk (Code Review) 04 Apr '24

04 Apr '24

Attention is currently required from: Dmitry Mikushin, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81605?usp=email ) Change subject: flashchips: Add support for Boya BY25D40 ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/81605/comment/42dbbc60_9060d74b : PS1, Line 9: tested on YW500U3M industrial camera Wow this is unusual! Which programmer were you using? Patchset: PS1: Thank you for contribution! Do you have a link to the datasheet for the chip? -- To view, visit https://review.coreboot.org/c/flashrom/+/81605?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: I9e07a48dce3988e9d75cf02fca1f4a048c2e0cad Gerrit-Change-Number: 81605 Gerrit-PatchSet: 1 Gerrit-Owner: Dmitry Mikushin <dmitry(a)kernelgen.org> Gerrit-Reviewer: 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Dmitry Mikushin <dmitry(a)kernelgen.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 04 Apr 2024 03:15:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: [WIP] tests: Add tests for erasing chip with protected region
by Anastasia Klimchuk (Code Review) 04 Apr '24

04 Apr '24

Attention is currently required from: Nikolai Artemiev. Anastasia Klimchuk 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) This change is ready for review. File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/81664/comment/97765eb6_3a1ae879 : PS1, Line 130: TEST_ERASE_INJECTOR One thing I haven't figured yet how to do in this test: It is important to distinguish which erasefn has been selected, however there is only one TEST_ERASE_INJECTOR value for all tests. So one and the same erasefn for all sizes of eraseblocks (as you can see in the code below), while in real chip that would be different functions. If we can distinguish which eraseblock fn has been selected , that would be last remaining bit to fully test 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-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 04 Apr 2024 02:40:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: [WIP] tests: Add tests for erasing chip with protected region
by Anastasia Klimchuk (Code Review) 04 Apr '24

04 Apr '24

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/81664?usp=email ) Change subject: [WIP] tests: Add tests for erasing chip with protected region ...................................................................... [WIP] tests: Add tests for erasing chip with protected region Change-Id: I1e7cfd027d37437c917c054f30f3be59b76b0069 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M tests/erase_func_algo.c M tests/tests.c M tests/tests.h 3 files changed, 291 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/81664/1 diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index ed9eb04..a600de2 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -49,8 +49,8 @@ uint8_t written_buf[MOCK_CHIP_SIZE]; /* Expected content after write. */ struct erase_invoke eraseblocks_expected[MOCK_CHIP_SIZE]; /* Expected order of eraseblocks invocations. */ unsigned int eraseblocks_expected_ind; /* Expected number of eraseblocks invocations. */ - char erase_test_name[20]; /* Test case display name for testing erase operation. */ - char write_test_name[20]; /* Test case display name for testing write operation. */ + char erase_test_name[40]; /* Test case display name for testing erase operation. */ + char write_test_name[40]; /* Test case display name for testing write operation. */ }; struct all_state { @@ -703,6 +703,127 @@ }, }; + +#define START_PROTECTED_REGION 8 +#define END_PROTECTED_REGION 16 + +/* + * Setup all test cases with protected region. + * Protected region is the same for all test cases, between bytes 8 - 15. + */ +static struct test_case test_cases_protected_region[] = { + { + /* + * Test case #0 + * + * Initial vs written: all 16 bytes are different. + * One layout region for the whole chip. + * Chip with eraseblocks 1, 2, 4, 8, 16. + */ + .chip = &chip_1_2_4_8_16, + .regions = {{0, MIN_REAL_CHIP_SIZE - 1, "whole chip"}}, + .initial_buf = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, + 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}, + .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected_ind = 1, + .erase_test_name = "Erase protected region test case #0", + }, { + /* + * Test case #1 + * + * Initial vs written: 9 bytes the same, 7 bytes different. + * Two layout regions each one 8 bytes, which is 1/2 size of chip. + * Chip with eraseblocks 1, 2, 4, 8, 16. + */ + .chip = &chip_1_2_4_8_16, + .regions = {{0, MOCK_CHIP_SIZE/2 - 1, "part1"}, {MOCK_CHIP_SIZE/2, MIN_REAL_CHIP_SIZE - 1, "part2"}}, + .initial_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, + 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff}, + .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected_ind = 1, + .erase_test_name = "Erase protected region test case #1", + }, { + /* + * Test case #2 + * + * Initial vs written: 6 bytes the same, 4 bytes different, 4 bytes the same, 2 bytes different. + * Two layout regions 11 and 5 bytes each. + * Chip with eraseblocks 1, 2, 4, 8, 16. + */ + .chip = &chip_1_2_4_8_16, + .regions = {{0, 10, "odd1"}, {11, 15, "odd2"}, {MOCK_CHIP_SIZE, MIN_REAL_CHIP_SIZE - 1, "longtail"}}, + .initial_buf = {0xff, 0xff, 0x0, 0xff, 0x0, 0xff, 0x0, 0xff, + 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff}, + .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected_ind = 1, + .erase_test_name = "Erase protected region test case #2", + }, { + /* + * Test case #3 + * + * 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, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x7}, + .eraseblocks_expected = {{0x0, 0x8}, {0x0, 0x8}, {0x0, 0x8}}, + .eraseblocks_expected_ind = 3, + .erase_test_name = "Erase protected region test case #3", + }, { + /* + * Test case #4 + * + * 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, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .eraseblocks_expected = {{0x0, 0x8}, {0x0, 0x8}}, + .eraseblocks_expected_ind = 2, + .erase_test_name = "Erase protected region test case #4", + }, { + /* + * Test case #5 + * + * 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, + 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .eraseblocks_expected = {{0x0, 0x8}}, + .eraseblocks_expected_ind = 1, + .erase_test_name = "Erase test case #5", + }, +}; + static int setup(void **state) { struct test_case *current_test_case = *state; g_state.current_test_case = current_test_case; @@ -746,6 +867,29 @@ } /* + * Creates the array of tests for each test case in test_cases_protected_region[]. + * The caller needs to free the allocated memory. + */ +struct CMUnitTest *get_erase_protected_region_algo_tests(size_t *num_tests) { + const size_t test_cases_num = ARRAY_SIZE(test_cases_protected_region); + + struct CMUnitTest *all_cases = calloc(test_cases_num, sizeof(struct CMUnitTest)); + *num_tests = test_cases_num; + + for (size_t i = 0; i < test_cases_num; i++) { + all_cases[i] = (struct CMUnitTest) { + .name = test_cases_protected_region[i].erase_test_name, + .setup_func = setup, + .teardown_func = teardown, + .initial_state = &test_cases_protected_region[i], + .test_func = erase_unwritable_regions_skipflag_on_test_success, + }; + } + + return all_cases; +} + +/* * This function is invoked for every test case in test_cases[], * current test case is passed as an argument. */ @@ -845,3 +989,135 @@ assert_int_equal(0, all_write_test_result); } + +static void get_protected_region(const struct flashctx *flash, unsigned int addr, struct flash_region *region) +{ + if (addr < 20) + printf("Inside test_get_flash_region for addr=0x%x\n", addr); + + if (addr < START_PROTECTED_REGION) { + region->name = strdup("not protected"); + region->start = 0; + region->end = START_PROTECTED_REGION; + region->read_prot = false; + region->write_prot = false; + } else if (addr < END_PROTECTED_REGION) { + region->name = strdup("protected"); + region->start = START_PROTECTED_REGION; + region->end = END_PROTECTED_REGION; + region->read_prot = true; + region->write_prot = true; + } else { + region->name = strdup("tail"); + region->start = 16; + region->end = flashrom_flash_getsize(flash); + region->read_prot = false; + region->write_prot = false; + } +} + +static int block_erase_chip_with_protected_region(struct flashctx *flash, 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_ind++; + } + + assert_in_range(blockaddr + blocklen, 0, MIN_REAL_CHIP_SIZE); + + // Check we are not trying to erase protected region. This should not happen, + // because the logic should handle protected regions and never invoke erasefn + // for them. If this happens, means there is a bug in erasure logic, and test fails. + // + // 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. + if ((blockaddr >= START_PROTECTED_REGION && blockaddr < END_PROTECTED_REGION) + || (blockaddr + blocklen - 1 >= START_PROTECTED_REGION + && blockaddr + blocklen - 1 < END_PROTECTED_REGION)) + return 1; + + memset(&g_state.buf[blockaddr], ERASE_VALUE, blocklen); + return 0; +} + +void erase_unwritable_regions_skipflag_on_test_success(void **state) +{ + struct test_case* current_test_case = *state; + + int all_erase_tests_result = 0; + struct flashrom_flashctx flashctx = { 0 }; + const char *param = ""; /* Default values for all params. */ + + struct flashrom_layout *layout; + + 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; + + flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS, true); + + // We use dummyflasher programmer in tests, but for this test we need to + // replace dummyflasher's default get_region fn with test one. + // The rest of master struct is fine for this test. + // Note dummyflasher registers multiple masters by default, so replace + // get_region for each of them. + flashctx.mst->spi.get_region = &get_protected_region; + flashctx.mst->opaque.get_region = &get_protected_region; + + printf("%s started.\n", current_test_case->erase_test_name); + int ret = flashrom_flash_erase(&flashctx); + printf("%s returned %d.\n", current_test_case->erase_test_name, ret); + + int chip_erased = !memcmp(g_state.buf, current_test_case->erased_buf, MOCK_CHIP_SIZE); + + int eraseblocks_in_order = !memcmp(g_state.eraseblocks_actual, + current_test_case->eraseblocks_expected, + current_test_case->eraseblocks_expected_ind * sizeof(struct erase_invoke)); + + int eraseblocks_invocations = (g_state.eraseblocks_actual_ind == + current_test_case->eraseblocks_expected_ind); + + if (chip_erased) + printf("Erased chip memory state for %s is CORRECT\n", + current_test_case->erase_test_name); + else + printf("Erased chip memory state for %s is WRONG\n", + current_test_case->erase_test_name); + + if (eraseblocks_in_order) + printf("Eraseblocks order of invocation for %s is CORRECT\n", + current_test_case->erase_test_name); + else + printf("Eraseblocks order of invocation for %s is WRONG\n", + current_test_case->erase_test_name); + + if (eraseblocks_invocations) + printf("Eraseblocks number of invocations for %s is CORRECT\n", + current_test_case->erase_test_name); + else + printf("Eraseblocks number of invocations for %s is WRONG, expected %d actual %d\n", + current_test_case->erase_test_name, + current_test_case->eraseblocks_expected_ind, + g_state.eraseblocks_actual_ind); + + all_erase_tests_result |= ret; + all_erase_tests_result |= !chip_erased; + all_erase_tests_result |= !eraseblocks_in_order; + all_erase_tests_result |= !eraseblocks_invocations; + + teardown_chip(&layout); + + assert_int_equal(0, all_erase_tests_result); +} + +void erase_unwritable_regions_skipflag_off_test_success(void **state) +{ +// flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS, false); +} diff --git a/tests/tests.c b/tests/tests.c index c38a66c..bff24e5 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -509,6 +509,16 @@ ret |= _cmocka_run_group_tests("erase_func_algo.c tests", erase_func_algo_tests, n_erase_tests, NULL, NULL); free(erase_func_algo_tests); + size_t n_erase_protected_region_tests; + struct CMUnitTest *erase_protected_region_algo_tests + = get_erase_protected_region_algo_tests(&n_erase_protected_region_tests); + ret |= _cmocka_run_group_tests("erase_func_algo.c protected region tests", + erase_protected_region_algo_tests, + n_erase_protected_region_tests, + NULL, + NULL); + free(erase_protected_region_algo_tests); + // Write-protect group should run last. const struct CMUnitTest chip_wp_tests[] = { cmocka_unit_test(invalid_wp_range_dummyflasher_test_success), diff --git a/tests/tests.h b/tests/tests.h index af5f968..7f8fc3c 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -107,7 +107,10 @@ /* erase_func_algo.c */ struct CMUnitTest *get_erase_func_algo_tests(size_t *num_tests); +struct CMUnitTest *get_erase_protected_region_algo_tests(size_t *num_tests); void erase_function_algo_test_success(void **state); void write_function_algo_test_success(void **state); +void erase_unwritable_regions_skipflag_off_test_success(void **state); +void erase_unwritable_regions_skipflag_on_test_success(void **state); #endif /* TESTS_H */ -- 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-MessageType: newchange

1 0

[S] Change in flashrom[main]: erasure_layout: do not copy region buffers if they are null
by Alexander Goncharov (Code Review) 03 Apr '24

03 Apr '24

Attention is currently required from: Aarya, Anastasia Klimchuk, Nikolai Artemiev. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81548?usp=email ) Change subject: erasure_layout: do not copy region buffers if they are null ...................................................................... Patch Set 1: (1 comment) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/81548/comment/e1b115ea_fb024bc5 : PS1, Line 263: old_start_buf = (uint8_t *)malloc(old_start - region_start); : if (!old_start_buf) { : msg_cerr("Not enough memory!\n"); : ret = -1; : goto _end; : } : old_end_buf = (uint8_t *)malloc(region_end - old_end); : if (!old_end_buf) { : msg_cerr("Not enough memory!\n"); : ret = -1; : goto _end; : } > Oh I think this piece is also somewhat relevant to the patch. […] Thanks for the details, Anastasia! Yeah, I will dive deeper to see possible solutions and understand code better. I have a few thoughts on how to rewrite it, but I need to test them. -- To view, visit https://review.coreboot.org/c/flashrom/+/81548?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: I99aad4119f9c11bea75e3419926cc833bc1f68c5 Gerrit-Change-Number: 81548 Gerrit-PatchSet: 1 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 03 Apr 2024 16:07:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25L3273F
by Anastasia Klimchuk (Code Review) 03 Apr '24

03 Apr '24

Attention is currently required from: DZ, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81562?usp=email ) Change subject: flashchips: Add support for MXIC MX25L3273F ...................................................................... Patch Set 7: Code-Review+1 (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/81562/comment/75e0403f_84a2a4f9 : PS7, Line 9522: 1024B Same as other patch: datasheet says 4K-bit secured OTP, so maybe 512B ? -- To view, visit https://review.coreboot.org/c/flashrom/+/81562?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: I4adaaa796d1db34702e7b0ed8e6fb167a3a5f6d7 Gerrit-Change-Number: 81562 Gerrit-PatchSet: 7 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> Gerrit-Reviewer: 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: DZ <danielzhang(a)mxic.com.cn> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 03 Apr 2024 08:24:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25L3239E
by Anastasia Klimchuk (Code Review) 03 Apr '24

03 Apr '24

Attention is currently required from: DZ, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81563?usp=email ) Change subject: flashchips: Add support for MXIC MX25L3239E ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/81563/comment/5854d838_d299ecee : PS3, Line 9523: 1024B I found it is "4K-bit secured OTP" in datasheet, so maybe twice less? -- To view, visit https://review.coreboot.org/c/flashrom/+/81563?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: Ic7a848028fe937deb1bf83ef2a9dddf1330334b6 Gerrit-Change-Number: 81563 Gerrit-PatchSet: 3 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> Gerrit-Reviewer: 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: DZ <danielzhang(a)mxic.com.cn> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Wed, 03 Apr 2024 07:59:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[L] Change in flashrom[main]: udelay: only use OS time for delays, except on DOS
by Peter Marheine (Code Review) 03 Apr '24

03 Apr '24

Attention is currently required from: Anastasia Klimchuk, Brian Norris, Thomas Heijligen. Hello Anastasia Klimchuk, Brian Norris, Thomas Heijligen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/81545?usp=email to look at the new patch set (#7). Change subject: udelay: only use OS time for delays, except on DOS ...................................................................... udelay: only use OS time for delays, except on DOS As proposed on the mailing list ("RFC: remove the calibrated delay loop"), this removes the calibrated delay loop and uses OS-based timing functions for all delays because the calibrated delay loop can delay for shorter times than intended. When sleeping this now uses nanosleep() unconditionally, since usleep was only used on DOS (where DJGPP lacks nanosleep). When busy-looping, it uses clock_gettime() with CLOCK_MONOTONIC or CLOCK_REALTIME depending on availability, and gettimeofday() otherwise. The calibrated delay loop is retained for DOS only, because timer resolution on DJGPP is only about 50 milliseconds. Since typical delays in flashrom are around 10 microseconds, using OS timing there would regress performance by around 500x. The old implementation is reused with some branches removed based on the knowledge that timer resolution will not be better than about 50 milliseconds. Tested by reading and writing flash on several Intel and AMD systems: * Lenovo P920 (Intel C620, read/verify only) * "nissa" chromebook (Intel Alder Lake-N) * "zork" chromebook (AMD Zen+) Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> Change-Id: I7ac5450d194a475143698d65d64d8bcd2fd25e3f --- M Makefile M include/programmer.h M libflashrom.c M meson.build M tests/meson.build M tests/tests.c M tests/tests.h A tests/udelay.c M udelay.c A udelay_dos.c 10 files changed, 261 insertions(+), 172 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/81545/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/81545?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: I7ac5450d194a475143698d65d64d8bcd2fd25e3f Gerrit-Change-Number: 81545 Gerrit-PatchSet: 7 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[main]: udelay: only use OS time for delays, except on DOS
by Peter Marheine (Code Review) 03 Apr '24

03 Apr '24

Attention is currently required from: Anastasia Klimchuk, Brian Norris, Thomas Heijligen. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81545?usp=email ) Change subject: udelay: only use OS time for delays, except on DOS ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/81545/comment/dcab515f_21107f18 : PS4, Line 25: > For the patch like this, it would be useful to know what was tested, if you could add testing info h […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/81545?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: I7ac5450d194a475143698d65d64d8bcd2fd25e3f Gerrit-Change-Number: 81545 Gerrit-PatchSet: 6 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Brian Norris <briannorris(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Brian Norris <briannorris(a)chromium.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 03 Apr 2024 00:52:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit April 2024