diff --git a/cli_classic.c b/cli_classic.c index 3e9db50e4..896c1670a 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -45,6 +45,7 @@ enum { OPTION_WP_DISABLE, OPTION_WP_LIST, OPTION_PROGRESS, + OPTION_SACRIFICE_RATIO, }; struct cli_options { @@ -73,6 +74,7 @@ struct cli_options { char *logfile; char *referencefile; const char *chip_to_probe; + int sacrifice_ratio; }; static void cli_classic_usage(const char *name) @@ -119,6 +121,14 @@ static void cli_classic_usage(const char *name) " --flash-contents assume flash contents to be \n" " -L | --list-supported print supported devices\n" " --progress show progress percentage on the standard output\n" + " --sacrifice-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 [:] 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 @@ static void parse_options(int argc, char **argv, const char *optstring, 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 @@ int main(int argc, char *argv[]) {"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 @@ int main(int argc, char *argv[]) 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 cdb70601c..f0fd6812c 100644 --- a/doc/classic_cli_manpage.rst +++ b/doc/classic_cli_manpage.rst @@ -20,7 +20,7 @@ SYNOPSIS | [--wp-status] [--wp-list] [--wp-enable|--wp-disable] | [--wp-range ,|--wp-region ] | [-n] [-N] [-f])] -| [-V[V[V]]] [-o ] [--progress] +| [-V[V[V]]] [-o ] [--progress] [--sacrifice-ratio ] DESCRIPTION @@ -319,6 +319,17 @@ All operations involving any chip access (probe/read/write/...) require the ``-p **--progress** [Experimental feature] Show progress percentage of operations on the standard output. +**--sacrifice-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 f54b5f17e..e0c42a503 100644 --- a/doc/release_notes/devel.rst +++ b/doc/release_notes/devel.rst @@ -38,6 +38,35 @@ Progress display 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 55da163d3..9b99c68b3 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -249,9 +249,11 @@ static void select_erase_functions(struct flashctx *flashctx, const struct erase } 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 60c099cd5..f13d20288 100644 --- a/include/flash.h +++ b/include/flash.h @@ -593,6 +593,9 @@ struct flashrom_flashctx { 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 cec487979..0d37b077b 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -1191,6 +1191,7 @@ struct CMUnitTest *get_erase_func_algo_tests(size_t *num_tests) { } 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 @@ static void write_unwritable_regions_skipflag_on_test_success(void **); */ 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 @@ struct CMUnitTest *get_erase_protected_region_algo_tests(size_t *num_tests) { (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 @@ static void test_erase_fails_for_unwritable_region(void **state) { 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); +}