- flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: tests: Add tests for verify operation
by Edward O'Callaghan (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59239 ) Change subject: tests: Add tests for verify operation ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/59239 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1cc6f73f9b1e385eb963adccf20759c13a40ed3b Gerrit-Change-Number: 59239 Gerrit-PatchSet: 8 Gerrit-Owner: 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 18 Feb 2022 03:39:36 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom/fmap: Don't use off_t for flash offsets
by Julius Werner (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Nico Huber, David Hendricks, Angel Pons, Arthur Heymans. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/61943 ) Change subject: libflashrom/fmap: Don't use off_t for flash offsets ...................................................................... Patch Set 2: (2 comments) File fmap.c: https://review.coreboot.org/c/flashrom/+/61943/comment/188c5e0b_f73f1933 PS1, Line 94: int32_t > Thoughts on using `ssize_t` instead? Done File libflashrom.h: https://review.coreboot.org/c/flashrom/+/61943/comment/8615cd94_4e4dc337 PS1, Line 112: uint32_t > +1 Done -- To view, visit https://review.coreboot.org/c/flashrom/+/61943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee Gerrit-Change-Number: 61943 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 18 Feb 2022 01:39:59 +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

1 0

Change in flashrom[master]: libflashrom/fmap: Don't use off_t for flash offsets
by Julius Werner (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Nico Huber, David Hendricks, Julius Werner, Arthur Heymans. Hello build bot (Jenkins), Nico Huber, David Hendricks, Edward O'Callaghan, Angel Pons, Arthur Heymans, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/61943 to look at the new patch set (#2). Change subject: libflashrom/fmap: Don't use off_t for flash offsets ...................................................................... libflashrom/fmap: Don't use off_t for flash offsets off_t is a special POSIX type that is used to represent file offsets in certain APIs (e.g. lseek(), mmap()), and should not be reused to represent anything else (such as flash offsets). In particular, the width of the type may change based on the definition of the _FILE_OFFSET_BITS macro. Using such a type at the libflashrom interface is particularly dangerous, because if a program is built with a different _FILE_OFFSET_BITS value than libflashrom, the resulting ABI corruption will cause very very nasty and confusing bugs. This patch replaces all instances of off_t that are not related to file offsets with (s)size_t. Signed-off-by: Julius Werner <jwerner(a)chromium.org> Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee --- M fmap.c M libflashrom.c M libflashrom.h 3 files changed, 6 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/43/61943/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/61943 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I68a386973f79ea634f63dfcd7d95a63400e1fdee Gerrit-Change-Number: 61943 Gerrit-PatchSet: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> 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: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: flashchips,writeprotect_ranges: add range decoding function
by Nico Huber (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58480 ) Change subject: flashchips,writeprotect_ranges: add range decoding function ...................................................................... Patch Set 35: Code-Review+1 (3 comments) File flash.h: https://review.coreboot.org/c/flashrom/+/58480/comment/64ae2632_f4e8bb88 PS35, Line 311: size_t *start, size_t *len Should we put output parameters first? File writeprotect_ranges.c: https://review.coreboot.org/c/flashrom/+/58480/comment/85f64148_89ad2fa1 PS35, Line 8: * the Free Software Foundation; version 2 of the License. Can we have v2+? https://review.coreboot.org/c/flashrom/+/58480/comment/4451d3b8_61283d57 PS35, Line 23: * various chips is more fully known. I guess this would be something that's never 100% finished. Maybe not worth a TODO. -- To view, visit https://review.coreboot.org/c/flashrom/+/58480 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id163ed80938a946a502ed116e48e8236e36eb203 Gerrit-Change-Number: 58480 Gerrit-PatchSet: 35 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 17 Feb 2022 18:12:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom,writeprotect: add functions for reading/writing WP configs
by Nico Huber (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58479 ) Change subject: libflashrom,writeprotect: add functions for reading/writing WP configs ...................................................................... Patch Set 34: (7 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/58479/comment/3599d703_9f7602c9 PS34, Line 11: a spurious `a` File libflashrom.h: https://review.coreboot.org/c/flashrom/+/58479/comment/0a94607f_f13bb936 PS34, Line 125: FLASHROM_WP_OK = 0, NB. we should unify this across the whole API sooner or later. https://review.coreboot.org/c/flashrom/+/58479/comment/24decda6_5743a364 PS34, Line 151: excessive empty line File libflashrom.c: https://review.coreboot.org/c/flashrom/+/58479/comment/5674ad61_45e26c0b PS34, Line 642: struct flashrom_wp_cfg Or `sizeof(**cfg)` https://review.coreboot.org/c/flashrom/+/58479/comment/8d3d4189_1ba933ec PS34, Line 643: (*cfg) Nit, no parentheses needed. https://review.coreboot.org/c/flashrom/+/58479/comment/9444f0b3_905b5e56 PS34, Line 717: * has its own WP operations we should use instead. Shouldn't we check the type right away and return an error in case? Or check inside wp_write_cfg()? Currently it relies on `flash->chip-> bus_type == BUS_SPI` and matching contents of `flash->chip->reg_bits`. File writeprotect.c: https://review.coreboot.org/c/flashrom/+/58479/comment/9b42faee_09a3fa74 PS34, Line 51: = FLASHROM_WP_OK This is overwritten on the very next line. -- 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: 34 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: 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-Comment-Date: Thu, 17 Feb 2022 18:12:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: writeprotect.h: add structure to represent chip wp configuration bits
by Nico Huber (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Nico Huber 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 30: Code-Review+1 (1 comment) File writeprotect.h: https://review.coreboot.org/c/flashrom/+/58478/comment/7fdb8f49_b727ccf4 PS30, Line 39: /* Complement bit (CMP) */ : bool cmp_bit_present; : uint8_t cmp; : : /* Sector/block protection bit (SEC) */ : bool sec_bit_present; : uint8_t sec; : : /* Top/bottom protection bit (TB) */ : bool tb_bit_present; : uint8_t tb; Something I didn't notice earlier: The `_present` flags are only consumed when interpreting the range. And there, a `_present == false` is always treated the same as if the bit itself is 0. So, AFAICS, the code would still do the same if we'd drop the flags (if not present bits are 0 initialzed). I guess that's a matter of taste. Leaner code vs. more explicit code. -- 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: 30 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 17 Feb 2022 18:11:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flash.h,flashchips.c: add writeprotect bit layout map to chips
by Nico Huber (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58477 ) Change subject: flash.h,flashchips.c: add writeprotect bit layout map to chips ...................................................................... Patch Set 29: Code-Review+1 (1 comment) File flash.h: https://review.coreboot.org/c/flashrom/+/58477/comment/b70ac19c_9be71955 PS29, Line 188: guaranteethe Missing space. -- To view, visit https://review.coreboot.org/c/flashrom/+/58477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id08d77e6d4ca5109c0d698271146d026dbc21284 Gerrit-Change-Number: 58477 Gerrit-PatchSet: 29 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 17 Feb 2022 17:02:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: layout: Hoist get_region_range() into libflashrom API
by Nico Huber (Code Review) 18 Feb '22

18 Feb '22

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60881 ) Change subject: layout: Hoist get_region_range() into libflashrom API ...................................................................... Patch Set 4: (1 comment) Patchset: PS4: What's the use case? If we need the info to feed it back to libflashrom, another API might be lacking. -- To view, visit https://review.coreboot.org/c/flashrom/+/60881 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8cf95b5eaec943a51d0ea668f26a56bf6d6b4446 Gerrit-Change-Number: 60881 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Thu, 17 Feb 2022 16:49:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi25_statusreg,flashchips: add SR2 read/write support
by Nico Huber (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58570 ) Change subject: spi25_statusreg,flashchips: add SR2 read/write support ...................................................................... Patch Set 25: Code-Review+1 (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/58570/comment/0a92f87a_4c0eab9c PS25, Line 6710: /* Supports both WRSR2 and extended WRSR */ Comment only states the same as the code. -- To view, visit https://review.coreboot.org/c/flashrom/+/58570 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I34a503b0958e8f2f22a2a993a6ea529eb46b41db Gerrit-Change-Number: 58570 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 17 Feb 2022 16:46:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi25_statusreg: make register read/write functions generic
by Nico Huber (Code Review) 18 Feb '22

18 Feb '22

Attention is currently required from: Angel Pons, Nikolai Artemiev, Sergii Dmytruk. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58475 ) Change subject: spi25_statusreg: make register read/write functions generic ...................................................................... Patch Set 22: Code-Review+1 (1 comment) Patchset: PS22: Would be nice to test this intensively, not wrt. write protection but to avoid regressions. -- To view, visit https://review.coreboot.org/c/flashrom/+/58475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a3951bbf993f2d8d830143b29d3ce16cc6901d7 Gerrit-Change-Number: 58475 Gerrit-PatchSet: 22 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Thu, 17 Feb 2022 16:37:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit