Anastasia Klimchuk submitted this change.
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@hardenedlinux.org
Co-developed-by: Anastasia Klimchuk <aklm@flashrom.org>
Signed-off-by: persmule <persmule@hardenedlinux.org>
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84721
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Peter Marheine <pmarheine@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(-)
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 change 84721. To unsubscribe, or for help writing mail filters, visit settings.