October 2024 - flashrom-gerrit

[S] Change in flashrom[main]: erase/write: Deselect all smaller blocks when large block is selected
by Anastasia Klimchuk (Code Review) Oct. 8, 2024

Oct. 8, 2024

Attention is currently required from: Aarya, Bill XIE. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84686?usp=email ) Change subject: erase/write: Deselect all smaller blocks when large block is selected ...................................................................... Patch Set 2: (1 comment) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/84686/comment/746d8238_aedf7b2d?us… : PS1, Line 242: while (true) > I want to re-write this as recursive fn `deselect_erase_functions` Done -- 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: comment 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> Gerrit-Comment-Date: Tue, 08 Oct 2024 11:01:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: erase/write: Deselect all smaller blocks when large block is selected
by Anastasia Klimchuk (Code Review) Oct. 8, 2024

Oct. 8, 2024

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>

1 0

[M] Change in flashrom[main]: flashchips: add GD25F256F
by Victor Lim (Code Review) Oct. 7, 2024

Oct. 7, 2024

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>

1 0

[M] Change in flashrom[main]: flashchips: add GD25F256F
by Victor Lim (Code Review) Oct. 7, 2024

Oct. 7, 2024

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>

1 0

[XS] Change in flashrom[main]: erasure_layout: Erase larger block only when all sub-block need erase
by Bill XIE (Code Review) Oct. 7, 2024

Oct. 7, 2024

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>

1 0

[XS] Change in flashrom[main]: erasure_layout: Erase larger block only when all sub-block need erase
by Bill XIE (Code Review) Oct. 7, 2024

Oct. 7, 2024

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>

1 0

[XS] Change in flashrom[main]: erasure_layout: Erase larger block only when all sub-block need erase
by Bill XIE (Code Review) Oct. 7, 2024

Oct. 7, 2024

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>

1 0

[XS] Change in flashrom[main]: ichspi: Change the opcode position for reprogramming on the fly 2->4
by Bill XIE (Code Review) Oct. 7, 2024

Oct. 7, 2024

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>

1 0

[XS] Change in flashrom[main]: ichspi: Change the opcode position for reprogramming on the fly 2->4
by Bill XIE (Code Review) Oct. 7, 2024

Oct. 7, 2024

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>

1 0

[XS] Change in flashrom[main]: ichspi: Probe opcode in POSSIBLE_OPCODES[] as well
by Bill XIE (Code Review) Oct. 7, 2024

Oct. 7, 2024

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>

1 0