November 2022 - flashrom-gerrit

[M] Change in flashrom[master]: flashrom_tester: Move layout_file to TestEnv
by Evan Benn (Code Review) Oct. 31, 2022

Oct. 31, 2022

Evan Benn has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/69063 ) Change subject: flashrom_tester: Move layout_file to TestEnv ...................................................................... flashrom_tester: Move layout_file to TestEnv layout_file is part of the test environment, move it from a global to a member of the TestEnv struct. This allows layout to be part of the tempdir in a subsequent patch. BUG=b:243460685 BRANCH=None TEST=/usr/bin/flashrom_tester --flashrom_binary /usr/sbin/flashrom host TEST=/usr/bin/flashrom_tester --libflashrom host Change-Id: Ia7e8efeb4fbac0a46627f079956d671aed43f1c7 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/src/tester.rs M util/flashrom_tester/src/tests.rs 2 files changed, 56 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/69063/1 diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs index f2c3846..292fb78 100644 --- a/util/flashrom_tester/src/tester.rs +++ b/util/flashrom_tester/src/tester.rs @@ -39,6 +39,8 @@ use flashrom::FlashromError; use flashrom::{FlashChip, Flashrom}; use serde_json::json; +use std::fs::File; +use std::io::Write; use std::mem::MaybeUninit; use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Mutex; @@ -63,10 +65,16 @@ /// The path to a file containing flash-sized random data // TODO(pmarheine) make this a PathBuf too random_data: String, + /// The path to a file containing layout data. + pub layout_file: String, } impl<'a> TestEnv<'a> { - pub fn create(chip_type: FlashChip, cmd: &'a dyn Flashrom) -> Result<Self, FlashromError> { + pub fn create( + chip_type: FlashChip, + cmd: &'a dyn Flashrom, + print_layout: bool, + ) -> Result<Self, FlashromError> { let rom_sz = cmd.get_size()?; let out = TestEnv { chip_type, @@ -75,6 +83,7 @@ wp: WriteProtectState::from_hardware(cmd, chip_type)?, original_flash_contents: "/tmp/flashrom_tester_golden.bin".into(), random_data: "/tmp/random_content.bin".into(), + layout_file: create_layout_file(rom_sz, "/tmp/", print_layout), }; info!("Stashing golden image for verification/recovery on completion"); @@ -487,17 +496,39 @@ } } +fn create_layout_file(rom_sz: i64, tmp_dir: &str, print_layout: bool) -> String { + info!("Calculate ROM partition sizes & Create the layout file."); + let layout_sizes = utils::get_layout_sizes(rom_sz).expect("Could not partition rom"); + + let mut layout_file = tmp_dir.to_string(); + layout_file.push_str("/layout.file"); + let mut f = File::create(&layout_file).expect("Could not create layout file"); + let mut buf: Vec<u8> = vec![]; + utils::construct_layout_file(&mut buf, &layout_sizes).expect("Could not construct layout file"); + + f.write_all(&buf).expect("Writing layout file failed"); + if print_layout { + info!( + "Dumping layout file as requested:\n{}", + String::from_utf8_lossy(&buf) + ); + } + layout_file +} + pub fn run_all_tests<T, TS>( chip: FlashChip, cmd: &dyn Flashrom, ts: TS, terminate_flag: Option<&AtomicBool>, + print_layout: bool, ) -> Vec<(String, (TestConclusion, Option<TestError>))> where T: TestCase + Copy, TS: IntoIterator<Item = T>, { - let mut env = TestEnv::create(chip, cmd).expect("Failed to set up test environment"); + let mut env = + TestEnv::create(chip, cmd, print_layout).expect("Failed to set up test environment"); let mut results = Vec::new(); for t in ts { diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index 1527a6e..3e1737e 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -40,10 +40,9 @@ use std::collections::{HashMap, HashSet}; use std::convert::TryInto; use std::fs::{self, File}; -use std::io::{BufRead, Write}; +use std::io::BufRead; use std::sync::atomic::AtomicBool; -const LAYOUT_FILE: &str = "/tmp/layout.file"; const ELOG_FILE: &str = "/tmp/elog.file"; /// Iterate over tests, yielding only those tests with names matching filter_names. @@ -91,23 +90,6 @@ ) -> Result<(), Box<dyn std::error::Error>> { utils::ac_power_warning(); - info!("Calculate ROM partition sizes & Create the layout file."); - let rom_sz: i64 = cmd.get_size()?; - let layout_sizes = utils::get_layout_sizes(rom_sz)?; - { - let mut f = File::create(LAYOUT_FILE)?; - let mut buf: Vec<u8> = vec![]; - utils::construct_layout_file(&mut buf, &layout_sizes)?; - - f.write_all(&buf)?; - if print_layout { - info!( - "Dumping layout file as requested:\n{}", - String::from_utf8_lossy(&buf) - ); - } - } - info!("Record crossystem information.\n{}", crossystem); // Register tests to run: @@ -144,7 +126,7 @@ // ------------------------. // Run all the tests and collate the findings: - let results = tester::run_all_tests(fc, cmd, tests, terminate_flag); + let results = tester::run_all_tests(fc, cmd, tests, terminate_flag, print_layout); // Any leftover filtered names were specified to be run but don't exist for leftover in filter_names.iter().flatten() { @@ -295,7 +277,7 @@ // Check that we cannot write to the protected region. let rws = flashrom::ROMWriteSpecifics { - layout_file: Some(LAYOUT_FILE), + layout_file: Some(&env.layout_file), write_file: Some(env.random_data_file()), name_file: Some(wp_section_name), }; @@ -313,7 +295,7 @@ let (non_wp_section_name, _, _) = utils::layout_section(env.layout(), section.get_non_overlapping_section()); let rws = flashrom::ROMWriteSpecifics { - layout_file: Some(LAYOUT_FILE), + layout_file: Some(&env.layout_file), write_file: Some(env.random_data_file()), name_file: Some(non_wp_section_name), }; -- To view, visit https://review.coreboot.org/c/flashrom/+/69063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia7e8efeb4fbac0a46627f079956d671aed43f1c7 Gerrit-Change-Number: 69063 Gerrit-PatchSet: 1 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: flashrom_tester: Use tempdir crate for test data
by Evan Benn (Code Review) Oct. 31, 2022

Oct. 31, 2022

Attention is currently required from: Anastasia Klimchuk. Hello build bot (Jenkins), Anastasia Klimchuk, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/68739 to look at the new patch set (#3). Change subject: flashrom_tester: Use tempdir crate for test data ...................................................................... flashrom_tester: Use tempdir crate for test data Use tempdir crate to create a tempdir that is deleted when the tester closes. BUG=b:243460685 BRANCH=None TEST=/usr/bin/flashrom_tester --flashrom_binary /usr/sbin/flashrom host TEST=/usr/bin/flashrom_tester --libflashrom host Change-Id: I14089ef69c3f509cb6f01cc44c7924c244ab7d73 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> --- M util/flashrom_tester/Cargo.toml M util/flashrom_tester/src/tester.rs M util/flashrom_tester/src/tests.rs 3 files changed, 32 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/68739/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/68739 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I14089ef69c3f509cb6f01cc44c7924c244ab7d73 Gerrit-Change-Number: 68739 Gerrit-PatchSet: 3 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: flashrom.c: Make 'chip_to_probe' a param to probe_flash()
by Felix Singer (Code Review) Oct. 31, 2022

Oct. 31, 2022

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67091 ) Change subject: flashrom.c: Make 'chip_to_probe' a param to probe_flash() ...................................................................... Patch Set 13: Code-Review+2 (2 comments) File flashrom.c: https://review.coreboot.org/c/flashrom/+/67091/comment/9e69b4d4_43df1e6b PS13, Line 868: goto notfound; Please a line after that File w29ee011.c: https://review.coreboot.org/c/flashrom/+/67091/comment/003650e6_4db81a3d PS13, Line 25: override_chip || strcmp(override_chip, chip_name) Just an idea, a nit: How about moving this to a variable `can_override`? This also would allow us having one path regarding the returns. On the other side this way it's more clear what is returned. Not sure what's better. Feel free to mark as resolved if you think it's fine this way 😊 -- To view, visit https://review.coreboot.org/c/flashrom/+/67091 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifcdace07ea2135d83dea92cfa5c6bec8d7ddf05d Gerrit-Change-Number: 67091 Gerrit-PatchSet: 13 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Nov 2022 00:50:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XL] Change in flashrom[master]: tree/: Convert flashchip read func ptr to enumerate
by Felix Singer (Code Review) Oct. 31, 2022

Oct. 31, 2022

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66788 ) Change subject: tree/: Convert flashchip read func ptr to enumerate ...................................................................... Patch Set 15: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/66788/comment/82d26f1c_7eaf7bf5 PS15, Line 14: Squash in, : flashchips.c: convert edi_chip_read() : flashchips.c: convert spi_chip_read() : flashchips.c: convert read_opaque() : flashchips.c: convert read_memmapped() : flashchips.c: convert spi_read_at45db_e8() : flashchips.c: convert spi_read_at45db() : flashchips.c: convert NULL cases Not needed anymore -- To view, visit https://review.coreboot.org/c/flashrom/+/66788 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I612d46fefedf2b69e7e2064aa857fa0756efb4e7 Gerrit-Change-Number: 66788 Gerrit-PatchSet: 15 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Nov 2022 00:26:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XL] Change in flashrom[master]: tree/: Convert flashchip write func ptr to enumerate
by Felix Singer (Code Review) Oct. 31, 2022

Oct. 31, 2022

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66782 ) Change subject: tree/: Convert flashchip write func ptr to enumerate ...................................................................... Patch Set 14: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/66782/comment/07a991a6_2215a2e1 PS14, Line 14: Squash in, : flashchips.c: convert spi_chip_write_256() : flashchips.c: convert write_jedec_1() : flashchips.c: convert write_jedec() : flashchips.c: convert write_opaque() : flashchips.c: convert spi_chip_write_1() : flashchips.c: convert spi_aai_write() : flashchips.c: convert edi_chip_write() : flashchips.c: convert write_en29lv640b() : flashchips.c: convert write_82802ab() : flashchips.c: convert spi_write_at45db() : flashchips.c: convert write_28sf040() : flashchips.c: convert NULL write Not needed anymore -- To view, visit https://review.coreboot.org/c/flashrom/+/66782 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I80149de169464b204fb09f1424a86fc645b740fd Gerrit-Change-Number: 66782 Gerrit-PatchSet: 14 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> 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-Comment-Date: Tue, 01 Nov 2022 00:19:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XL] Change in flashrom[master]: tree/: Convert flashchip probe func ptr to enumerate
by Felix Singer (Code Review) Oct. 31, 2022

Oct. 31, 2022

Attention is currently required from: Edward O'Callaghan, Angel Pons, Arthur Heymans, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66781 ) Change subject: tree/: Convert flashchip probe func ptr to enumerate ...................................................................... Patch Set 13: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/66781/comment/00f7682b_d7737eb1 PS3, Line 759: static probe_func_t lookup_probe_func_ptr(const struct flashchip *chip) > Seems reasonable, just took a quick look though. I think this is resolved :) Marking as done. -- To view, visit https://review.coreboot.org/c/flashrom/+/66781 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I00aaab9c83f305cd47e78c36d9c2867f2b73c396 Gerrit-Change-Number: 66781 Gerrit-PatchSet: 13 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Nov 2022 00:19:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[XL] Change in flashrom[master]: tree/: Convert flashchip probe func ptr to enumerate
by Felix Singer (Code Review) Oct. 31, 2022

Oct. 31, 2022

Attention is currently required from: Edward O'Callaghan, Arthur Heymans, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66781 ) Change subject: tree/: Convert flashchip probe func ptr to enumerate ...................................................................... Patch Set 13: Code-Review+2 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/66781/comment/95fc0642_04d2d16c PS13, Line 14: squash in, : : flashchip.c: probe_jedec_29gl : flashchip.c: probe_82802ab : flashchip.c: probe_opaque : flashchip.c: probe_jedec : flashchip.c: probe_spi_res2 : flashchip.c: probe_spi_res1 : flashchip.c: probe_spi_rems : flashchip.c: probe_spi_at25f : flashchip.c: probe_spi_at45db : flashchip.c: probe_spi_st95 : flashchip.c: probe_spi_sfdp : flashchip.c: probe_spi_rdid{4} : flashchip.c: probe_w29ee011 : flashchip.c: probe_en29lv640b : flashchip.c: edi_probe_kb9012 : flashchip.c: probe_spi_big_spansion I guess this is not needed anymore ;) Patchset: PS13: Thanks for keeping this up for so long Edward! Just one small thing is left 😊 -- To view, visit https://review.coreboot.org/c/flashrom/+/66781 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I00aaab9c83f305cd47e78c36d9c2867f2b73c396 Gerrit-Change-Number: 66781 Gerrit-PatchSet: 13 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 01 Nov 2022 00:17:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0