Attention is currently required from: Aarya, Carly Zlabek, Vincent Fazio.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79354?usp=email )
Change subject: erasure_layout: Remove redundant verifications from `erase_write`
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> Anastasia, do you prefer tracking issues in the flashrom redmine, the github mirror, or somewhere el […]
We track issues in flashrom redmine, thank you so much! You are very welcome to add tickets, or comment on existing ones.
Github is only a mirror, and it should be an auto-responder to issues and PRs which tells you to use redmine and gerrit (hopefully the auto-responder still works)
--
To view, visit https://review.coreboot.org/c/flashrom/+/79354?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: 79354
Gerrit-PatchSet: 2
Gerrit-Owner: Carly Zlabek <carlyzlabek(a)gmail.com>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Vincent Fazio <vfazio(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Carly Zlabek <carlyzlabek(a)gmail.com>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Vincent Fazio <vfazio(a)gmail.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 22:14:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Vincent Fazio <vfazio(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: David Wu, Edward O'Callaghan, Evan Benn, Hsuan Ting Chen.
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 3:
(2 comments)
File util/flashrom_tester/flashrom/src/flashromlib.rs:
https://review.coreboot.org/c/flashrom/+/79304/comment/51359454_48fb908f :
PS1, Line 200: .flag_set(FlashromFlag::FlashromFlagSkipUnreadableRegions, true);
> These two are to handle the case of the CSME locking parts of flash. […]
IIUC we have to set all the values in Default? So I set them as false (boolean deafult value, also align with what classic_cli works
File util/flashrom_tester/flashrom/src/flashromlib.rs:
https://review.coreboot.org/c/flashrom/+/79304/comment/57cac227_442f2760 :
PS2, Line 185: 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);
: }
> You probably mean to be using the `Default` trait for the `FlashromFlag` type? Since `enum`'s are un […]
Done
Create another struct FlashromFlags and set its Default function.
--
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: 3
Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Evan Benn <evanbenn(a)gmail.com>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Comment-Date: Thu, 14 Dec 2023 08:02:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Evan Benn <evanbenn(a)gmail.com>
Comment-In-Reply-To: Hsuan-ting Chen <roccochen(a)google.com>
Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: David Wu, Evan Benn, Hsuan Ting Chen, Hsuan-ting Chen.
Hello David Wu, Edward O'Callaghan, 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 (#3).
The following approvals got outdated and were removed:
Code-Review+1 by David Wu, 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/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
6 files changed, 123 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/04/79304/3
--
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: 3
Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
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: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Evan Benn <evanbenn(a)gmail.com>
Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Anastasia Klimchuk, David Wu, Hsuan-ting Chen, Kapil Porwal, Lean Sheng Tan, Nikolai Artemiev, Stefan Reinauer, Subrata Banik, Tyler Wang.
Hello Anastasia Klimchuk, David Wu, Hsuan-ting Chen, Kapil Porwal, Lean Sheng Tan, Nikolai Artemiev, Stefan Reinauer, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/79460?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review+1 by Hsuan-ting Chen, Verified+1 by build bot (Jenkins)
Change subject: flashchips: Add write-protect support for GD25LQ255E
......................................................................
flashchips: Add write-protect support for GD25LQ255E
datasheet: https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20221129/DS-0…
BUG=b:311336475
TEST=none
Change-Id: I1425e931433c00caceaabc6037a79099d6d5eac5
Signed-off-by: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
---
M flashchips.c
1 file changed, 11 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/60/79460/4
--
To view, visit https://review.coreboot.org/c/flashrom/+/79460?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: I1425e931433c00caceaabc6037a79099d6d5eac5
Gerrit-Change-Number: 79460
Gerrit-PatchSet: 4
Gerrit-Owner: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com>
Gerrit-Attention: Subrata Banik <subratabanik(a)google.com>
Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Tyler Wang <tyler.wang(a)quanta.corp-partner.google.com>
Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: Aarya, Carly Zlabek.
Vincent Fazio has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79354?usp=email )
Change subject: erasure_layout: Remove redundant verifications from `erase_write`
......................................................................
Patch Set 2:
(1 comment)
Patchset:
PS1:
> I am marking this comment as resolved, because taking care of `get_flash_region` situation will be i […]
Anastasia, do you prefer tracking issues in the flashrom redmine, the github mirror, or somewhere else? There are a couple of things I'd like to jot down.
--
To view, visit https://review.coreboot.org/c/flashrom/+/79354?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: 79354
Gerrit-PatchSet: 2
Gerrit-Owner: Carly Zlabek <carlyzlabek(a)gmail.com>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Vincent Fazio <vfazio(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Carly Zlabek <carlyzlabek(a)gmail.com>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 01:20:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Vincent Fazio <vfazio(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Aarya, Carly Zlabek, Vincent Fazio.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79354?usp=email )
Change subject: erasure_layout: Remove redundant verifications from `erase_write`
......................................................................
Patch Set 2: Code-Review+2
(2 comments)
Patchset:
PS1:
> That's a good observation about using `get_flash_region` with potential shorter end range. […]
I am marking this comment as resolved, because taking care of `get_flash_region` situation will be in a separate patch, this patch is independent and does not need to wait.
Patchset:
PS2:
> In addition to the testing results posted above, the meson tests completed with no issues.
Looks great! Thank you so much for your work!
--
To view, visit https://review.coreboot.org/c/flashrom/+/79354?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: 79354
Gerrit-PatchSet: 2
Gerrit-Owner: Carly Zlabek <carlyzlabek(a)gmail.com>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Vincent Fazio <vfazio(a)gmail.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Carly Zlabek <carlyzlabek(a)gmail.com>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Vincent Fazio <vfazio(a)gmail.com>
Gerrit-Comment-Date: Thu, 14 Dec 2023 01:15:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Carly Zlabek <carlyzlabek(a)gmail.com>
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Vincent Fazio <vfazio(a)gmail.com>
Gerrit-MessageType: comment
Attention is currently required from: Arthur Heymans, Edward O'Callaghan, Paul Menzel.
Brian Norris has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/79060?usp=email )
Change subject: fmap: Update major/minor version check
......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/79060/comment/b8e16748_4ca6a9cf :
PS2, Line 7: Update
> Maybe more specific: […]
Except, it does more than that. There's only so much one can fit in a commit subject.
--
To view, visit https://review.coreboot.org/c/flashrom/+/79060?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: I984835579d3b257a2462906f1f5091b179891bd0
Gerrit-Change-Number: 79060
Gerrit-PatchSet: 2
Gerrit-Owner: Brian Norris <briannorris(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Hsuan Ting Chen <roccochen(a)chromium.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Comment-Date: Thu, 14 Dec 2023 00:15:08 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-MessageType: comment