Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 )
Change subject: libflashrom,writeprotect: add functions for reading/writing WP settings
......................................................................
Patch Set 25:
(4 comments)
File libflashrom.h:
https://review.coreboot.org/c/flashrom/+/58479/comment/d472e6ce_9db44993
PS22, Line 129: };
> Please move to the commit that makes use of it.
Done
https://review.coreboot.org/c/flashrom/+/58479/comment/8c828b1e_eea17327
PS22, Line 131: const struct flashrom_flashctx *, struct flashrom_wp_chip_config *
> Something about APIs in general. We are used to have the destination […]
Done - here and in other files/commits. I think I got all occurrences, let me know if I missed something :)
File writeprotect.c:
https://review.coreboot.org/c/flashrom/+/58479/comment/fa897570_4f3894a9
PS22, Line 48: **
> Library functions should be in a respective documentation group, e.g. @defgroup or @addtogroup.
Done
https://review.coreboot.org/c/flashrom/+/58479/comment/abdb9bf5_109adc23
PS22, Line 122: INVALID_REG
> IMO it is a bit confusing when loop starts with "invalid" value. […]
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/58479
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad
Gerrit-Change-Number: 58479
Gerrit-PatchSet: 25
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 11 Jan 2022 06:26:21 +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>
Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Angel Pons, Sergii Dmytruk.
Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58478 )
Change subject: writeprotect.h: add structure to represent chip wp configuration bits
......................................................................
Patch Set 24:
(4 comments)
File flash.h:
https://review.coreboot.org/c/flashrom/+/58478/comment/36674104_0ab70722
PS22, Line 201: Complete
> `Complete` seems a bit exaggerated. Given the many different […]
The intention is that it will be kept complete, i.e. bits will be added as necessary to support new chips. The count/present fields allow unused bits to be ignored for chips that don't need them.
You're right that we'll need to add WPS for some chips.
https://review.coreboot.org/c/flashrom/+/58478/comment/e01aaec4_46ce0bd9
PS22, Line 202: chip
> Isn't everything about chips? Could we just call it `flashrom_wp_config`?
I've changed it to `wp_bits`. The name is probably less important now that it is just used internally by writeprotect code, but I'm happy to change it to something else.
https://review.coreboot.org/c/flashrom/+/58478/comment/f9e6439a_d1e232e0
PS22, Line 203: srp
> Please add comments what all these acronyms mean.
Done
File flash.h:
https://review.coreboot.org/c/flashrom/+/58478/comment/ee60f3a0_39f9647b
PS21, Line 201: /* Complete description of a chip's write protection configuration */
: struct flashrom_wp_chip_config {
: size_t srp_bit_count;
: uint8_t srp[MAX_SRP_BITS];
:
: bool cmp_bit_present;
: uint8_t cmp;
:
: bool sec_bit_present;
: uint8_t sec;
:
: bool tb_bit_present;
: uint8_t tb;
:
: size_t bp_bit_count;
: uint8_t bp[MAX_BP_BITS];
: };
> Well, not by nature, probably by coincidence. I don't see why one couldn't […]
I've though about this more and I agree that it would be best to keep it private to writeprotect/wp_ranges code. I will move it to writeprotect.h.
--
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: 24
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: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-Comment-Date: Tue, 11 Jan 2022 06:25:58 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Edward O'Callaghan, Nikolai Artemiev, 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/+/58479
to look at the new patch set (#25).
Change subject: libflashrom,writeprotect: add functions for reading/writing WP settings
......................................................................
libflashrom,writeprotect: add functions for reading/writing WP settings
Two new functions are exposed through the libflashrom API:
`flashrom_wp_{read,write}_settings()`. The input/output of these
functions is a `struct flashrom wp_settings` instance, which includes
the flash protection range and status register protection mode.
This commit also adds `{read,write}_wp_bits()` helper functions that
read/write chip-specific WP configuration bits.
BUG=b:195381327,b:153800563
TEST=flashrom --wp-{enable,disable,range,list,status} at end of patch series
BRANCH=none
Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M libflashrom.h
M writeprotect.c
2 files changed, 229 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/58479/25
--
To view, visit https://review.coreboot.org/c/flashrom/+/58479
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3ad25708c3321b8fb0216c3eaf6ffc07616537ad
Gerrit-Change-Number: 58479
Gerrit-PatchSet: 25
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: Felix Singer <felixsinger(a)posteo.net>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Nikolai Artemiev.
Hello build bot (Jenkins), Nico Huber, Anastasia Klimchuk,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/59909
to look at the new patch set (#8).
Change subject: writeprotect: print chip range truth table for debugging
......................................................................
writeprotect: print chip range truth table for debugging
Make it easier to add new chips and debug range decoding functions by
printing a complete truth table of config bits / ranges.
BUG=b:195381327,b:153800563
TEST=flashrom --wp-list -V
BRANCH=none
Change-Id: I2cda441229ffdcabff27906f7804efba82baa6bc
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
---
M writeprotect.c
1 file changed, 15 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/09/59909/8
--
To view, visit https://review.coreboot.org/c/flashrom/+/59909
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I2cda441229ffdcabff27906f7804efba82baa6bc
Gerrit-Change-Number: 59909
Gerrit-PatchSet: 8
Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset