Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim.
Anastasia Klimchuk has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/84827?usp=email )
Change subject: flashchips: add GD25F128F
......................................................................
Patch Set 1:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/84827/comment/b001940b_bcb02b63?us… :
PS1, Line 12: Will send datasheet via email
Is it possible that you will also include Nikolai in the email when you send the datasheet? Thank you!
--
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: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I14c0905f50e92492287d50f8377790b997c4acfe
Gerrit-Change-Number: 84827
Gerrit-PatchSet: 1
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>
Gerrit-Attention: Victor Lim <vlim(a)gigadevice.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Thu, 24 Oct 2024 09:54:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Attention is currently required from: Aarya, Anastasia Klimchuk.
Bill XIE has posted comments on this change by Bill XIE. ( https://review.coreboot.org/c/flashrom/+/84721?usp=email )
Change subject: erasure_layout: Add an option to sacrifice unchanged blocks for speed
......................................................................
Patch Set 4:
(2 comments)
Patchset:
PS3:
> Sorry for some delay, I am getting back to this. […]
Okay.
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84721/comment/a0090eab_8f259bfc?us… :
PS3, Line 127: harder
> Maybe "faster" instead of "harder"?
No problem. Done
--
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: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I154e8a713f626c37dbbe118db700055b96d24803
Gerrit-Change-Number: 84721
Gerrit-PatchSet: 4
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Thu, 24 Oct 2024 06:34:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Aarya, Bill XIE.
Hello Aarya, Anastasia Klimchuk, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/84721?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: erasure_layout: Add an option to sacrifice unchanged blocks for speed
......................................................................
erasure_layout: Add an option to sacrifice unchanged blocks for speed
Someone may prefer to sacrifice sub-blocks needless to change within a
larger erase block for programming speed. These people who are okay to
fry their chip sooner can set this option. If the percentage of
sub-blocks needless to change within a larger erase block is lower
than the given value, the whole larger erase block will be erased.
Change-Id: I154e8a713f626c37dbbe118db700055b96d24803
Signed-off-by: persmule <persmule(a)hardenedlinux.org>
---
M cli_classic.c
M erasure_layout.c
M include/flash.h
3 files changed, 25 insertions(+), 3 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/84721/4
--
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: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I154e8a713f626c37dbbe118db700055b96d24803
Gerrit-Change-Number: 84721
Gerrit-PatchSet: 4
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Attention is currently required from: Aarya, Bill XIE.
Anastasia Klimchuk has posted comments on this change by Bill XIE. ( https://review.coreboot.org/c/flashrom/+/84721?usp=email )
Change subject: erasure_layout: Add an option to sacrifice unchanged blocks for speed
......................................................................
Patch Set 3:
(3 comments)
Patchset:
PS3:
Sorry for some delay, I am getting back to this. I want to add a test into this patch, because now it's possible to add such test (after CB:84783).
If it goes by the plan, I will upload new patchset with test, hope it's fine.
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/84721/comment/130b3202_9beea412?us… :
PS3, Line 124: " --sacrifice-ratio <ratio> The percentage of overall sub-blocks needless to\n"
: " change within a larger erase block to be sacrificed\n"
: " for programming speed, default 0, max 100\n"
I was thinking, maybe to change this description a bit, to make it more clear: but I haven't composed anything in my head yet :) just keep thinking on the background.
If I don't come up with anything, then that will be it.
In any case, definitely will keep the DANGEROUS! warning.
https://review.coreboot.org/c/flashrom/+/84721/comment/ad5825c2_84a534a3?us… :
PS3, Line 127: harder
Maybe "faster" instead of "harder"?
--
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: comment
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: I154e8a713f626c37dbbe118db700055b96d24803
Gerrit-Change-Number: 84721
Gerrit-PatchSet: 3
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Thu, 24 Oct 2024 01:57:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/84686?usp=email )
(
3 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
)Change subject: erase/write: Deselect all smaller blocks when large block is selected
......................................................................
erase/write: Deselect all smaller blocks when large block is selected
Previously the logic which selected large block did deselect of
smaller blocks, but only one level below. So some even smaller blocks
could still remain selected, and this would result in duplicate erase.
This patch deselects all smaller blocks of all levels below, down to
the smallest size. If the area is covered by one large block, no
other smaller blocks inside are needed.
Change-Id: Icfc18d5c090b1dcb92ab157e2c139be71af59300
Spotted-by: persmule <persmule(a)hardenedlinux.org>
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
Co-authored-by: persmule <persmule(a)hardenedlinux.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84686
Reviewed-by: Peter Marheine <pmarheine(a)chromium.org>
Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org>
---
M erasure_layout.c
1 file changed, 31 insertions(+), 2 deletions(-)
Approvals:
Peter Marheine: Looks good to me, approved
build bot (Jenkins): Verified
diff --git a/erasure_layout.c b/erasure_layout.c
index 56841ab..7f3bb46 100644
--- a/erasure_layout.c
+++ b/erasure_layout.c
@@ -189,6 +189,27 @@
}
}
+/* Deselect all the blocks from index_to_deselect and down to the smallest. */
+static void deselect_erase_functions(const struct erase_layout *layout, size_t index_to_deselect,
+ int sub_block_start, const int sub_block_end)
+{
+ for (int j = sub_block_start; j <= sub_block_end; j++)
+ layout[index_to_deselect].layout_list[j].selected = false;
+
+ int block_start_to_deselect =
+ layout[index_to_deselect].layout_list[sub_block_start].first_sub_block_index;
+ int block_end_to_deselect =
+ layout[index_to_deselect].layout_list[sub_block_end].last_sub_block_index;
+
+ if (index_to_deselect)
+ deselect_erase_functions(layout,
+ index_to_deselect - 1,
+ block_start_to_deselect,
+ block_end_to_deselect);
+ else
+ return; // index_to_deselect has already reached 0, the smallest size of block. we are done.
+}
+
/*
* @brief Function to select the list of sectors that need erasing
*
@@ -229,9 +250,17 @@
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 (ll->start_addr >= rstart && ll->end_addr <= rend) {
- for (int j = sub_block_start; j <= sub_block_end; j++)
- layout[findex - 1].layout_list[j].selected = false;
+ /* Deselect all smaller blocks covering the same region. */
+ deselect_erase_functions(layout,
+ findex - 1,
+ sub_block_start,
+ sub_block_end);
+
+ /* Select large block. */
ll->selected = true;
}
}
--
To view, visit https://review.coreboot.org/c/flashrom/+/84686?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: Icfc18d5c090b1dcb92ab157e2c139be71af59300
Gerrit-Change-Number: 84686
Gerrit-PatchSet: 5
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
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/+/84840?usp=email )
Change subject: cli_classic: Add rpmc command support
......................................................................
Patch Set 1:
(1 comment)
File meson.build:
https://review.coreboot.org/c/flashrom/+/84840/comment/583f569c_5db29044?us… :
PS1, Line 179: add_project_arguments('-DCONFIG_RPMC_ENABLED=1', language : 'c')
rpmc should be added as an option (in `meson_options.txt`), so users can assert they want it enabled and the build will fail if dependencies aren't met.
If you do:
```
option('rpmc', type : 'feature', value : 'auto', description : 'Support for Replay Protected Monotonic Counter (RPMC) commands as specified by JESD260')
```
Then here libcrypto can be required sometimes:
```
libcrypto = dependency('libcrypto', required : get_option('rpmc'), version : '>=3.0.0')
...
if libcrypto.found()
# Add options for RPMC implementation
...
endif
```
..and you don't really need the `message()` anymore since a user can force the build to fail if they want RPMC support but are missing dependencies, instead of manually looking for the message.
--
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: 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: Thu, 24 Oct 2024 00:17:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No