Attention is currently required from: Aarya, Bill XIE.
Hello Aarya, Bill XIE, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/84686?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: 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>
---
M erasure_layout.c
1 file changed, 31 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/84686/2
--
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: newpatchset
Gerrit-Project: flashrom
Gerrit-Branch: main
Gerrit-Change-Id: Icfc18d5c090b1dcb92ab157e2c139be71af59300
Gerrit-Change-Number: 84686
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.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: Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer.
Victor Lim has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/84090?usp=email )
Change subject: flashchips: add GD25F256F
......................................................................
Patch Set 1:
(3 comments)
Patchset:
PS1:
completed
Commit Message:
https://review.coreboot.org/c/flashrom/+/84090/comment/ab59d307_48623bdb?us… :
PS1, Line 10: Will send the datasheet.
> Got it by email, you can remove this line, thank you!
Done
File flashchips.c:
https://review.coreboot.org/c/flashrom/+/84090/comment/5034f9f4_a81de1a3?us… :
PS1, Line 8058: STATUS1, 7
> But this bit (S7 on page 14) is marked as Reserved?
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/84090?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: Ibbbbb8a55adbcbc2ee1785782c4eb3771d50c167
Gerrit-Change-Number: 84090
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: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Mon, 07 Oct 2024 21:41:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim.
Hello Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/84090?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: flashchips: add GD25F256F
......................................................................
flashchips: add GD25F256F
GD25F256F: 3V 256Mbit, high performance
Tested on ch347 with erase, write, read, and protection
Change-Id: Ibbbbb8a55adbcbc2ee1785782c4eb3771d50c167
Signed-off-by: Victor Lim <vlim(a)gigadevice.com>
---
M flashchips.c
M include/flashchips.h
2 files changed, 54 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/90/84090/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/84090?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: Ibbbbb8a55adbcbc2ee1785782c4eb3771d50c167
Gerrit-Change-Number: 84090
Gerrit-PatchSet: 2
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>
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/+/84614?usp=email )
Change subject: erasure_layout: Erase larger block only when all sub-block need erase
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> I will never consider to exchange programming speed with chip lifetime.
I mean, I will never consider "to exchange chip lifetime for programming speed", that is, to obtain programming speed at the cost of chip lifetime.
--
To view, visit https://review.coreboot.org/c/flashrom/+/84614?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: I9e10749186e395da67ec80e296119f33c3f83122
Gerrit-Change-Number: 84614
Gerrit-PatchSet: 2
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: Mon, 07 Oct 2024 13:57:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bill XIE <persmule(a)hardenedlinux.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Aarya, Anastasia Klimchuk.
Hello Aarya, Anastasia Klimchuk, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/84614?usp=email
to look at the new patch set (#2).
Change subject: erasure_layout: Erase larger block only when all sub-block need erase
......................................................................
erasure_layout: Erase larger block only when all sub-block need erase
A larger (not the smallest) erase block used to get erased when half
of sub-blocks it contains need erase, which has at least 2 issues:
1. The rest half of sub-blocks that do not need erase are also erased,
introducing some erase overheads.
2. More severely, since this logic only selects a block and delects
its sub-blocks when half of sub-blocks need erase, but this logic
does not deselect "nested sub-blocks (sub-blocks of sub-block)" not
reach the limit under this block, the logic may cause duplicated
erase. For example, if a erase block (often the largest one
corresponding to the whole chip) has half of its sub-blocks and
some incontiguous nested sub-blocks needing erase, these double
sub-blocks will end up being erased twice, introducing even more
erase overheads than whole-chip erase.
The older behavior of flashrom before adding erasure_layout.c, when no
communicational error occurs, will neither erase blocks that do not
need erase, nor cause duplicated erase. Higher efficiency should be
achieved without introducing extra erase overheads, by allowing
combining contiguous small erase blocks only when they can
coincidently form a larger erase block.
Change-Id: I9e10749186e395da67ec80e296119f33c3f83122
Signed-off-by: persmule <persmule(a)hardenedlinux.org>
---
M erasure_layout.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/14/84614/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/84614?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: I9e10749186e395da67ec80e296119f33c3f83122
Gerrit-Change-Number: 84614
Gerrit-PatchSet: 2
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>
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/+/84614?usp=email )
Change subject: erasure_layout: Erase larger block only when all sub-block need erase
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> I would call it "nested sub-blocks" instead of "double sub-blocks", what do you think?
Okay. I will change the commit message later.
> p.1 is a good question to discuss. My understanding, the idea was that if we have a region, and there are many small blocks inside that need erase, maybe it's faster to send one opcode once, and erase the whole region (instead of sending lots of opcodes for each small region).
My consideration is to reduce the wearout as much as possible like the old version of flashrom before erasure_layout.c was introduced, so a block needless to erase is never erased.
> And before the patch the threshold was 50% , you changed it to 100%.
I am thinking that maybe the optimal is in between?
I will never consider to exchange programming speed with chip lifetime 😞, so we may leave it for the end user to decide: We may add a (command line) option to specify "extra erase ratio"--the percentage of overall sub-blocks needless to erase within a larger erase block to be sacrificed for programming speed, default 0, max 100 (meaning whole chip erase 😜).
--
To view, visit https://review.coreboot.org/c/flashrom/+/84614?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: I9e10749186e395da67ec80e296119f33c3f83122
Gerrit-Change-Number: 84614
Gerrit-PatchSet: 1
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: Mon, 07 Oct 2024 13:20:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Anastasia Klimchuk.
Bill XIE has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84567?usp=email )
Change subject: ichspi: Change the opcode position for reprogramming on the fly 2->4
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> Needs to be tested
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/84567?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: I6bc855daedf0af2e8de191f23a3512de3ebc3fef
Gerrit-Change-Number: 84567
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 07 Oct 2024 12:42:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Anastasia Klimchuk.
Bill XIE has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84567?usp=email )
Change subject: ichspi: Change the opcode position for reprogramming on the fly 2->4
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS2:
> Oh you put my patch first in the chain, this is so nice thank you! :) […]
Yes. From my point of view, the whole chain works fine.
--
To view, visit https://review.coreboot.org/c/flashrom/+/84567?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: I6bc855daedf0af2e8de191f23a3512de3ebc3fef
Gerrit-Change-Number: 84567
Gerrit-PatchSet: 2
Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 07 Oct 2024 12:42:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Attention is currently required from: Nikolai Artemiev, Peter Marheine.
Bill XIE has posted comments on this change by Bill XIE. ( https://review.coreboot.org/c/flashrom/+/84253?usp=email )
Change subject: ichspi: Probe opcode in POSSIBLE_OPCODES[] as well
......................................................................
Patch Set 5:
(1 comment)
File ichspi.c:
https://review.coreboot.org/c/flashrom/+/84253/comment/d20e0c78_379f8b03?us… :
PS1, Line 1843:
: static bool ich_spi_probe_opcode(const struct flashctx *flash, uint8_t opcode)
: {
: return find_opcode(curopcodes, opcode) >= 0;
> > Please show me your elegant solution to program other erasing opcodes on the fly, but not before C […]
Yes. From my point of view, the whole chain works fine.
--
To view, visit https://review.coreboot.org/c/flashrom/+/84253?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: I3fc831fc072e2af9265835cb2f71bf8c222c6a64
Gerrit-Change-Number: 84253
Gerrit-PatchSet: 5
Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Comment-Date: Mon, 07 Oct 2024 12:42:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Bill XIE <persmule(a)hardenedlinux.org>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.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/+/84614?usp=email )
Change subject: erasure_layout: Erase larger block only when all sub-block need erase
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> Bill, thank you so much for looking into the details of the erasure logic! And the patch with the te […]
I made a patch for p.2 CB:84686
--
To view, visit https://review.coreboot.org/c/flashrom/+/84614?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: I9e10749186e395da67ec80e296119f33c3f83122
Gerrit-Change-Number: 84614
Gerrit-PatchSet: 1
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: Mon, 07 Oct 2024 07:48:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>