- flashrom-gerrit - mail.coreboot.org

[L] Change in flashrom[master]: Add AMD Ryzen Support & VIA VL805 Support
by Arthur Heymans (Code Review) 18 Jan '23

18 Jan '23

Attention is currently required from: ChrisEric1 CECL, Thomas Heijligen. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72020 ) Change subject: Add AMD Ryzen Support & VIA VL805 Support ...................................................................... Patch Set 6: (3 comments) Patchset: PS6: Please drop the AMD changes and just add in VL805 in one patch. File sb600spi.c: https://review.coreboot.org/c/flashrom/+/72020/comment/7ad4595c_2f5a0f66 PS6, Line 56: uint8_t RZN32BM = 0; Accessing larger than 16M is indeed a problem. https://review.coreboot.org/c/flashrom/+/71565 fixes it. https://review.coreboot.org/c/flashrom/+/72020/comment/de025556_ec0d118f PS6, Line 128: (rev == 0x51 || rev == 0x59 || rev == 0x61) Those cases are covered below... -- To view, visit https://review.coreboot.org/c/flashrom/+/72020 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7452b169ad89e0a884a46acc6a0499e97bb6ff7e Gerrit-Change-Number: 72020 Gerrit-PatchSet: 6 Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 18 Jan 2023 06:35:46 +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 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/+/72004 ) Change subject: flashrom_tester: Use Range and usize for all ranges ...................................................................... Patch Set 6: (2 comments) File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/72004/comment/9f65720e_71bfde2d PS5, Line 56: usize > I think `u64` is more appropriate, since we don't really care how this relates to the system's point […] yes this is where we get into the mess of the flashrom API types. getsize returns a `size_t`. Other parts of the API do other stuff, including `int`, `unsigned int` uint32_t and more :'). usize is big enough on 32 bit platforms to hold flashrom range types, because those dont even get to 32 bits afaict. The advantage of usize is that I can index into arrays with it without annoying type conversions `buf[range]`. I have deja vu, I think we had this conversation last time round! https://review.coreboot.org/c/flashrom/+/72004/comment/1d664a3b_e3223234 PS5, Line 56: start,length > It's confusing to describe this as `(start,length)` just because the range itself is using the usual […] Done -- 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: 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 06:12:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> 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 7: (4 comments) File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/71975/comment/111f5c46_32ee18cf PS6, Line 207: Range<usize> > Is it possible for multiple ranges to be protected at the same time? No, although the stuff nikolai is working on might introduce this, it doesnt exist yet. https://evanbenn.users.x20web.corp.google.com/www/libflashrom-doc/html/grou… https://review.coreboot.org/c/flashrom/+/71975/comment/50ff5b4b_3ddbdb2b PS6, Line 227: \d > `[[:xdigit:]]` ! lucky that worked in my test? https://review.coreboot.org/c/flashrom/+/71975/comment/6605ec4d_e7c82de3 PS6, Line 231: range_match.get(1).unwrap().as_str() > You can use `range_match[1]` to be more concise. Done File util/flashrom_tester/flashrom/src/lib.rs: https://review.coreboot.org/c/flashrom/+/71975/comment/7b6dc926_5e088838 PS6, Line 123: range > "range of addresses that are protected" seems a little clearer. Done -- 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: 7 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 06:11:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> 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: Evan Benn. Hello build bot (Jenkins), Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71975 to look at the new patch set (#8). Change subject: flashrom_tester: Check WP range after setting it ...................................................................... flashrom_tester: Check WP range after setting it Double check that fetching the software write protect range after we set it matches what we tried to set it to. BUG=b:236085139 BRANCH=None TEST=None Change-Id: I464b1f1393a275a0e636654c6aec7b7b94bcb16c Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/flashrom/Cargo.toml 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/tester.rs 5 files changed, 63 insertions(+), 26 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/71975/8 -- 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: 8 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

[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 5: (1 comment) File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/71973/comment/b79c4b9e_194537a7 PS4, Line 82: // an operation must provide a file or a region.file or both. > I tried it a bit like this way initially, I didn't like that the things inside the enum variants don […] If each operation can take both a region and file path (or more generally, if every operation can take the same options), then it seems okay to not put fields in the `Operation` and instead put it alongside a `Region` like I've outlined; the bigger win seems like how the `Region` better expresses where data might come from. If you want named fields you can always use struct variants (so you don't need to define more structs): ``` enum Operation { Read { region: Region, path: Path } ... } ``` -- 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-Comment-Date: Wed, 18 Jan 2023 06:05:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

[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: Peter Marheine. Evan Benn 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 5: (2 comments) File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/71973/comment/4acc3667_be24a538 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 u […] I tried it a bit like this way initially, I didn't like that the things inside the enum variants dont have names. I suppose I could put a struct in each though. I did not notice that Erase can have a region, but I'm not using it yet so I'll leave it off. https://review.coreboot.org/c/flashrom/+/71973/comment/4ebcb97b_beebed73 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 a […] I had thought the point of it was to save me if I add a field back in and forget to modify this site. I think I am misunderstanding -- 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 18 Jan 2023 06:00:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

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: Peter Marheine. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71972 ) Change subject: flashrom_tester: Rename simplify lock_test hwwp_locks_swwp_test ...................................................................... Patch Set 2: (1 comment) File util/flashrom_tester/src/tests.rs: https://review.coreboot.org/c/flashrom/+/71972/comment/bb1c40ec_6dd3326f PS1, Line 208: .set_sw(true)?; > This changes the ordering to now enable HW before SW. […] it did still work on the duts I tried it on. I'll just leave it how it was, chestertons fence and all -- 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Wed, 18 Jan 2023 05:59:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

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

[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

[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: Evan Benn. Hello build bot (Jenkins), Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71975 to look at the new patch set (#7). Change subject: flashrom_tester: Check WP range after setting it ...................................................................... flashrom_tester: Check WP range after setting it Double check that fetching the software write protect range after we set it matches what we tried to set it to. BUG=b:236085139 BRANCH=None TEST=None Change-Id: I464b1f1393a275a0e636654c6aec7b7b94bcb16c Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/flashrom/Cargo.toml 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/tester.rs 5 files changed, 63 insertions(+), 26 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/71975/7 -- 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: 7 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

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit