January 2023 - flashrom-gerrit

[L] Change in flashrom[master]: flashrom_tester: Rewrite IOOpts to support more operations
by Evan Benn (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Evan Benn. Hello build bot (Jenkins), Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71973 to look at the new patch set (#5). Change subject: flashrom_tester: Rewrite IOOpts to support more operations ...................................................................... flashrom_tester: Rewrite IOOpts to support more operations flashrom cli supports include regions for all of read write and verify, as well as omitting the read/write/verify file if an include region with file is specified. Use an enum to allow only one operation at a time. Unify the read and write region implementations. BUG=b:235916336 BRANCH=None TEST=None Change-Id: I1cb46bb1b26949fd9c19949c43708a8b652e00da Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/flashrom/src/cmd.rs M util/flashrom_tester/flashrom/src/flashromlib.rs M util/flashrom_tester/flashrom/src/lib.rs M util/flashrom_tester/src/tests.rs 4 files changed, 160 insertions(+), 123 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/73/71973/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/71973 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1cb46bb1b26949fd9c19949c43708a8b652e00da Gerrit-Change-Number: 71973 Gerrit-PatchSet: 5 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom_tester: Rename simplify lock_test hwwp_locks_swwp_test
by Evan Benn (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Evan Benn. Hello build bot (Jenkins), Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71972 to look at the new patch set (#2). Change subject: flashrom_tester: Rename simplify lock_test hwwp_locks_swwp_test ...................................................................... flashrom_tester: Rename simplify lock_test hwwp_locks_swwp_test Rename lock_test to hwwp_locks_swwp_test. BUG=b:235916336 BRANCH=None TEST=None Change-Id: I6080622755ff16d8fba7044b38f9e09db0c62f97 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/src/tests.rs 1 file changed, 17 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/72/71972/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71972 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6080622755ff16d8fba7044b38f9e09db0c62f97 Gerrit-Change-Number: 71972 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Change chip restore data type from uint8_t to void ptr
by Nikolai Artemiev (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk. Hello build bot (Jenkins), Edward O'Callaghan, Anastasia Klimchuk, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70349 to look at the new patch set (#9). Change subject: tree/: Change chip restore data type from uint8_t to void ptr ...................................................................... tree/: Change chip restore data type from uint8_t to void ptr Chip restore callbacks currently are used by - spi25_statusreg.c unlock functions to restore status register 1. - s25f.c to restore config register 3. Both of these cases only need to save a single uint8_t value to restore the original chip state, however storing a void pointer will allow more flexible chip restore behaviour. In particular, it will allow flashrom_wp_cfg objects to be saved and restored, enabling writeprotect-based unlocking. BUG=b:237485865,b:247421511 BRANCH=none TEST=Tested on grunt DUT (prog: sb600spi, flash: W25Q128.W): `flashrom --wp-range 0x0,0x1000000 \ flashrom --wp-status # Result: range=0x0,0x1000000 \ flashrom -w random.bin # Result: success \ flashrom -v random.bin # Result: success \ flashrom --wp-status # Result: range=0x0,0x1000000` Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c M include/flash.h M s25f.c M spi25_statusreg.c 4 files changed, 61 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/70349/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/70349 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Gerrit-Change-Number: 70349 Gerrit-PatchSet: 9 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> 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

1 0

[M] Change in flashrom[master]: flashrom_tester: Check WP range after setting it
by Peter Marheine (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Evan Benn. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71975 ) Change subject: flashrom_tester: Check WP range after setting it ...................................................................... Patch Set 6: (4 comments) File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/71975/comment/60ff7b06_243d9441 PS6, Line 207: Range<usize> Is it possible for multiple ranges to be protected at the same time? https://review.coreboot.org/c/flashrom/+/71975/comment/3cd73119_f57e6499 PS6, Line 227: \d `[[:xdigit:]]` https://review.coreboot.org/c/flashrom/+/71975/comment/c0033693_91f1604a PS6, Line 231: range_match.get(1).unwrap().as_str() You can use `range_match[1]` to be more concise. File util/flashrom_tester/flashrom/src/lib.rs: https://review.coreboot.org/c/flashrom/+/71975/comment/ae2b6819_7afc693b PS6, Line 123: range "range of addresses that are protected" seems a little clearer. -- To view, visit https://review.coreboot.org/c/flashrom/+/71975 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I464b1f1393a275a0e636654c6aec7b7b94bcb16c Gerrit-Change-Number: 71975 Gerrit-PatchSet: 6 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 18 Jan 2023 04:15:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Use Range and usize for all ranges
by Peter Marheine (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Evan Benn. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72004 ) Change subject: flashrom_tester: Use Range and usize for all ranges ...................................................................... Patch Set 5: (2 comments) File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/72004/comment/f87c31e2_d97abca6 PS5, Line 56: start,length It's confusing to describe this as `(start,length)` just because the range itself is using the usual Rust semantics (and it gets translated to the flashrom semantics behind the scenes). Could just add some commentary here to that effect if you wanted to retain the information on how this maps to command line options, I guess. https://review.coreboot.org/c/flashrom/+/72004/comment/65df7985_2f60552e PS5, Line 56: usize I think `u64` is more appropriate, since we don't really care how this relates to the system's pointer width and that still satisfies the goal of using an unsigned type. -- To view, visit https://review.coreboot.org/c/flashrom/+/72004 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic09be66c3598acf45fbe8952093006a9b185810a Gerrit-Change-Number: 72004 Gerrit-PatchSet: 5 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 18 Jan 2023 04:07:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: flashrom_tester: Rewrite IOOpts to support more operations
by Peter Marheine (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Evan Benn. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71973 ) Change subject: flashrom_tester: Rewrite IOOpts to support more operations ...................................................................... Patch Set 4: (2 comments) File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/71973/comment/525e8598_96929efd PS4, Line 82: // an operation must provide a file or a region.file or both. Does this vary with what operation is being performed? It might be more obvious (regarding correct use of fields) to make each `Operation` have fields for the relevant options: ``` enum LayoutSource { Fmap, LayoutFile(Path), } enum Region { EntireChip, Named(&str, LayoutSource), } enum Operation { Read(Region, Path), Write(Region, Path), ... Erase(Region), } ``` https://review.coreboot.org/c/flashrom/+/71973/comment/2ad73be2_293b7e40 PS4, Line 148: #[allow(clippy::needless_update)] // clippy doesnt like Default when all fields are filled Seems okay to omit the `default` then, unless you anticipate adding more fields that would require adding it back in. -- To view, visit https://review.coreboot.org/c/flashrom/+/71973 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1cb46bb1b26949fd9c19949c43708a8b652e00da Gerrit-Change-Number: 71973 Gerrit-PatchSet: 4 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 18 Jan 2023 03:58:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom_tester: Rename and simplify lock_test
by Peter Marheine (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Evan Benn. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71972 ) Change subject: flashrom_tester: Rename and simplify lock_test ...................................................................... Patch Set 1: (1 comment) File util/flashrom_tester/src/tests.rs: https://review.coreboot.org/c/flashrom/+/71972/comment/a1b985bb_fc706c70 PS1, Line 208: .set_sw(true)?; This changes the ordering to now enable HW before SW. Does that still work? -- To view, visit https://review.coreboot.org/c/flashrom/+/71972 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6080622755ff16d8fba7044b38f9e09db0c62f97 Gerrit-Change-Number: 71972 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 18 Jan 2023 03:44:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom_tester: Simplify wp_toggle_test and rename
by Peter Marheine (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Evan Benn. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71971 ) Change subject: flashrom_tester: Simplify wp_toggle_test and rename ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/71971 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I314aa8d9708c39cc162a8d5e95ca3e019c2fb5b8 Gerrit-Change-Number: 71971 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 18 Jan 2023 03:43:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom_tester: Add a description of each test
by Peter Marheine (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Evan Benn. Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71970 ) Change subject: flashrom_tester: Add a description of each test ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/71970 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Iacd23e5ac4635eee19f99d849c6e58c5a193f904 Gerrit-Change-Number: 71970 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Wed, 18 Jan 2023 03:40:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: Check WP range after setting it
by Evan Benn (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: Peter Marheine. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71975 ) Change subject: flashrom_tester: Check WP range after setting it ...................................................................... Patch Set 6: This change is ready for review. -- To view, visit https://review.coreboot.org/c/flashrom/+/71975 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I464b1f1393a275a0e636654c6aec7b7b94bcb16c Gerrit-Change-Number: 71975 Gerrit-PatchSet: 6 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> 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-Comment-Date: Wed, 18 Jan 2023 03:26:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2023