- flashrom-gerrit - mail.coreboot.org

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Matti Finder (Code Review) 07 Nov '24

07 Nov '24

Attention is currently required from: Matti Finder, Peter Marheine. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84934?usp=email to look at the new patch set (#9). The following approvals got outdated and were removed: Code-Review+2 by Peter Marheine, Verified+1 by build bot (Jenkins) Change subject: rpmc: add rpmc commands feature ...................................................................... rpmc: add rpmc commands feature Added optional support for all the commands specified in JESD260. Added a new optional dependency to openssls libcrypto. Added parsing for the rpmc parameter sfdp table. Added necessary rpmc parameter information to flashchips struct and the flash hardening feature to enable rpmc commands. Enables future use of these commands in the cli_client and also libflashrom. Change-Id: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Signed-off-by: Matti Finder <matti.finder(a)gmail.com> --- M MAINTAINERS M include/flash.h A include/rpmc.h M meson.build M meson_options.txt A rpmc.c M sfdp.c 7 files changed, 819 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/84934/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 9 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Matti Finder <matti.finder(a)gmail.com>

1 0

[L] Change in flashrom[main]: cli_client: Add rpmc command support
by Anastasia Klimchuk (Code Review) 07 Nov '24

07 Nov '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_client: Add rpmc command support ...................................................................... Patch Set 12: (2 comments) Patchset: PS10: > 2) I think that change still fits quite well to into this commit. I have started the change. […] I think it's totally fine that you added the info for the distros you use, as a first version. Docs can be updated anytime, and better we add the instructions that we know are true. I will mark this as resolved. Patchset: PS12: I have tried to download the patch (to check how docs are generated) and got a merge conflict, maybe you can rebase on latest head? Because otherwise it's all good, and you added all the docs! -- 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: 12 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: Thu, 07 Nov 2024 04:54:32 +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]: rpmc: add rpmc commands feature
by Anastasia Klimchuk (Code Review) 07 Nov '24

07 Nov '24

Attention is currently required from: Matti Finder. Anastasia Klimchuk has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email ) Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 8: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/84934/comment/0c5d3832_054556ea?us… : PS8, Line 177: get_option('rpmc').allowed() > I found the trace where current minimum 0.56.0 comes from: […] Matti, I feel bad that I discovered something again, last moment. I could try building this few days ago! :\ To help, I just tested the code that I suggested in my previous comment, so you can use it. I tested various scenarios: 1) libcrypto found, rpmc=auto or enabled => flashrom builds with rpmc feature 2) libcrypto found, rpmc=disabled => flashrom builds without rpmc feature 3) libcrypto not found, rpmc=auto or disabled => flashrom builds without rpmc feature 4) libcrypto not found, rpmc=enabled => flashrom build fails (ERROR: Dependency lookup for libcrypto with method 'pkgconfig' failed) That should cover everything. (the way to try is to run in the builddir `meson configure -Drpmc=enabled` and then rebuild) -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 8 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-Comment-Date: Thu, 07 Nov 2024 04:37:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Anastasia Klimchuk (Code Review) 07 Nov '24

07 Nov '24

Attention is currently required from: Matti Finder. Anastasia Klimchuk has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email ) Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 8: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/84934/comment/b8964c4c_c7be3d93?us… : PS8, Line 177: get_option('rpmc').allowed() > So, I downloaded the patch locally, to try and build it, and discover something. […] I found the trace where current minimum 0.56.0 comes from: In this patch https://review.coreboot.org/c/flashrom/+/73361 there is a link (in the first comment) that Debian Bullseye ships 0.56.2 https://packages.debian.org/bullseye/meson And maybe we can talk about upgrading the minimum, but I definitely don't want to hold this patch for that. I would rather change it too use the "old-style" construction (get_option('rpmc').auto() or get_option('rpmc').enabled()) and get this merged. If later we upgrade the meson version, we will upgrade to isAllowed() everywhere. So, I suggest to change the lines 177-181 to ``` if (get_option('rpmc').auto() or get_option('rpmc').enabled()) and libcrypto.found() add_project_arguments('-DCONFIG_RPMC_ENABLED=1', language : 'c') srcs += 'rpmc.c' deps += libcrypto endif ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 8 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-Comment-Date: Thu, 07 Nov 2024 04:28:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Anastasia Klimchuk (Code Review) 07 Nov '24

07 Nov '24

Attention is currently required from: Matti Finder. Anastasia Klimchuk has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 8: (2 comments) File meson.build: https://review.coreboot.org/c/flashrom/+/84934/comment/17766e77_c4596e19?us… : PS8, Line 177: get_option('rpmc').allowed() So, I downloaded the patch locally, to try and build it, and discover something. I got a warning: WARNING: Project specifies a minimum meson_version '>=0.56.0' but uses features which were added in newer versions: * 0.59.0: {'feature_option.allowed()'} I have locally meson of 1.4.1 so it all builds anyway, and yes it builds, I can see object file for rpmc, good! BUT we can't have minimum version of 0.56.0 and use a feature of 0.59.0. Maybe we can upgrade the minimum version, but this needs a careful look, and definitely not for this commit. For this commit I would just replace > get_option('rpmc').allowed() with > (get_option('rpmc').auto() or get_option('rpmc').enabled()) (can also be nested if, first level checks for libcrypto.found() and inside check for option) what do you think? File sfdp.c: https://review.coreboot.org/c/flashrom/+/84934/comment/3041c0fb_e7fedb4f?us… : PS6, Line 457: return 0; > I just realized that this doesn't catch the failure if we have no Jedec parameter page at at all. […] Looks good, thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 8 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-Comment-Date: Thu, 07 Nov 2024 02:32:21 +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

[M] Change in flashrom[main]: doc: Add few sections to recent development doc
by Peter Marheine (Code Review) 07 Nov '24

07 Nov '24

Attention is currently required from: Anastasia Klimchuk. Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84954?usp=email ) Change subject: doc: Add few sections to recent development doc ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84954?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: Iedaca4a704c57c1db399c7888f743ad2961cbf9d Gerrit-Change-Number: 84954 Gerrit-PatchSet: 2 Gerrit-Owner: 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: Thu, 07 Nov 2024 00:07:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: doc: Change link from gitiles to github
by Anastasia Klimchuk (Code Review) 06 Nov '24

06 Nov '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/84960?usp=email ) Change subject: doc: Change link from gitiles to github ...................................................................... doc: Change link from gitiles to github gitiles are not always available, so the link was not always working, which could make readers confused. While we are here, add missing link to Dev Guide. Change-Id: I9103e5199bdf1b65fa3136aa01259a85e788a251 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/84960 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M doc/about_flashrom/team.rst 1 file changed, 2 insertions(+), 2 deletions(-) Approvals: Peter Marheine: Looks good to me, approved build bot (Jenkins): Verified diff --git a/doc/about_flashrom/team.rst b/doc/about_flashrom/team.rst index 215d930..cbcef45 100644 --- a/doc/about_flashrom/team.rst +++ b/doc/about_flashrom/team.rst @@ -6,7 +6,7 @@ All contributors and users who have a Gerrit account can send patches, add comments to patches and vote +1..-1 on patches. -All contributors and users are expected to follow Development guidelines and +All contributors and users are expected to follow :doc:`/dev_guide/development_guide` and :doc:`code_of_conduct`. There are two special groups in Gerrit. @@ -18,7 +18,7 @@ can do full approval of patches (i.e. vote +2). In general, members of the group have some area of responsibility in the -`MAINTAINERS <https://review.coreboot.org/plugins/gitiles/flashrom/+/refs/heads/main/MAIN…>`_ file, +`MAINTAINERS <https://github.com/flashrom/flashrom/blob/main/MAINTAINERS>`_ file, and are automatically added as reviewers to patches when the patch touches this area. The responsibilities are the following. -- To view, visit https://review.coreboot.org/c/flashrom/+/84960?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: I9103e5199bdf1b65fa3136aa01259a85e788a251 Gerrit-Change-Number: 84960 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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>

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Peter Marheine (Code Review) 06 Nov '24

06 Nov '24

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/+/84934?usp=email ) Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 8 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: Wed, 06 Nov 2024 22:30:53 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[L] Change in flashrom[main]: rpmc: add rpmc commands feature
by Matti Finder (Code Review) 06 Nov '24

06 Nov '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Matti Finder has posted comments on this change by Matti Finder. ( https://review.coreboot.org/c/flashrom/+/84934?usp=email ) Change subject: rpmc: add rpmc commands feature ...................................................................... Patch Set 8: (1 comment) File sfdp.c: https://review.coreboot.org/c/flashrom/+/84934/comment/d5e4b4dd_8a3b2597?us… : PS6, Line 457: return 0; > Had to also add free(tbuf) so that we don't create a memory leak. I just realized that this doesn't catch the failure if we have no Jedec parameter page at at all. I went back to the way it was handeled originally. Since this page being first is mandatory by the standard we can just fail in case we can't find it at position 0. This also solves some other errors in the way the page is parsed, because it sets feature bits instead of or-ing them. This would invalidate pages parsed beforehand, if the set feature bits -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 8 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 06 Nov 2024 17:25:35 +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]: rpmc: add rpmc commands feature
by Matti Finder (Code Review) 06 Nov '24

06 Nov '24

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84934?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: rpmc: add rpmc commands feature ...................................................................... rpmc: add rpmc commands feature Added optional support for all the commands specified in JESD260. Added a new optional dependency to openssls libcrypto. Added parsing for the rpmc parameter sfdp table. Added necessary rpmc parameter information to flashchips struct and the flash hardening feature to enable rpmc commands. Enables future use of these commands in the cli_client and also libflashrom. Change-Id: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Signed-off-by: Matti Finder <matti.finder(a)gmail.com> --- M MAINTAINERS M include/flash.h A include/rpmc.h M meson.build M meson_options.txt A rpmc.c M sfdp.c 7 files changed, 819 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/84934/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/84934?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: I6ab3d0446e9fd674b20550fdbfaf499b8d4a9b38 Gerrit-Change-Number: 84934 Gerrit-PatchSet: 8 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: Peter Marheine <pmarheine(a)chromium.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit