- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[main]: erasure_layout: Add an option to sacrifice unchanged blocks for speed
by Anastasia Klimchuk (Code Review) 01 Nov '24

01 Nov '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/84721?usp=email ) Change subject: erasure_layout: Add an option to sacrifice unchanged blocks for speed ...................................................................... erasure_layout: Add an option to sacrifice unchanged blocks for speed The patch adds command line option to handle the following situation: There is a region which is requested to be erased (or written, because the write operation uses erase too). Some of the areas inside this region don't need to be erased, because the bytes already have expected value. Such areas can be skipped. The logic selects eraseblocks that can cover the areas which need to be erased. Suppose there is a region which is partially covered by eraseblocks of size S (partially because remaining areas don't need to be erased). Now suppose we can cover the whole region with eraseblock of larger size, S+1, and erase it all at once. This will run faster: erase opcode will only be sent once instead of many smaller opcodes. However, this will run erase over some areas of the chip memory that didn't need to be erased. Which means, the chip, as a hardware, will wear faster. New command line option sets the maximum % memory that is allowed for redundant erase. Default is 0, S+1 size block only selected if all the area needs to be erased in full. 50 means that if more than a half of the area needs to be erased, a S+1 size block can be selected to cover all area with one block. The tradeoff is the speed of programming operation VS the longevity of the chip. Default is longevity. Change-Id: I154e8a713f626c37dbbe118db700055b96d24803 Co-developed-by: persmule <persmule(a)hardenedlinux.org Co-developed-by: Anastasia Klimchuk <aklm(a)flashrom.org> Signed-off-by: persmule <persmule(a)hardenedlinux.org> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/84721 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M cli_classic.c M doc/classic_cli_manpage.rst M doc/release_notes/devel.rst M erasure_layout.c M include/flash.h M tests/erase_func_algo.c 6 files changed, 152 insertions(+), 6 deletions(-) Approvals: build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index 3e9db50..896c167 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -45,6 +45,7 @@ OPTION_WP_DISABLE, OPTION_WP_LIST, OPTION_PROGRESS, + OPTION_SACRIFICE_RATIO, }; struct cli_options { @@ -73,6 +74,7 @@ char *logfile; char *referencefile; const char *chip_to_probe; + int sacrifice_ratio; }; static void cli_classic_usage(const char *name) @@ -119,6 +121,14 @@ " --flash-contents <ref-file> assume flash contents to be <ref-file>\n" " -L | --list-supported print supported devices\n" " --progress show progress percentage on the standard output\n" + " --sacrifice-ratio <ratio> Fraction (as a percentage, 0-50) of an erase block\n" + " that may be erased even if unmodified. Larger values\n" + " may complete programming faster, but may also hurt\n" + " chip longevity by erasing cells unnecessarily.\n" + " Default is 0, tradeoff is the speed of programming\n" + " operation VS the longevity of the chip. Default is\n" + " longevity.\n" + " DANGEROUS! It wears your chip faster!\n" " -p | --programmer <name>[:<param>] specify the programmer device. One of\n"); list_programmers_linebreak(4, 80, 0); printf(".\n\nYou can specify one of -h, -R, -L, " @@ -810,6 +820,10 @@ case OPTION_PROGRESS: options->show_progress = true; break; + case OPTION_SACRIFICE_RATIO: + /* It is okay to convert invalid input to 0. */ + options->sacrifice_ratio = atoi(optarg); + break; default: cli_classic_abort_usage(NULL); break; @@ -879,6 +893,7 @@ {"version", 0, NULL, 'R'}, {"output", 1, NULL, 'o'}, {"progress", 0, NULL, OPTION_PROGRESS}, + {"sacrifice-ratio", 1, NULL, OPTION_SACRIFICE_RATIO}, {NULL, 0, NULL, 0}, }; @@ -1125,6 +1140,14 @@ goto out_shutdown; } + if (options.sacrifice_ratio) { + if (options.sacrifice_ratio < 0 || options.sacrifice_ratio > 50) { + msg_ginfo("Invalid input of sacrifice ratio, valid 0-50. Fallback to default value 0.\n"); + options.sacrifice_ratio = 0; + } + fill_flash->sacrifice_ratio = options.sacrifice_ratio; + } + if (options.ifd && (flashrom_layout_read_from_ifd(&options.layout, fill_flash, NULL, 0) || process_include_args(options.layout, options.include_args))) { ret = 1; diff --git a/doc/classic_cli_manpage.rst b/doc/classic_cli_manpage.rst index cdb7060..f0fd681 100644 --- a/doc/classic_cli_manpage.rst +++ b/doc/classic_cli_manpage.rst @@ -20,7 +20,7 @@ | [--wp-status] [--wp-list] [--wp-enable|--wp-disable] | [--wp-range <start>,<length>|--wp-region <region>] | [-n] [-N] [-f])] -| [-V[V[V]]] [-o <logfile>] [--progress] +| [-V[V[V]]] [-o <logfile>] [--progress] [--sacrifice-ratio <ratio>] DESCRIPTION @@ -319,6 +319,17 @@ **--progress** [Experimental feature] Show progress percentage of operations on the standard output. +**--sacrifice-ratio <ratio>** + Fraction (as a percentage, 0-50) of an erase block that may be erased even if unmodified. + Larger values may complete programming faster, but may also hurt chip longevity by erasing cells unnecessarily. + + Default is 0, S+1 size block only selected if all the S size blocks inside it need to be erased in full. + 50 means that if more than a half of the area needs to be erased, + a S+1 size block can be selected to cover all the area with one erase. + The tradeoff is the speed of programming operation VS the longevity of the chip. Default is longevity. + + DANGEROUS! It wears your chip faster! + **-R, --version** Show version information and exit. diff --git a/doc/release_notes/devel.rst b/doc/release_notes/devel.rst index f54b5f1..e0c42a5 100644 --- a/doc/release_notes/devel.rst +++ b/doc/release_notes/devel.rst @@ -38,6 +38,35 @@ Progress display feature is now working for all operations: read, erase, write. +Command-line option to sacrifice unchanged blocks for speed +=========================================================== + +New command line option ``sacrifice-ratio`` handles the following situation: + +There is a region which is requested to be erased (or written, because +the write operation uses erase too). Some of the areas inside this +region don't need to be erased, because the bytes already have expected +value. Such areas can be skipped. + +The logic selects eraseblocks that can cover the areas which need to be +erased. Suppose there is a region which is partially covered by +eraseblocks of size S (partially because remaining areas don't need to +be erased). Now suppose we can cover the whole region with eraseblock +of larger size, S+1, and erase it all at once. This will run faster: +erase opcode will only be sent once instead of many smaller opcodes. +However, this will run erase over some areas of the chip memory that +didn't need to be erased. Which means the physical chip will wear out +faster. + +This new option sets the maximum % of memory that is allowed for +redundant erase. Default is 0, S+1 size block only selected if all the +area needs to be erased in full. 50 means that if more than a half of +the area needs to be erased, a S+1 size block can be selected to cover +the entire area with one block. + +The tradeoff is the speed of programming operation VS the longevity of +the chip. Default is longevity. + Chipset support =============== diff --git a/erasure_layout.c b/erasure_layout.c index 55da163..9b99c68 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -249,9 +249,11 @@ } const int total_blocks = sub_block_end - sub_block_start + 1; - if (count == total_blocks) { - /* We are selecting one large block instead, so send opcode once - * instead of sending many smaller ones. + if (total_blocks - count <= total_blocks * flashctx->sacrifice_ratio / 100) { + /* Number of smaller blocks not needed to change is lower than the + * sacrifice ratio, so we can sacrifice them. + * We are selecting one large block to cover the area, so + * send opcode once instead of sending many smaller ones. */ if (ll->start_addr >= rstart && ll->end_addr <= rend) { /* Deselect all smaller blocks covering the same region. */ diff --git a/include/flash.h b/include/flash.h index 60c099c..f13d202 100644 --- a/include/flash.h +++ b/include/flash.h @@ -593,6 +593,9 @@ flashrom_progress_callback *progress_callback; struct flashrom_progress *progress_state; struct stage_progress stage_progress[FLASHROM_PROGRESS_NR]; + + /* Maximum allowed % of redundant erase */ + int sacrifice_ratio; }; /* Timing used in probe routines. ZERO is -2 to differentiate between an unset diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index cec4879..0d37b07 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -1191,6 +1191,7 @@ } static void test_erase_fails_for_unwritable_region(void **); +static void test_write_with_sacrifice_ratio50(void **); static void erase_unwritable_regions_skipflag_on_test_success(void **); static void write_unwritable_regions_skipflag_on_test_success(void **); @@ -1200,7 +1201,7 @@ */ struct CMUnitTest *get_erase_protected_region_algo_tests(size_t *num_tests) { const size_t num_parameterized = ARRAY_SIZE(test_cases_protected_region); - const size_t num_unparameterized = 1; + const size_t num_unparameterized = 2; // Twice the number of parameterized test cases, because each test case is run twice: // for erase and write. const size_t num_cases = num_parameterized * 2 + num_unparameterized; @@ -1231,7 +1232,11 @@ (const struct CMUnitTest) { .name = "erase failure for unskipped unwritable regions", .test_func = test_erase_fails_for_unwritable_region, - } + }, + (const struct CMUnitTest) { + .name = "write with sacrifice ratio 50", + .test_func = test_write_with_sacrifice_ratio50, + }, }, sizeof(*all_cases) * num_unparameterized ); @@ -1705,3 +1710,76 @@ assert_int_not_equal(ret, 0); } + +static void test_write_with_sacrifice_ratio50(void **state) { + struct test_case* current_test_case = &test_cases[20]; + + int all_write_test_result = 0; + struct flashrom_flashctx flashctx = { + /* If eraseblocks of smaller size fill in more than a half of the area, + * erase one larger size block instead. */ + .sacrifice_ratio = 50, + }; + /* Custom expectations because sacrifice ratio is modified (not default). */ + struct erase_invoke eraseblocks_expected = {0x0, 0x10, TEST_ERASE_INJECTOR_5}; + unsigned int eraseblocks_expected_ind = 1; + + uint8_t newcontents[MIN_BUF_SIZE]; + const char *param = ""; /* Default values for all params. */ + + struct flashrom_layout *layout; + + const chipoff_t verify_end_boundary = setup_chip(&flashctx, &layout, param, current_test_case); + memcpy(&newcontents, current_test_case->written_buf, MOCK_CHIP_SIZE); + + printf("%s started.\n", __func__); + int ret = flashrom_image_write(&flashctx, &newcontents, MIN_BUF_SIZE, NULL); + printf("%s returned %d.\n", __func__, ret); + + int chip_written = !memcmp(g_state.buf, current_test_case->written_buf, MOCK_CHIP_SIZE); + + int eraseblocks_in_order = !memcmp(g_state.eraseblocks_actual, &eraseblocks_expected, + eraseblocks_expected_ind * sizeof(struct erase_invoke)); + + int eraseblocks_invocations = (g_state.eraseblocks_actual_ind == eraseblocks_expected_ind); + + int chip_verified = 1; + for (unsigned int i = 0; i <= verify_end_boundary; i++) + if (g_state.was_modified[i] && !g_state.was_verified[i]) { + chip_verified = 0; /* the byte was modified, but not verified after */ + printf("Error: byte 0x%x, modified: %d, verified: %d\n", i, g_state.was_modified[i], g_state.was_verified[i]); + } + + if (chip_written) + printf("Written chip memory state for %s is CORRECT\n", __func__); + else + printf("Written chip memory state for %s is WRONG\n", __func__); + + if (eraseblocks_in_order) + printf("Eraseblocks order of invocation for %s is CORRECT\n", __func__); + else + printf("Eraseblocks order of invocation for %s is WRONG\n", __func__); + + if (eraseblocks_invocations) + printf("Eraseblocks number of invocations for %s is CORRECT\n", __func__); + else + printf("Eraseblocks number of invocations for %s is WRONG, expected %d actual %d\n", + __func__, + eraseblocks_expected_ind, + g_state.eraseblocks_actual_ind); + + if (chip_verified) + printf("Written chip memory state for %s was verified successfully\n", __func__); + else + printf("Written chip memory state for %s was NOT verified completely\n", __func__); + + all_write_test_result |= ret; + all_write_test_result |= !chip_written; + all_write_test_result |= !eraseblocks_in_order; + all_write_test_result |= !eraseblocks_invocations; + all_write_test_result |= !chip_verified; + + teardown_chip(&layout); + + assert_int_equal(0, all_write_test_result); +} -- To view, visit https://review.coreboot.org/c/flashrom/+/84721?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: I154e8a713f626c37dbbe118db700055b96d24803 Gerrit-Change-Number: 84721 Gerrit-PatchSet: 12 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> 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>

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Peter Marheine (Code Review) 31 Oct '24

31 Oct '24

Attention is currently required from: Anastasia Klimchuk, Matti Finder. Peter Marheine has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email ) Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 2: (1 comment) File include/rpmc.h: https://review.coreboot.org/c/flashrom/+/84934/comment/5b67ddf3_b8ad00be?us… : PS1, Line 37: uint8_t status; > Within JESD260 there is a specific 8-bit extended status register defined. […] I don't mind returning 0x80 on success, but the documentation comments should be clear on how to interpret the return value. -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 2 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com> Gerrit-Comment-Date: Thu, 31 Oct 2024 22:36:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matti Finder <matti.finder(a)gmail.com> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Matti Finder (Code Review) 31 Oct '24

31 Oct '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Matti Finder has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email ) Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 2: (3 comments) File include/rpmc.h: https://review.coreboot.org/c/flashrom/+/84934/comment/d7912236_6d9d1418?us… : PS1, Line 37: uint8_t status; > It looks like you use status values in the CLI and 0x80 always indicates success, but that's not a v […] Within JESD260 there is a specific 8-bit extended status register defined. The status for last command success is 0b10000000, hence the 0x80. The status 0 is only used as the state immideatly after reboot, so it should never happen after one of our transmissions. So we could also use it as a return value without losing valuable data. What would you prefer keeping it close to JESD260 and returning the status register value, and documenting it better (maybe adding a enum for the values). Or cheating a bit and returning 0 on success instead of 0x80. I currently prefer the second option, with some additional defines for the bits. File meson.build: https://review.coreboot.org/c/flashrom/+/84934/comment/aa3c01a9_54dc9fd4?us… : PS1, Line 177: enabled > `enabled` is only true if the feature is set to `enabled` (not `auto`). […] Done File rpmc.c: https://review.coreboot.org/c/flashrom/+/84934/comment/794bd7c8_11441024?us… : PS1, Line 287: unsigned char msg[RPMC_UPDATE_HMAC_KEY_MSG_LENGTH]; > I'd prefer to use initializers for arrays like this (throughout this file), to make it less prone to […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 2 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 31 Oct 2024 16:21:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Matti Finder (Code Review) 31 Oct '24

31 Oct '24

Attention is currently required from: Anastasia Klimchuk, Matti Finder. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84934?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: rpmc: add rpmc commands feature ...................................................................... rpmc: add rpmc commands feature Added optional support for all the commands specified in JESD260. Added a new optional dependency to openssls libcrypto. Added parsing for the rpmc parameter sfdp table. Added necessary rpmc parameter information to flashchips struct and the flash hardening feature to enable rpmc commands. Enables future use of these commands in the cli_client and also libflashrom. Change-Id: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Signed-off-by: Matti Finder <matti.finder(a)gmail.com> --- M include/flash.h A include/rpmc.h M meson.build M meson_options.txt A rpmc.c M sfdp.c 6 files changed, 670 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/84934/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 2 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>

1 0

[M] Change in flashrom[main]: Display progress for what is actually erased/written
by Sergii Dmytruk (Code Review) 31 Oct '24

31 Oct '24

Attention is currently required from: Anastasia Klimchuk. Sergii Dmytruk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84439?usp=email ) Change subject: Display progress for what is actually erased/written ...................................................................... Patch Set 17: Code-Review+2 (2 comments) Patchset: PS16: > Just out of curiosity I ran the test case from here https://review.coreboot. […] Read also reaches 50% for the second command: ``` Found Winbond flash chip "W25Q128.V" (16384 kB, SPI) on dummy. Reading old flash chip contents... [READ: 100%]...done. Updating flash chip contents... [READ: 50%][ERASE: 50%][WRITE: 100%]...Erase/write done from 0 to ffffff Verifying flash... [READ: 100%]...VERIFIED. ``` Erasure is probably the result of selecting more fine-grained erasure functions in `erasure_layout.c`. Not sure about reads. File flashrom.c: https://review.coreboot.org/c/flashrom/+/84439/comment/a8f29ee5_d947e26b?us… : PS13, Line 1270: // FIXME: round up to granularity? > Okay so I will close this (and the other one). […] Sure. -- To view, visit https://review.coreboot.org/c/flashrom/+/84439?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: I88ac4d40f1b6ccc1636b1efb690d8d68bdebec08 Gerrit-Change-Number: 84439 Gerrit-PatchSet: 17 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 31 Oct 2024 14:44:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>

1 0

[S] Change in flashrom[main]: flashchips: add GD25F128F
by Anastasia Klimchuk (Code Review) 31 Oct '24

31 Oct '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/84827?usp=email ) Change subject: flashchips: add GD25F128F ...................................................................... flashchips: add GD25F128F GD25F128F: 3V 128Mbit, high performance Tested on ch347 with erase, write, read, and protection Corrected the OTP statement. Change-Id: I14c0905f50e92492287d50f8377790b997c4acfe Signed-off-by: Victor <vlim(a)gigadevice.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/84827 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashchips.c M include/flashchips.h 2 files changed, 46 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index f480078..c32159d 100644 --- a/flashchips.c +++ b/flashchips.c @@ -8188,6 +8188,51 @@ .decode_range = DECODE_RANGE_SPI25, }, +{ + .vendor = "GigaDevice", + .name = "GD25F128F", + .bustype = BUS_SPI, + .manufacture_id = GIGADEVICE_ID, + .model_id = GIGADEVICE_GD25F128F, + .total_size = 16384, + .page_size = 256, + /* OTP: 3x2048B; read 0x48; write 0x42, erase 0x44 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR2 | FEATURE_WRSR3, + .tested = TEST_OK_PREWB, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 4096} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 512} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 256} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {16 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {16 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD, + .unlock = SPI_DISABLE_BLOCKPROTECT_BP4_SRWD, /* TODO: 2nd status reg (read with 0x35) */ + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {2700, 3600}, + .reg_bits = + { + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like TB */ + }, + .decode_range = DECODE_RANGE_SPI25, + }, + { .vendor = "GigaDevice", .name = "GD25F256F", diff --git a/include/flashchips.h b/include/flashchips.h index 886ddc9..719486e 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -402,6 +402,7 @@ #define GIGADEVICE_GD25VQ80C 0x4214 #define GIGADEVICE_GD25VQ16C 0x4215 #define GIGADEVICE_GD25F64F 0x4317 +#define GIGADEVICE_GD25F128F 0x4318 #define GIGADEVICE_GD25F256F 0x4319 #define GIGADEVICE_GD25LQ40 0x6013 #define GIGADEVICE_GD25LQ80 0x6014 -- To view, visit https://review.coreboot.org/c/flashrom/+/84827?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: I14c0905f50e92492287d50f8377790b997c4acfe Gerrit-Change-Number: 84827 Gerrit-PatchSet: 3 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> 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>

1 0

[S] Change in flashrom[main]: doc: Add chip models support into recent development
by Anastasia Klimchuk (Code Review) 30 Oct '24

30 Oct '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/84870?usp=email ) Change subject: doc: Add chip models support into recent development ...................................................................... doc: Add chip models support into recent development Change-Id: I9b96902a02b83d35f0a0f221bd1678b7edf99dad Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/84870 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M doc/release_notes/devel.rst 1 file changed, 45 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved diff --git a/doc/release_notes/devel.rst b/doc/release_notes/devel.rst index c542bea..f54b5f1 100644 --- a/doc/release_notes/devel.rst +++ b/doc/release_notes/devel.rst @@ -26,6 +26,7 @@ New Feature =========== + Libpci 3.13.0 and onwards support ECAM to access pci registers. Flashrom will be moved to ECAM from IO port 0xcf8/0xcfc if the libpci version is >= 3.13.0. The ECAM has been supported for a very long time, most platforms should support @@ -39,4 +40,48 @@ Chipset support =============== + Added Raptor Point PCH support. + +Chip model support added +======================== + +* FM25Q04 +* FM25Q64 +* FM25Q128 + +* GD25B128E +* GD25B256E +* GD25B512MF +* GD25F64F +* GD25F256F +* GD25R128E +* GD25R256E +* GD25R512MF +* GD25LB256F +* GD25LB512ME +* GD25LB512MF +* GD25LR256F +* GD25LR512MF +* GD25LF256F +* GD25LF512MF + +* MX25U25645G +* MX77U51250F + +* W25Q32JV_M + +* XM25LU64C +* XM25QH32C +* XM25QH32D +* XM25QH64D +* XM25QH128D +* XM25QH256D +* XM25QH512C +* XM25QH512D +* XM25QU16C +* XM25QU32C +* XM25QU128D +* XM25QU256D +* XM25QU512C +* XM25QU512D -- To view, visit https://review.coreboot.org/c/flashrom/+/84870?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: I9b96902a02b83d35f0a0f221bd1678b7edf99dad Gerrit-Change-Number: 84870 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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>

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Peter Marheine (Code Review) 30 Oct '24

30 Oct '24

Attention is currently required from: Anastasia Klimchuk, Matti Finder. Peter Marheine has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email ) Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 1: (1 comment) File include/rpmc.h: https://review.coreboot.org/c/flashrom/+/84934/comment/38966ef0_ec9b0810?us… : PS1, Line 37: uint8_t status; It looks like you use status values in the CLI and 0x80 always indicates success, but that's not a value I would expect to indicate success. Can you add a little documentation here and perhaps also explain better what the function return values mean (even if it's just referring to an explanation attached to this field)? -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 1 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com> Gerrit-Comment-Date: Wed, 30 Oct 2024 22:35:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Peter Marheine (Code Review) 30 Oct '24

30 Oct '24

Attention is currently required from: Anastasia Klimchuk, Matti Finder. Peter Marheine has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email ) Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 1: (2 comments) File meson.build: https://review.coreboot.org/c/flashrom/+/84934/comment/6fb0d9a0_4affdf5c?us… : PS1, Line 177: enabled `enabled` is only true if the feature is set to `enabled` (not `auto`). You probably want `get_option('rpmc').allowed()` since that's true if it's either `auto` or `enabled`. File rpmc.c: https://review.coreboot.org/c/flashrom/+/84934/comment/7534816b_ddc98ec5?us… : PS1, Line 287: unsigned char msg[RPMC_UPDATE_HMAC_KEY_MSG_LENGTH]; I'd prefer to use initializers for arrays like this (throughout this file), to make it less prone to length-related errors. ``` unsigned char msg[RPMC_UPDATE_HMAC_KEY_MSG_LENGTH] = { flash->chip->rpmc_ctx.op1_opcode, 0x01, counter_address, ... }; ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 1 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com> Gerrit-Comment-Date: Wed, 30 Oct 2024 22:30:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in flashrom[main]: cli_client: Add rpmc command support
by Matti Finder (Code Review) 30 Oct '24

30 Oct '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Matti Finder has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84840?usp=email ) Change subject: cli_client: Add rpmc command support ...................................................................... Patch Set 5: (8 comments) This change is ready for review. Patchset: PS4: > Oh one more thing I forgot to mention sorry... […] 1) I think that makes sense I have created a new change at ID 84934 for the first commit and left this one as the documenation and cli_client change. 2) Done. 3) Yes I would like to become a maintainer for the rpmc feature :) Commit Message: https://review.coreboot.org/c/flashrom/+/84840/comment/f74ea504_d66478d3?us… : PS4, Line 21: usefull > usefull -> useful (typo) […] Done File cli_classic.c: https://review.coreboot.org/c/flashrom/+/84840/comment/4753fab4_29821d26?us… : PS4, Line 147: #if CONFIG_RPMC_ENABLED == 1 : "RPMC COMMANDS\n" : " --get-rpmc-status read the extended status\n" : " --write-root-key write the root key register\n" : " --update-hmac-key update the hmac key register\n" : " --increment-counter <previous>\n" : " increment rpmc counter\n" : " --get-counter get rpmc counter\n" : "RPMC OPTIONS\n" : " --counter-address <number> counter address (default: 0)\n" : " --rpmc-root-key <keyfile> rpmc root key file\n" : " --key-data <keydata> hex number to use as key data (default: 0)\n" : #endif // CONFIG_RPMC_ENABLED > These new commands need to also be documented on the manpage. […] Done https://review.coreboot.org/c/flashrom/+/84840/comment/8fc2ba09_c9679c38?us… : PS4, Line 1323: if (options.rpmc_read_data) : ret = rpmc_read_data(fill_flash); > I think having code in a separate file rpmc.c is better, you don't need to move it. Keep rpmc. […] Ok then File include/flash.h: https://review.coreboot.org/c/flashrom/+/84840/comment/7edbebcc_d5824ca9?us… : PS4, Line 553: implement > I know it's a small thing, but if you start with uppercase it would look better. This comment doesn't really make sense in the final commit, since it was more of an todo for me. I have removed it. https://review.coreboot.org/c/flashrom/+/84840/comment/fcf66055_c25ccc4d?us… : PS4, Line 575: /* : * all times in microsecond (us) : */ > Also small comment, but this can be 1-line comment, and starting with uppercase […] Done https://review.coreboot.org/c/flashrom/+/84840/comment/68c5445c_2d8c4f65?us… : PS4, Line 581: rpmc_ctx > I understand this goes into a flashchip definition? and you mentioned you tested on one chip? It wou […] This is sadly not as easily possible, since the chip reports itself with the same id as the W25Q128.V. So adding this feature would require changing the polling function to basically just be the same as the one for sfdp-capable chips. File meson.build: https://review.coreboot.org/c/flashrom/+/84840/comment/a486222c_9d85dce1?us… : PS4, Line 177: if libcrypto.found() : add_project_arguments('-DCONFIG_RPMC_ENABLED=1', language : 'c') > This ignores the meson option you added. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/84840?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: I36c823bbee65f256eb6edabe6f058321c9a0cfa1 Gerrit-Change-Number: 84840 Gerrit-PatchSet: 5 Gerrit-Owner: Matti Finder <matti.finder(a)gmail.com> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 30 Oct 2024 17:18:01 +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> Comment-In-Reply-To: Matti Finder <matti.finder(a)gmail.com>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit