Attention is currently required from: Nico Huber, Sergii Dmytruk.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59408 )
Change subject: flashrom.8.tmpl: remove outdated warning about v1.0
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
Patchset:
PS2:
I agree this is out-of-date. Can you please rebase this change atop master? This way we can quickly get this in without having to wait for the OTP patches to land.
--
To view, visit https://review.coreboot.org/c/flashrom/+/59408
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id7e0ce412901ccb27124a9958d5ef214ab289518
Gerrit-Change-Number: 59408
Gerrit-PatchSet: 2
Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 13:07:49 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 )
Change subject: cli_classic: add writeprotect CLI
......................................................................
Patch Set 10:
(4 comments)
File cli_classic.c:
https://review.coreboot.org/c/flashrom/+/58738/comment/86e8eda4_00fcd4d4
PS7, Line 63: mode
> What <mode> is? I think this needs explanation, if someone runs `flashrom -h` they need to be able t […]
Since permanent and powercycle protection are very rarely supported and not very useful, I think we should just drop the mode parameter for now.
If anyone knows of a case where it would be useful let me know. I've dropped support for it in patchset #10 but I can bring it back from #9.
https://review.coreboot.org/c/flashrom/+/58738/comment/3c3d224d_666f98d1
PS7, Line 222: if (!any_wp_op) return 0;
> any_wp_op seems like a duplication of information, you could check if any of commands is present?
Done
https://review.coreboot.org/c/flashrom/+/58738/comment/0980a71b_c565891a
PS7, Line 257: return ret;
> Please move conditional body on the new line (and same for all other occurrences like this).
Done
https://review.coreboot.org/c/flashrom/+/58738/comment/768ecf61_1c4c0a7d
PS7, Line 920: goto out_release;
> and the ones like this too, please move on a new line
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/58738
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I499f521781ee8999921996517802c0c0c641d869
Gerrit-Change-Number: 58738
Gerrit-PatchSet: 10
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 10:37:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58484 )
Change subject: libflashrom,writeprotect: add flashrom_wp_{config,range,mode}_to_str()
......................................................................
Patch Set 13:
(2 comments)
File libflashrom.h:
https://review.coreboot.org/c/flashrom/+/58484/comment/4cfb3c23_0fe37301
PS11, Line 138: #define FLASHROM_WP_OUT_STR_SIZE 1024
> Why 1024? Same question for 128 above. […]
Done
File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/58484/comment/db2121d3_a412756f
PS11, Line 436: for (int i = cfg->srp_bit_count - 1; i >= 0; i--) {
: buf += sprintf(buf, "SRP%d=%u ", i, cfg->srp[i]);
: }
:
: if (cfg->cmp_bit_present)
: buf += sprintf(buf, "CMP=%u ", cfg->cmp);
:
: if (cfg->sec_bit_present)
: buf += sprintf(buf, "SEC=%u ", cfg->sec);
:
: if (cfg->tb_bit_present)
: buf += sprintf(buf, "TB=%u ", cfg->tb);
:
: for (int i = cfg->bp_bit_count - 1; i >= 0; i--) {
: buf += sprintf(buf, "BP%d=%u ", i, cfg->bp[i]);
: }
:
> I would prefer the same ordering as in wp_chip_struct defined in CB:58478
This ordering is actually better and more consistent with the way that configs are ordered by the get_available_ranges function. I've changed the ordering in the struct to match.
--
To view, visit https://review.coreboot.org/c/flashrom/+/58484
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I82441c60c239a5b947b5a2a87b61e7bdf60bee89
Gerrit-Change-Number: 58484
Gerrit-PatchSet: 13
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 10:30:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/58481
to look at the new patch set (#12).
Change subject: libflashrom,writeprotect: add flashrom_wp_get_available_ranges()
......................................................................
libflashrom,writeprotect: add flashrom_wp_get_available_ranges()
Generate list of available ranges by enumerating all possible values
that range bits (BPx, TB, ...) can take and using the chip's range
decoding function to get the range associated with each one.
BUG=b:195381327,b:153800563
TEST=flashrom --wp-list
BRANCH=none
Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M libflashrom.h
M writeprotect.c
2 files changed, 143 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/81/58481/12
--
To view, visit https://review.coreboot.org/c/flashrom/+/58481
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d
Gerrit-Change-Number: 58481
Gerrit-PatchSet: 12
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.
Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/58478
to look at the new patch set (#11).
Change subject: flash: add structure to represent chip wp configuration
......................................................................
flash: add structure to represent chip wp configuration
Add `struct flashrom_wp_chip_config` for representing values of all WP
bits in a chip's status register(s).
It allows most WP code to store and manipulate a chip's configuration
without knowing the exact layout of bits in the chip's status registers.
Supporting other chips may require additional fields to be added to the
structure.
BUG=b:195381327,b:153800563
TEST=flashrom --wp-{enable,disable,range,list,status} at end of patch series
BRANCH=none
Change-Id: I17dee630248ce7b51e624a6e46d7097d5d0de809
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M flash.h
1 file changed, 18 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/58478/11
--
To view, visit https://review.coreboot.org/c/flashrom/+/58478
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I17dee630248ce7b51e624a6e46d7097d5d0de809
Gerrit-Change-Number: 58478
Gerrit-PatchSet: 11
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58481 )
Change subject: libflashrom,writeprotect: add flashrom_wp_get_available_ranges()
......................................................................
Patch Set 11:
(3 comments)
File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/58481/comment/dee3641b_00feabce
PS10, Line 271: ranges[++last] = ranges[i];
> Thank you! […]
Done
https://review.coreboot.org/c/flashrom/+/58481/comment/fec2ffd6_8b09854f
PS10, Line 271: ranges[++last] = ranges[i];
> Thank you! […]
I've rewritten the loop to avoid edge cases and be easier to read.
Probably out of scope for now, but maybe should we have a separate file for collecting common algorithms like this (sort, binary search, deduplicate, etc) instead of repeating them in various places?
https://review.coreboot.org/c/flashrom/+/58481/comment/8721f486_2d9a7be3
PS10, Line 274: *count = last + 1;
> If `*count` was 0 initially, i.e. an empty list, this would set it to `1`.
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/58481
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id51f038f03305c8536d80313e52f77d27835f34d
Gerrit-Change-Number: 58481
Gerrit-PatchSet: 11
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 10:20:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59183 )
Change subject: libflashrom,writeprotect: add flashrom_wp_get_range()
......................................................................
Patch Set 3:
(2 comments)
File libflashrom.h:
https://review.coreboot.org/c/flashrom/+/59183/comment/34ca5562_13aa21b4
PS2, Line 141: int flashrom_wp_get_range(const struct flashrom_wp_chip_config *cfg, struct flashrom_wp_range *range);
> You can omit param names here, and the same for the rest of the chain (since the rest of the chain i […]
Done
File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/59183/comment/8cee4361_ab7fa8cd
PS2, Line 182: cfg->chip
> Unless people convince me otherwise, I think chip can be an argument to flashrom_wp_get_range , and […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/59183
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I5a1dfcf384166b1bac319d286306747e1dcaa000
Gerrit-Change-Number: 59183
Gerrit-PatchSet: 3
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Fri, 19 Nov 2021 10:20:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment