November 2022 - flashrom-gerrit

[M] Change in flashrom[master]: tests: Add selfcheck to unit tests
by Evan Benn (Code Review) 25 Nov '22

25 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69620 ) Change subject: tests: Add selfcheck to unit tests ...................................................................... Patch Set 7: (1 comment) File tests/well_formed_tables.c: https://review.coreboot.org/c/flashrom/+/69620/comment/291c2a76_9868dc5b PS6, Line 24: do { \ : if (!(assertion)) \ : fail_msg(#assertion " is false for index:%zu name:%s", (index), \ : (name) ? (name) : "unknown"); \ : } while (0) > Didn't think of that. […] I agree that would be the preferred style but a macro is used so that the __LINE__ format used by fail_msg points to the location of the assertion, not the function -- To view, visit https://review.coreboot.org/c/flashrom/+/69620 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I41cd014d9bf909296b6c28e3e00548e6883ff41a Gerrit-Change-Number: 69620 Gerrit-PatchSet: 7 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Fri, 25 Nov 2022 05:30:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: opaque_master: Mark Opaque chip as tested for WP
by Anastasia Klimchuk (Code Review) 25 Nov '22

25 Nov '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69842 ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: opaque_master: Mark Opaque chip as tested for WP ...................................................................... opaque_master: Mark Opaque chip as tested for WP Opaque masters, by design, populate the flashchip structure during the execution of their probe function. Therefore any opaque master operation displays a message to the user: "This flash part has status UNTESTED for operations: WP". However, for all the other operations (read, write, erase) opaque masters always mark them as tested. Thus, align WP as marked tested inline with other opaque chip operations. BUG=b:258755442 TEST=the following does not display untested message: 1) flashrom -p dummy:size=8388608,emulate=VARIABLE_SIZE 2) flashrom -p internal (on Intel device) Change-Id: I5ae4cb49eb0abc6ab26cfe2f3359e4e50dd4fd4f Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/69842 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M dummyflasher.c M ichspi.c M nicintel_eeprom.c 3 files changed, 33 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved Sergii Dmytruk: Looks good to me, but someone else must approve diff --git a/dummyflasher.c b/dummyflasher.c index 110a88c..35cdfef 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -145,7 +145,7 @@ msg_cdbg("%s: set flash->total_size to %dK bytes.\n", __func__, flash->chip->total_size); - flash->chip->tested = TEST_OK_PREW; + flash->chip->tested = TEST_OK_PREWB; if (emu_data->erase_to_zero) flash->chip->feature_bits |= FEATURE_ERASED_ZERO; diff --git a/ichspi.c b/ichspi.c index 4e982f0..cd330e3 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1485,7 +1485,7 @@ msg_cdbg("In that range are %d erase blocks with %d B each.\n", size_high / erase_size_high, erase_size_high); } - flash->chip->tested = TEST_OK_PREW; + flash->chip->tested = TEST_OK_PREWB; return 1; } diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index ff2c3ce..4d5ab6b 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -115,7 +115,7 @@ /* Emulated eeprom has a fixed size of 4 KB */ flash->chip->total_size = 4; flash->chip->page_size = flash->chip->total_size * 1024; - flash->chip->tested = TEST_OK_PREW; + flash->chip->tested = TEST_OK_PREWB; flash->chip->gran = write_gran_1byte_implicit_erase; flash->chip->block_erasers->eraseblocks[0].size = flash->chip->page_size; flash->chip->block_erasers->eraseblocks[0].count = 1; @@ -146,7 +146,7 @@ } flash->chip->page_size = EE_PAGE_MASK + 1; - flash->chip->tested = TEST_OK_PREW; + flash->chip->tested = TEST_OK_PREWB; flash->chip->gran = write_gran_1byte_implicit_erase; flash->chip->block_erasers->eraseblocks[0].size = (EE_PAGE_MASK + 1); flash->chip->block_erasers->eraseblocks[0].count = (flash->chip->total_size * 1024) / (EE_PAGE_MASK + 1); -- To view, visit https://review.coreboot.org/c/flashrom/+/69842 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5ae4cb49eb0abc6ab26cfe2f3359e4e50dd4fd4f Gerrit-Change-Number: 69842 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: tests: Add selfcheck to unit tests
by Felix Singer (Code Review) 25 Nov '22

25 Nov '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk, Evan Benn. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69620 ) Change subject: tests: Add selfcheck to unit tests ...................................................................... Patch Set 7: (1 comment) File tests/well_formed_tables.c: https://review.coreboot.org/c/flashrom/+/69620/comment/54af8323_868ec379 PS6, Line 24: do { \ : if (!(assertion)) \ : fail_msg(#assertion " is false for index:%zu name:%s", (index), \ : (name) ? (name) : "unknown"); \ : } while (0) > yes its the standard semicolon trick for macros. So you can call it like a function. Didn't think of that. How about making this a static inline function? It does the same but it's less confusing :) -- To view, visit https://review.coreboot.org/c/flashrom/+/69620 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I41cd014d9bf909296b6c28e3e00548e6883ff41a Gerrit-Change-Number: 69620 Gerrit-PatchSet: 7 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Fri, 25 Nov 2022 03:06:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: add probe lifecycle test for ch341a_spi
by Anastasia Klimchuk (Code Review) 24 Nov '22

24 Nov '22

Attention is currently required from: Thomas Heijligen, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68755 ) Change subject: tests: add probe lifecycle test for ch341a_spi ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/68755 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a2d5591d097435fc69719e1d9bd153433425821 Gerrit-Change-Number: 68755 Gerrit-PatchSet: 4 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 24 Nov 2022 09:11:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: add basic lifecycle test for ch341a_spi
by Anastasia Klimchuk (Code Review) 24 Nov '22

24 Nov '22

Attention is currently required from: Felix Singer, Thomas Heijligen, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67664 ) Change subject: tests: add basic lifecycle test for ch341a_spi ...................................................................... Patch Set 10: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/67664 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If28fbe09ad685082152aa3a7e8d5a150169aee9e Gerrit-Change-Number: 67664 Gerrit-PatchSet: 10 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 24 Nov 2022 09:07:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests: add mocks for libusb's asynchronous API
by Anastasia Klimchuk (Code Review) 24 Nov '22

24 Nov '22

Attention is currently required from: Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69873 ) Change subject: tests: add mocks for libusb's asynchronous API ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69873 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5a316687ab39a112d968eeaedb71f7b4b659d8d5 Gerrit-Change-Number: 69873 Gerrit-PatchSet: 1 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 24 Nov 2022 08:58:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests: add more wrappers for libusb funcs
by Anastasia Klimchuk (Code Review) 24 Nov '22

24 Nov '22

Attention is currently required from: Thomas Heijligen, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69872 ) Change subject: tests: add more wrappers for libusb funcs ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic11efb9fd746cb91911dbe87e1c0028759f5bb0b Gerrit-Change-Number: 69872 Gerrit-PatchSet: 1 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Thu, 24 Nov 2022 08:58:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashchips: Add support for GigaDevice GD251R512ME
by Kapil Porwal (Code Review) 24 Nov '22

24 Nov '22

Attention is currently required from: Harsha B R, Ravishankar Sarawadi. Kapil Porwal has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69928 ) Change subject: flashchips: Add support for GigaDevice GD251R512ME ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69928/comment/cc2e75ab_9250ed43 PS1, Line 7: 1 `L`? -- To view, visit https://review.coreboot.org/c/flashrom/+/69928 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I15f76ce089cb2a2baf8bf8525510a7a7fe44b352 Gerrit-Change-Number: 69928 Gerrit-PatchSet: 1 Gerrit-Owner: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Harsha B R <harsha.b.r(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Harsha B R <harsha.b.r(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Comment-Date: Thu, 24 Nov 2022 07:07:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: flashrom_tester: partial_lock: Use WriteProtectState cache
by Edward O'Callaghan (Code Review) 24 Nov '22

24 Nov '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69418 ) ( 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: flashrom_tester: partial_lock: Use WriteProtectState cache ...................................................................... flashrom_tester: partial_lock: Use WriteProtectState cache partial_lock test was bypassing the WriteProtectState cache of the software write protect by directly calling env.cmd.wp_range. It was also unnesesarily disabling software WP. Fix those issues and more clearly document what the test is doing and expecting. BUG=b:244663741 BRANCH=None TEST=flashrom_tester --libflashrom host Change-Id: Ic3f89ff5d22e74e4e6c94e755b936e58cb27182d Signed-off-by: Evan Benn <evanbenn(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/69418 Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M util/flashrom_tester/src/tester.rs M util/flashrom_tester/src/tests.rs 2 files changed, 47 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Peter Marheine: Looks good to me, but someone else must approve diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs index d19e45c..1fa44a8 100644 --- a/util/flashrom_tester/src/tester.rs +++ b/util/flashrom_tester/src/tester.rs @@ -246,6 +246,24 @@ } } + // Set software write protect with a custom range + pub fn set_range(&mut self, range: (i64, i64), enable: bool) -> Result<&mut Self, String> { + info!("set_range request={}, current={}", enable, self.current.sw); + self.cmd + .wp_range(range, enable) + .map_err(|e| e.to_string())?; + let actual_state = Self::get_sw(self.cmd).map_err(|e| e.to_string())?; + if actual_state != enable { + Err(format!( + "set_range request={}, real={}", + enable, actual_state + )) + } else { + self.current.sw = enable; + Ok(self) + } + } + /// Set the hardware write protect if supported and check that the state is as expected. pub fn set_hw(&mut self, enable: bool) -> Result<&mut Self, String> { info!("set_hw request={}, current={}", enable, self.current.hw); diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index e6968c8..3e39e4d 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -272,10 +272,12 @@ env.ensure_golden()?; let (wp_section_name, start, len) = utils::layout_section(env.layout(), section); - // Disable software WP so we can do range protection, but hardware WP - // must remain enabled for (most) range protection to do anything. - env.wp.set_hw(false)?.set_sw(false)?; - env.cmd.wp_range((start, len), true)?; + // Disable hardware WP so we can modify the protected range. + env.wp.set_hw(false)?; + // Then enable software WP so the range is enforced and enable hardware + // WP so that flashrom does not disable software WP during the + // operation. + env.wp.set_range((start, len), true)?; env.wp.set_hw(true)?; // Check that we cannot write to the protected region. -- To view, visit https://review.coreboot.org/c/flashrom/+/69418 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic3f89ff5d22e74e4e6c94e755b936e58cb27182d Gerrit-Change-Number: 69418 Gerrit-PatchSet: 6 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: flashrom_tester: Change the wp_toggle semantics
by Edward O'Callaghan (Code Review) 24 Nov '22

24 Nov '22

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/69417 ) Change subject: flashrom_tester: Change the wp_toggle semantics ...................................................................... flashrom_tester: Change the wp_toggle semantics wp_toggle and wp_range had some confusing behaviour where enabling wp would set a range, but disabling wp would not unset the range (explicitly). This was a way to workaround the MTD kernel driver semantics. Now make things very explicit, enabling software write protect will set the range to the whole chip. Disabling write protect will set the range to 0,0. This makes all drivers behave the same as MTD, and documents the exact behaviour explicitly. BUG=b:244663741 BRANCH=None TEST=flashrom_tester --libflashrom host # MTD and non-MTD TEST=flashrom_tester --flashrom_binary # MTD and non-MTD Change-Id: Ia01d612d988e6580a7c5f0fd448ccc319ce9b181 Signed-off-by: Evan Benn <evanbenn(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/69417 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Edward O'Callaghan <quasisec(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 3 files changed, 40 insertions(+), 25 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs index 4078910..458f054 100644 --- a/util/flashrom_tester/flashrom/src/cmd.rs +++ b/util/flashrom_tester/flashrom/src/cmd.rs @@ -149,11 +149,12 @@ Ok(true) } - fn wp_range(&self, range: (i64, i64), wp_enable: bool) -> Result<bool, FlashromError> { + fn wp_range(&self, range: (i64, i64), en: bool) -> Result<bool, FlashromError> { let opts = FlashromOpt { wp_opt: WPOpt { + enable: en, + disable: !en, range: Some(range), - enable: wp_enable, ..Default::default() }, ..Default::default() @@ -200,28 +201,14 @@ } fn wp_toggle(&self, en: bool) -> Result<bool, FlashromError> { - let status = if en { "en" } else { "dis" }; - - // For MTD, --wp-range and --wp-enable must be used simultaneously. let range = if en { let rom_sz: i64 = self.get_size()?; - Some((0, rom_sz)) // (start, len) + (0, rom_sz) // (start, len) } else { - None + (0, 0) }; - - let opts = FlashromOpt { - wp_opt: WPOpt { - range, - enable: en, - disable: !en, - ..Default::default() - }, - ..Default::default() - }; - - self.dispatch(opts, "wp_toggle")?; - + self.wp_range(range, en)?; + let status = if en { "en" } else { "dis" }; match self.wp_status(true) { Ok(_ret) => { info!("Successfully {}abled write-protect", status); diff --git a/util/flashrom_tester/flashrom/src/flashromlib.rs b/util/flashrom_tester/flashrom/src/flashromlib.rs index 3036394..6cdd55d 100644 --- a/util/flashrom_tester/flashrom/src/flashromlib.rs +++ b/util/flashrom_tester/flashrom/src/flashromlib.rs @@ -102,10 +102,8 @@ } fn wp_toggle(&self, en: bool) -> Result<bool, FlashromError> { - // TODO why does the cmd impl not do this? - // for cmd, range is only set for enable - // and disable is not sent for the wp_range command - self.wp_range((0, self.get_size()?), en) + let range = if en { (0, self.get_size()?) } else { (0, 0) }; + self.wp_range(range, en) } fn read_into_file(&self, path: &Path) -> Result<(), FlashromError> { diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs index 11874f9..90e40e2 100644 --- a/util/flashrom_tester/flashrom/src/lib.rs +++ b/util/flashrom_tester/flashrom/src/lib.rs @@ -133,7 +133,7 @@ /// Write only a region of the flash. fn write_file_with_layout(&self, rws: &ROMWriteSpecifics) -> Result<bool, FlashromError>; - /// Set write protect status for a range. + /// Set write protect status and range. fn wp_range(&self, range: (i64, i64), wp_enable: bool) -> Result<bool, FlashromError>; /// Read the write protect regions for the flash. @@ -143,6 +143,10 @@ fn wp_status(&self, en: bool) -> Result<bool, FlashromError>; /// Set write protect status. + /// If en=true sets wp_range to the whole chip (0,getsize()). + /// If en=false sets wp_range to (0,0). + /// This is due to the MTD driver, which requires wp enable to use a range + /// length != 0 and wp disable to have the range 0,0. fn wp_toggle(&self, en: bool) -> Result<bool, FlashromError>; /// Read the whole flash to the file specified by `path`. -- To view, visit https://review.coreboot.org/c/flashrom/+/69417 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia01d612d988e6580a7c5f0fd448ccc319ce9b181 Gerrit-Change-Number: 69417 Gerrit-PatchSet: 5 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022