October 2024 - flashrom-gerrit

[XS] Change in flashrom[main]: linux_mtd: fix build with clang >= 19
by Anastasia Klimchuk (Code Review) 29 Oct '24

29 Oct '24

Attention is currently required from: Arnaud Ferraris, Nikolai Artemiev. Anastasia Klimchuk has posted comments on this change by Arnaud Ferraris. ( https://review.coreboot.org/c/flashrom/+/84856?usp=email ) Change subject: linux_mtd: fix build with clang >= 19 ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84856?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: If470a65702e9ae08e4303123a0014e53a1fee56e Gerrit-Change-Number: 84856 Gerrit-PatchSet: 3 Gerrit-Owner: Arnaud Ferraris <arnaud.ferraris(a)collabora.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arnaud Ferraris <arnaud.ferraris(a)collabora.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 29 Oct 2024 23:00:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[L] Change in flashrom[main]: cli_classic: Add rpmc command support
by Anastasia Klimchuk (Code Review) 29 Oct '24

29 Oct '24

Attention is currently required from: Matti Finder, Peter Marheine. Anastasia Klimchuk 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 4: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/84840/comment/2ae7e1e9_564bccff?us… : PS4, Line 1323: if (options.rpmc_read_data) : ret = rpmc_read_data(fill_flash); > Making it API-friendly sounds good, but I would hold off on adding this support to the public API si […] I think having code in a separate file rpmc.c is better, you don't need to move it. Keep rpmc.c and include/rpmc.h as separate files. When it comes to libflashrom, you will call functions from rpmc.c in the libflashrom.c. As existing example, write-protect functionality designed like this. You can look at the code, in libflashrom.c it has prefix `flashrom_wp` -- 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: 4 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: Matti Finder <matti.finder(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 29 Oct 2024 23:00:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Comment-In-Reply-To: Matti Finder <matti.finder(a)gmail.com>

1 0

[L] Change in flashrom[main]: cli_classic: Add rpmc command support
by Peter Marheine (Code Review) 29 Oct '24

29 Oct '24

Attention is currently required from: Anastasia Klimchuk. 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 4: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/84840/comment/fdf5ab80_6c77bc8b?us… : PS4, Line 1323: if (options.rpmc_read_data) : ret = rpmc_read_data(fill_flash); > I agree I think the stuff from rpmc.c should rather be in libflashrom. […] Making it API-friendly sounds good, but I would hold off on adding this support to the public API since it would be difficult to make backward-compatible changes if we find issues. -- 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: 4 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-Comment-Date: Tue, 29 Oct 2024 22:39:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Matti Finder <matti.finder(a)gmail.com>

1 0

[L] Change in flashrom[main]: cli_classic: Add rpmc command support
by Matti Finder (Code Review) 29 Oct '24

29 Oct '24

Attention is currently required from: Anastasia Klimchuk. Matti Finder 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 4: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/84840/comment/8ba2cb37_837095b6?us… : PS4, Line 1323: if (options.rpmc_read_data) : ret = rpmc_read_data(fill_flash); > I am thinking, and maybe this is more the question to Peter, what do you think, […] I agree I think the stuff from rpmc.c should rather be in libflashrom. I'm gonna move my code over there, and make it more api friendly. And then seperate the commits. -- 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: 4 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-Comment-Date: Tue, 29 Oct 2024 17:05:38 +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]: linux_mtd: fix build with clang >= 19
by Maximilian Brune (Code Review) 29 Oct '24

29 Oct '24

Attention is currently required from: Anastasia Klimchuk, Arnaud Ferraris, Nikolai Artemiev. Maximilian Brune has posted comments on this change by Arnaud Ferraris. ( https://review.coreboot.org/c/flashrom/+/84856?usp=email ) Change subject: linux_mtd: fix build with clang >= 19 ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/84856?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: If470a65702e9ae08e4303123a0014e53a1fee56e Gerrit-Change-Number: 84856 Gerrit-PatchSet: 3 Gerrit-Owner: Arnaud Ferraris <arnaud.ferraris(a)collabora.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arnaud Ferraris <arnaud.ferraris(a)collabora.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 29 Oct 2024 15:21:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: linux_mtd: fix build with clang >= 19
by Arnaud Ferraris (Code Review) 29 Oct '24

29 Oct '24

Attention is currently required from: Anastasia Klimchuk, Maximilian Brune, Nikolai Artemiev. Arnaud Ferraris has posted comments on this change by Arnaud Ferraris. ( https://review.coreboot.org/c/flashrom/+/84856?usp=email ) Change subject: linux_mtd: fix build with clang >= 19 ...................................................................... Patch Set 3: (1 comment) File linux_mtd.c: https://review.coreboot.org/c/flashrom/+/84856/comment/51516784_3a557db1?us… : PS2, Line 52: char path[sizeof(LINUX_MTD_SYSFS_ROOT) + 32]; > Oh I got worried that I missed something. […] That's correct, `\0` is also counted by `sizeof`. Amended to keep the same size as previously. -- To view, visit https://review.coreboot.org/c/flashrom/+/84856?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: If470a65702e9ae08e4303123a0014e53a1fee56e Gerrit-Change-Number: 84856 Gerrit-PatchSet: 3 Gerrit-Owner: Arnaud Ferraris <arnaud.ferraris(a)collabora.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 29 Oct 2024 08:10:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Maximilian Brune <maximilian.brune(a)9elements.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[XS] Change in flashrom[main]: linux_mtd: fix build with clang >= 19
by Arnaud Ferraris (Code Review) 29 Oct '24

29 Oct '24

Attention is currently required from: Anastasia Klimchuk, Arnaud Ferraris, Maximilian Brune, Nikolai Artemiev. Hello Anastasia Klimchuk, Nikolai Artemiev, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84856?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Code-Review+2 by Anastasia Klimchuk, Code-Review+2 by Nikolai Artemiev, Verified+1 by build bot (Jenkins) The change is no longer submittable: Code-Review and Verified are unsatisfied now. Change subject: linux_mtd: fix build with clang >= 19 ...................................................................... linux_mtd: fix build with clang >= 19 Starting with version 19, clang issues a warning when using `strlen()` for initializing a static array's size. This causes the build to fail as the project also sets `-Werror`. This is fixed by using `sizeof()` instead, which is guaranteed to be evaluated at compilation time and therefore not triggering the problematic warning. Change-Id: If470a65702e9ae08e4303123a0014e53a1fee56e Signed-off-by: Arnaud Ferraris <arnaud.ferraris(a)collabora.com> --- M linux_mtd.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/56/84856/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/84856?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: If470a65702e9ae08e4303123a0014e53a1fee56e Gerrit-Change-Number: 84856 Gerrit-PatchSet: 3 Gerrit-Owner: Arnaud Ferraris <arnaud.ferraris(a)collabora.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Arnaud Ferraris <arnaud.ferraris(a)collabora.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: erasure_layout: Add an option to sacrifice unchanged blocks for speed
by Peter Marheine (Code Review) 29 Oct '24

29 Oct '24

Attention is currently required from: Aarya, Anastasia Klimchuk, Bill XIE. Peter Marheine 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 11: Code-Review+2 -- 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: 11 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: Peter Marheine <pmarheine(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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 29 Oct 2024 08:03:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in flashrom[main]: erasure_layout: Add an option to sacrifice unchanged blocks for speed
by Anastasia Klimchuk (Code Review) 29 Oct '24

29 Oct '24

Attention is currently required from: Aarya, Bill XIE, Peter Marheine. 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 11: (2 comments) File doc/classic_cli_manpage.rst: https://review.coreboot.org/c/flashrom/+/84721/comment/3b927795_124d3b60?us… : PS10, Line 23: | [-V[V[V]]] [-o <logfile>] [--progress][--sacrifice-ratio <ratio>] > Add a space between flags. […] Done File doc/release_notes/devel.rst: https://review.coreboot.org/c/flashrom/+/84721/comment/9b4e0305_28deaaa5?us… : PS10, Line 57: 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. > Small rewording suggestions. […] 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: 11 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: Peter Marheine <pmarheine(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-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 29 Oct 2024 05:59:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[M] Change in flashrom[main]: erasure_layout: Add an option to sacrifice unchanged blocks for speed
by Anastasia Klimchuk (Code Review) 29 Oct '24

29 Oct '24

Attention is currently required from: Aarya, Anastasia Klimchuk, Bill XIE. Anastasia Klimchuk has uploaded a new patch set (#11) to the change originally created by Bill XIE. ( https://review.coreboot.org/c/flashrom/+/84721?usp=email ) 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 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(a)hardenedlinux.org Co-developed-by: Anastasia Klimchuk <aklm(a)flashrom.org> Signed-off-by: persmule <persmule(a)hardenedlinux.org> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.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(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/84721/11 -- 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: 11 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: Peter Marheine <pmarheine(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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2024