- flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[main]: Remove dependency on C23 __has_include()
by Alexander Goncharov (Code Review) 28 Nov '23

28 Nov '23

Attention is currently required from: Anton Samsonov, Thomas Heijligen. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/77089?usp=email ) Change subject: Remove dependency on C23 __has_include() ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/77089?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic544963ffd29626ae0a21bdddb1c78850cc43ec6 Gerrit-Change-Number: 77089 Gerrit-PatchSet: 6 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: 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: Anton Samsonov <avscomputing(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Tue, 28 Nov 2023 20:31:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in flashrom[main]: erasure_layout: Remove redundant `verify_range` call from `erase_write`
by Carly Zlabek (Code Review) 28 Nov '23

28 Nov '23

Carly Zlabek has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/79324?usp=email ) Change subject: erasure_layout: Remove redundant `verify_range` call from `erase_write` ...................................................................... erasure_layout: Remove redundant `verify_range` call from `erase_write` Previously, verification would be unconditionally completed at the end of `erase_write` with the non-legacy erase path. Verification is already handled when writing and erasing chip data; in `flashrom_image_write` based on `flashctx` options, and in the erase loop of `erase_write` via `check_erased_range` calls, respectively. This unnecessary verification added a performance penalty that was not present with the legacy implementation. Now, this call to `verify_range` has been removed to improve performance and more closely reflect the verification done with the legacy path. This change was tested using the linux_spi programmer to erase and write to an MT25QL512 chip. Change-Id: I638835facd9311979c4991cc4ca41a4b9e174bd5 Signed-off-by: Carly Zlabek <carlyzlabek(a)gmail.com> Signed-off-by: Vincent Fazio <vfazio(a)gmail.com> --- M erasure_layout.c 1 file changed, 0 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/79324/1 diff --git a/erasure_layout.c b/erasure_layout.c index 0c31911..febfde1 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -385,13 +385,6 @@ *all_skipped = false; } - // verify write - ret = verify_range(flashctx, newcontents + region_start, region_start, region_end - region_start); - if (ret) { - msg_cerr("Verifying flash. Write failed for range %#"PRIx32" : %#"PRIx32", Abort.\n", - region_start, region_end); - goto _end; - } _end: memcpy(newcontents + region_start, old_start_buf, old_start - region_start); -- To view, visit https://review.coreboot.org/c/flashrom/+/79324?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I638835facd9311979c4991cc4ca41a4b9e174bd5 Gerrit-Change-Number: 79324 Gerrit-PatchSet: 1 Gerrit-Owner: Carly Zlabek <carlyzlabek(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[main]: buspirate_spi: Add support for hiz output with pullups=off
by Dreg (Code Review) 28 Nov '23

28 Nov '23

Dreg has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/79299?usp=email ) Change subject: buspirate_spi: Add support for hiz output with pullups=off ...................................................................... buspirate_spi: Add support for hiz output with pullups=off When working with low-voltage chips, the internal 10k pull-ups of the Bus Pirate might be too high. In such cases, it's necessary to create an external pull-up using lower-value resistors. For this, you can use the ``hiz`` parameter. This way, the Bus Pirate will operate as an open drain Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855 Signed-off-by: Dreg <dreg(a)rootkit.es> --- M buspirate_spi.c M doc/classic_cli_manpage.rst 2 files changed, 22 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/99/79299/1 diff --git a/buspirate_spi.c b/buspirate_spi.c index 72c28b0..7436d80 100644 --- a/buspirate_spi.c +++ b/buspirate_spi.c @@ -325,6 +325,7 @@ int spispeed = 0x7; int serialspeed_index = -1; int ret = 0; + bool hiz = false; bool pullup = false; bool psu = false; bool aux = true; @@ -368,6 +369,17 @@ } free(tmp); + tmp = extract_programmer_param_str(cfg, "hiz"); + if (tmp) { + if (strcasecmp("on", tmp) == 0) + hiz = true; + else if (strcasecmp("off", tmp) == 0) + ; // ignore + else + msg_perr("Invalid hiz state, not using them.\n"); + } + free(tmp); + tmp = extract_programmer_param_str(cfg, "pullups"); if (tmp) { if (strcasecmp("on", tmp) == 0) @@ -692,9 +704,9 @@ /* Set SPI config: output type, idle, clock edge, sample */ bp_commbuf[0] = 0x80 | 0xa; - if (pullup) { + if (pullup || hiz) { bp_commbuf[0] &= ~(1 << 3); - msg_pdbg("Pull-ups enabled, so using HiZ pin output! (Open-Drain mode)\n"); + msg_pdbg("Pull-ups or HiZ enabled, so using HiZ pin output! (Open-Drain mode)\n"); } ret = buspirate_sendrecv(bp_commbuf, 1, 1); if (ret) diff --git a/doc/classic_cli_manpage.rst b/doc/classic_cli_manpage.rst index 5d6b7ef..3157793 100644 --- a/doc/classic_cli_manpage.rst +++ b/doc/classic_cli_manpage.rst @@ -763,6 +763,14 @@ where ``baud`` can be ``115200``, ``230400``, ``250000`` or ``2000000`` (``2M``). The default is ``2M`` baud for Bus Pirate hardware version 3.0 and greater, and 115200 otherwise. +When working with low-voltage chips, the internal 10k pull-ups of the Bus Pirate might be too high. In such cases, it's +necessary to create an external pull-up using lower-value resistors. For this, you can use the ``hiz`` parameter. +This way, the Bus Pirate will operate as an open drain. Syntax is:: + + flashrom -p buspirate_spi:hiz=state + +where ``state`` can be ``on`` or ``off``. + An optional pullups parameter specifies the use of the Bus Pirate internal pull-up resistors. This may be needed if you are working with a flash ROM chip that you have physically removed from the board. Syntax is:: -- To view, visit https://review.coreboot.org/c/flashrom/+/79299?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I6d9909b23782ef3fcb1428fd127dae0d6b211855 Gerrit-Change-Number: 79299 Gerrit-PatchSet: 1 Gerrit-Owner: Dreg <dreg(a)fr33project.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[main]: flashrom_tester: Fix partial_lock_test on libflashrom
by Hsuan-ting Chen (Code Review) 28 Nov '23

28 Nov '23

Attention is currently required from: Evan Benn. Hsuan-ting Chen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79304?usp=email ) Change subject: flashrom_tester: Fix partial_lock_test on libflashrom ...................................................................... Patch Set 2: (5 comments) Patchset: PS1: Thanks for the prompt code review! > I think this fix is fine in the mean time, but obviously the write itself should fail if it is being prevented due to write protect, not the verify. I can confirm that this is the current ChromeOS behavior. I tested it by: * Disable HWWP * Enable SWWP * Enable HWWP * Write all zero to GBB with `flashrom -p internal -i GBB -w GBB.zero.bin` and expect a fail * Write again with `-n`: `flashrom -p internal -i GBB -w GBB.zero.bin -n` and expect a success * Check that GBB was not changed > Did this regress at some point? I cannot tell when this behavior changed, but the current behavior in ChromeOS is that "Write without verification will not fail." > I remember there were differences here between chromeos and upstream flashrom, have those diverged more or are in sync now? I'll have a look. I can't observe any for now. --- However, regardless of the behavior, it makes more sense to run libflashrom and the flashrom binary with exactly the same flags. File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/79304/comment/43a65e1d_e9d824d6 : PS1, Line 299: // flashrom cli has its own default flags. > `flashrom cli sets these flags by default` Done File util/flashrom_tester/flashrom/src/flashromlib.rs: https://review.coreboot.org/c/flashrom/+/79304/comment/70affd63_2d97f9f8 : PS1, Line 200: .flag_set(FlashromFlag::FlashromFlagSkipUnreadableRegions, true); > I don't see these last two being set in cli_classic. […] Those should be CrOS only default value. Let me remove them (and land only in CrOS repo: [CL](https://chromium-review.googlesource.com/c/chromiumos/third_party/flash…) File util/flashrom_tester/flashrom/src/lib.rs: https://review.coreboot.org/c/flashrom/+/79304/comment/cf91d477_8100c483 : PS1, Line 166: /// Set default flashrom flag if necessary. > `Set flags to the defaults used by the flashrom cli` Done File util/flashrom_tester/src/tester.rs: https://review.coreboot.org/c/flashrom/+/79304/comment/91aef8a9_deee5e9e : PS1, Line 87: info!("Setup default flashrom flag(s)"); > I don't think this is a necessary log Done -- To view, visit https://review.coreboot.org/c/flashrom/+/79304?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2 Gerrit-Change-Number: 79304 Gerrit-PatchSet: 2 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)gmail.com> Gerrit-Comment-Date: Tue, 28 Nov 2023 06:48:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Evan Benn <evanbenn(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashrom_tester: Fix partial_lock_test on libflashrom
by Hsuan-ting Chen (Code Review) 28 Nov '23

28 Nov '23

Attention is currently required from: Hsuan-ting Chen. Hello Evan Benn, Hsuan Ting Chen, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/79304?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: flashrom_tester: Fix partial_lock_test on libflashrom ...................................................................... flashrom_tester: Fix partial_lock_test on libflashrom partial_lock_test (Lock_top_quad, Lock_bottom_quad, Lock_bottom_half, and Lock_top_half) tries to: 1. Disable HWWP 2. Lock partial 3. Enable HWWP 4. Try to write the locked partial and expect a failure ... The 4th step only works on flashrom binary since the binary will set FLASHROM_FLAG_VERIFY_AFTER_WRITE=1 by default and it will error out while verifying. But libflashrom does not set any flag beforehand, so it has FLASHROM_FLAG_VERIFY_AFTER_WRITE=0 and thus it will think the write command works normally and raise no error. This causes the issue that flashrom_tester with libflashrom has been failed until today. To solve this issue, there are two solutions: 1. Take care of the default flags in libflashrom 2. Always pass --noverify to flashrom binary and verify it afterwards. To make both methods more consistent, I fix it with approach 1. BUG=b:304439294 BRANCH=none TEST=flashrom_tester internal --libflashrom Lock_top_quad Change-Id: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2 Signed-off-by: roccochen(a)chromium.com <roccochen(a)chromium.org> --- M bindings/rust/libflashrom/src/lib.rs 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, 66 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/79304/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/79304?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2 Gerrit-Change-Number: 79304 Gerrit-PatchSet: 2 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[main]: flashrom_tester: Fix partial_lock_test on libflashrom
by Evan Benn (Code Review) 28 Nov '23

28 Nov '23

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79304?usp=email ) Change subject: flashrom_tester: Fix partial_lock_test on libflashrom ...................................................................... Patch Set 1: (5 comments) Patchset: PS1: I vaguely recall many discussions about this, but can't recall the details enough. I think this fix is fine in the mean time, but obviously the write itself should fail if it is being prevented due to write protect, not the verify. Did this regress at some point? I remember there were differences here between chromeos and upstream flashrom, have those diverged more or are in sync now? File util/flashrom_tester/flashrom/src/cmd.rs: https://review.coreboot.org/c/flashrom/+/79304/comment/37b51cb8_0151f056 : PS1, Line 299: // flashrom cli has its own default flags. `flashrom cli sets these flags by default` File util/flashrom_tester/flashrom/src/flashromlib.rs: https://review.coreboot.org/c/flashrom/+/79304/comment/a668b391_406d2008 : PS1, Line 200: .flag_set(FlashromFlag::FlashromFlagSkipUnreadableRegions, true); I don't see these last two being set in cli_classic.c File util/flashrom_tester/flashrom/src/lib.rs: https://review.coreboot.org/c/flashrom/+/79304/comment/bbe08770_28c7fdae : PS1, Line 166: /// Set default flashrom flag if necessary. `Set flags to the defaults used by the flashrom cli` File util/flashrom_tester/src/tester.rs: https://review.coreboot.org/c/flashrom/+/79304/comment/dc5c9380_d08fd0c8 : PS1, Line 87: info!("Setup default flashrom flag(s)"); I don't think this is a necessary log -- To view, visit https://review.coreboot.org/c/flashrom/+/79304?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2 Gerrit-Change-Number: 79304 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nikolai Artemiev <nartemiev(a)chromium.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Comment-Date: Mon, 27 Nov 2023 22:42:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashrom_tester: Drop technical debt
by Evan Benn (Code Review) 28 Nov '23

28 Nov '23

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen. Evan Benn has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79303?usp=email ) Change subject: flashrom_tester: Drop technical debt ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/79303?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1ea174c1760fee3e41a2ef0e9779badf0c51298d Gerrit-Change-Number: 79303 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Comment-Date: Mon, 27 Nov 2023 22:16:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashrom_tester: Fix partial_lock_test on libflashrom
by Hsuan-ting Chen (Code Review) 27 Nov '23

27 Nov '23

Attention is currently required from: Evan Benn, Hsuan Ting Chen. Hello Hsuan Ting Chen, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/79304?usp=email to review the following change. Change subject: flashrom_tester: Fix partial_lock_test on libflashrom ...................................................................... flashrom_tester: Fix partial_lock_test on libflashrom partial_lock_test (Lock_top_quad, Lock_bottom_quad, Lock_bottom_half, and Lock_top_half) tries to: 1. Disable HWWP 2. Lock partial 3. Enable HWWP 4. Try to write the locked partial and expect a failure ... The 4th step only works on flashrom binary since the binary will set FLASHROM_FLAG_VERIFY_AFTER_WRITE=1 by default and it will error out while verifying. But libflashrom does not set any flag beforehand, so it has FLASHROM_FLAG_VERIFY_AFTER_WRITE=0 and thus it will think the write command works normally and raise no error. This causes the issue that flashrom_tester with libflashrom has been failed until today. To solve this issue, there are two solutions: 1. Take care of the default flags in libflashrom 2. Always pass --noverify to flashrom binary and verify it afterwards. To make both methods more consistent, I fix it with approach 1. BUG=b:304439294 BRANCH=none TEST=flashrom_tester internal --libflashrom Lock_top_quad Change-Id: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2 Signed-off-by: roccochen(a)chromium.com <roccochen(a)chromium.org> --- M bindings/rust/libflashrom/src/lib.rs 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, 74 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/79304/1 diff --git a/bindings/rust/libflashrom/src/lib.rs b/bindings/rust/libflashrom/src/lib.rs index 3f3fbc0..97da595 100644 --- a/bindings/rust/libflashrom/src/lib.rs +++ b/bindings/rust/libflashrom/src/lib.rs @@ -473,6 +473,43 @@ } } +/// A translation of the flashrom_flag type +/// +/// Keep this list in sync with libflashrom.h +#[derive(Clone, Debug, Eq, Hash, PartialEq)] +pub enum FlashromFlag { + FlashromFlagForce, + FlashromFlagForceBoardmismatch, + FlashromFlagVerifyAfterWrite, + FlashromFlagVerifyWholeChip, + FlashromFlagSkipUnreadableRegions, + FlashromFlagSkipUnwritableRegions, +} + +impl From<FlashromFlag> for libflashrom_sys::flashrom_flag { + fn from(e: FlashromFlag) -> Self { + match e { + FlashromFlag::FlashromFlagForce => libflashrom_sys::flashrom_flag::FLASHROM_FLAG_FORCE, + FlashromFlag::FlashromFlagForceBoardmismatch => { + libflashrom_sys::flashrom_flag::FLASHROM_FLAG_FORCE_BOARDMISMATCH + } + FlashromFlag::FlashromFlagVerifyAfterWrite => { + libflashrom_sys::flashrom_flag::FLASHROM_FLAG_VERIFY_AFTER_WRITE + } + FlashromFlag::FlashromFlagVerifyWholeChip => { + libflashrom_sys::flashrom_flag::FLASHROM_FLAG_VERIFY_WHOLE_CHIP + } + FlashromFlag::FlashromFlagSkipUnreadableRegions => { + libflashrom_sys::flashrom_flag::FLASHROM_FLAG_SKIP_UNREADABLE_REGIONS + } + FlashromFlag::FlashromFlagSkipUnwritableRegions => { + libflashrom_sys::flashrom_flag::FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS + } + e => panic!("Unexpected FlashromFlag: {:?}", e), + } + } +} + impl Chip { /// Probe for a chip /// @@ -705,6 +742,11 @@ }) } } + + /// Set a flag in the given flash context + pub fn flag_set(&mut self, flag: FlashromFlag, value: bool) -> () { + unsafe { libflashrom_sys::flashrom_flag_set(self.ctx.as_mut(), flag.into(), value) } + } } impl Drop for Chip { diff --git a/util/flashrom_tester/flashrom/src/cmd.rs b/util/flashrom_tester/flashrom/src/cmd.rs index 1f13a8e..43c4af8 100644 --- a/util/flashrom_tester/flashrom/src/cmd.rs +++ b/util/flashrom_tester/flashrom/src/cmd.rs @@ -294,6 +294,10 @@ fn can_control_hw_wp(&self) -> bool { self.fc.can_control_hw_wp() } + + fn set_default_flags(&self) -> () { + // flashrom cli has its own default flags. + } } fn flashrom_decode_opts(opts: FlashromOpt) -> Vec<OsString> { diff --git a/util/flashrom_tester/flashrom/src/flashromlib.rs b/util/flashrom_tester/flashrom/src/flashromlib.rs index 5e1747b..efbf968 100644 --- a/util/flashrom_tester/flashrom/src/flashromlib.rs +++ b/util/flashrom_tester/flashrom/src/flashromlib.rs @@ -32,7 +32,7 @@ // Software Foundation. // -use libflashrom::{Chip, Programmer}; +use libflashrom::{Chip, FlashromFlag, Programmer}; use std::{cell::RefCell, convert::TryFrom, fs, path::Path}; @@ -181,4 +181,25 @@ fn can_control_hw_wp(&self) -> bool { self.fc.can_control_hw_wp() } + + fn set_default_flags(&self) -> () { + self.flashrom + .borrow_mut() + .flag_set(FlashromFlag::FlashromFlagForce, false); + self.flashrom + .borrow_mut() + .flag_set(FlashromFlag::FlashromFlagForceBoardmismatch, false); + self.flashrom + .borrow_mut() + .flag_set(FlashromFlag::FlashromFlagVerifyAfterWrite, true); + self.flashrom + .borrow_mut() + .flag_set(FlashromFlag::FlashromFlagVerifyWholeChip, true); + self.flashrom + .borrow_mut() + .flag_set(FlashromFlag::FlashromFlagSkipUnreadableRegions, true); + self.flashrom + .borrow_mut() + .flag_set(FlashromFlag::FlashromFlagSkipUnwritableRegions, true); + } } diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs index e2c9149..887e369 100644 --- a/util/flashrom_tester/flashrom/src/lib.rs +++ b/util/flashrom_tester/flashrom/src/lib.rs @@ -162,4 +162,7 @@ /// Return true if the hardware write protect of this flash can be controlled. fn can_control_hw_wp(&self) -> bool; + + /// Set default flashrom flag if necessary. + fn set_default_flags(&self) -> (); } diff --git a/util/flashrom_tester/src/tester.rs b/util/flashrom_tester/src/tester.rs index 4629c2e..323b22b 100644 --- a/util/flashrom_tester/src/tester.rs +++ b/util/flashrom_tester/src/tester.rs @@ -84,6 +84,9 @@ layout_file: create_layout_file(rom_sz, Path::new("/tmp/"), print_layout), }; + info!("Setup default flashrom flag(s)"); + out.cmd.set_default_flags(); + info!("Stashing golden image for verification/recovery on completion"); out.cmd.read_into_file(&out.original_flash_contents)?; out.cmd.verify_from_file(&out.original_flash_contents)?; -- To view, visit https://review.coreboot.org/c/flashrom/+/79304?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7a8ac0c0984fef3cd9e73ed8d8097ddf429e54b2 Gerrit-Change-Number: 79304 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Evan Benn <evanbenn(a)gmail.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[main]: flashrom_tester: Drop technical debt
by Hsuan-ting Chen (Code Review) 27 Nov '23

27 Nov '23

Attention is currently required from: Hsuan Ting Chen. Hello Hsuan Ting Chen, I'd like you to do a code review. Please visit https://review.coreboot.org/c/flashrom/+/79303?usp=email to review the following change. Change subject: flashrom_tester: Drop technical debt ...................................................................... flashrom_tester: Drop technical debt Drop `-p host` ChromeOS miss-feature and use `-p internal` instead. BUG=b:296978620 BRANCH=none TEST=run flashrom-tester with newer flashrom Change-Id: I1ea174c1760fee3e41a2ef0e9779badf0c51298d Signed-off-by: roccochen(a)chromium.com <roccochen(a)chromium.org> --- M util/flashrom_tester/flashrom/src/lib.rs M util/flashrom_tester/src/main.rs M util/flashrom_tester/src/tests.rs 3 files changed, 7 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/79303/1 diff --git a/util/flashrom_tester/flashrom/src/lib.rs b/util/flashrom_tester/flashrom/src/lib.rs index 41393e8..e2c9149 100644 --- a/util/flashrom_tester/flashrom/src/lib.rs +++ b/util/flashrom_tester/flashrom/src/lib.rs @@ -51,19 +51,19 @@ #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum FlashChip { - HOST, + INTERNAL, } impl FlashChip { pub fn from(s: &str) -> Result<FlashChip, &str> { match s { - "host" => Ok(FlashChip::HOST), + "internal" => Ok(FlashChip::INTERNAL), _ => Err("cannot convert str to enum"), } } pub fn to(fc: FlashChip) -> &'static str { match fc { - FlashChip::HOST => "host", + FlashChip::INTERNAL => "internal", } } @@ -80,7 +80,7 @@ /// disabled. pub fn can_control_hw_wp(&self) -> bool { match self { - FlashChip::HOST => true, + FlashChip::INTERNAL => true, } } } diff --git a/util/flashrom_tester/src/main.rs b/util/flashrom_tester/src/main.rs index b8a2581..3035ae5 100644 --- a/util/flashrom_tester/src/main.rs +++ b/util/flashrom_tester/src/main.rs @@ -82,7 +82,7 @@ .arg( Arg::with_name("ccd_target_type") .required(true) - .possible_values(&["host"]), + .possible_values(&["internal"]), ) .arg( Arg::with_name("print-layout") diff --git a/util/flashrom_tester/src/tests.rs b/util/flashrom_tester/src/tests.rs index 721a789..d1e9d61 100644 --- a/util/flashrom_tester/src/tests.rs +++ b/util/flashrom_tester/src/tests.rs @@ -219,8 +219,8 @@ /// is actually able to write to the Flash. This only makes sense for chips /// running Coreboot, which we assume is just host. fn elog_sanity_test(env: &mut TestEnv) -> TestResult { - if env.chip_type() != FlashChip::HOST { - info!("Skipping ELOG sanity check for non-host chip"); + if env.chip_type() != FlashChip::INTERNAL { + info!("Skipping ELOG sanity check for non-internal chip"); return Ok(()); } // flash should be back in the golden state -- To view, visit https://review.coreboot.org/c/flashrom/+/79303?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I1ea174c1760fee3e41a2ef0e9779badf0c51298d Gerrit-Change-Number: 79303 Gerrit-PatchSet: 1 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-MessageType: newchange

1 0

[XS] Change in flashrom[main]: Makefile: Fix version string for non-Git builds
by Anton Samsonov (Code Review) 27 Nov '23

27 Nov '23

Attention is currently required from: Anton Samsonov, Thomas Heijligen. Anton Samsonov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79152?usp=email ) Change subject: Makefile: Fix version string for non-Git builds ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: > Jenkins derailed? […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/79152?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I8694e618878823a9e96b1f2bcfa63f6c71d3c2ed Gerrit-Change-Number: 79152 Gerrit-PatchSet: 5 Gerrit-Owner: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anton Samsonov <devel(a)zxlab.ru> Gerrit-Comment-Date: Mon, 27 Nov 2023 10:51:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anton Samsonov <contact-launchpad(a)zxlab.ru> Gerrit-MessageType: comment

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit