Attention is currently required from: DZ, Nikolai Artemiev, Stefan Reinauer.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81838?usp=email )
Change subject: flashchips: Add support for MXIC MX25R4035F
......................................................................
Patch Set 5:
(1 comment)
Patchset:
PS3:
> So there the other patch CB:81839 is ready, but they both modify the same line 544 in flashchips. […]
Okay another one got submitted, so this one only need to be rebased on top of head
--
To view, visit https://review.coreboot.org/c/flashrom/+/81838?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: I91dbc4735bf232e0b1dce72c7f06be967d35ebfb
Gerrit-Change-Number: 81838
Gerrit-PatchSet: 5
Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
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-Attention: DZ <danielzhang(a)mxic.com.cn>
Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Tue, 14 May 2024 06:16:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev, Peter Marheine.
Hello Aarya, Alexander Goncharov, Anastasia Klimchuk, Nikolai Artemiev, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/82393?usp=email
to look at the new patch set (#5).
Change subject: erasure_layout: Fix get_flash_region bug
......................................................................
erasure_layout: Fix get_flash_region bug
When flash regions are protected, erase could incorrectly erase regions
which were meant to be protected by requesting the correct size but
using an erase opcode with coarser granularity than desired (for
instance using a 16-byte erase command while attempting to erase only 8
bytes).
This fixes that by exchanging the nesting of the loops over erase blocks
and flash regions.
Old:
- Select erasefns
- Loop over blocks to erase for each selected erasefn
- Loop over programmer flash regions within erase block
- Erase regions (may fail since selected erasefn will be
too big if flash region is smaller than erase block)
New:
- Loop over programmer flash regions within erase block
- Select erasefns within programer flash region
- Loop over blocks to erase for each selected erasefn
- Erase regions
Eraser selection and erasing has also been factored out into a helper
function to manage nesting depth.
TEST=New test cases pass, whereas some of them fail without the changes
to erasure_layout.c
BUG=https://ticket.coreboot.org/issues/525
Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9
Co-authored-by: Nikolai Artemiev <nartemiev(a)google.com>
Co-authored-by: Anastasia Klimchuk <aklm(a)flashrom.org>
Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com>
Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org>
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
---
M erasure_layout.c
M tests/erase_func_algo.c
M tests/tests.c
M tests/tests.h
4 files changed, 441 insertions(+), 96 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/82393/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/82393?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: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9
Gerrit-Change-Number: 82393
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-MessageType: newpatchset
Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev, Peter Marheine.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email )
Change subject: erasure_layout: Fix get_flash_region bug
......................................................................
Patch Set 4:
(2 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/82393/comment/2016258a_6d59288d :
PS4, Line 40: Klimchunk
Klimchuk (typo :P) and also below :)
Patchset:
PS4:
This is great thank you so much!! I will review a bit later, but thank you for gathering all the work together!
there is one more smaller patch CB:81548 , but I inspected the changed lines and realised that there is no conflict. Somehow the two patches modify different lines, even though they basically fixing the same piece of logic. Top-pro work!
--
To view, visit https://review.coreboot.org/c/flashrom/+/82393?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: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9
Gerrit-Change-Number: 82393
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Tue, 14 May 2024 02:32:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81548?usp=email )
Change subject: erasure_layout: don't copy region buffers if they're null/zero-size
......................................................................
Patch Set 3: Code-Review+2
--
To view, visit https://review.coreboot.org/c/flashrom/+/81548?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: I99aad4119f9c11bea75e3419926cc833bc1f68c5
Gerrit-Change-Number: 81548
Gerrit-PatchSet: 3
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Comment-Date: Tue, 14 May 2024 02:25:17 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Aarya, Anastasia Klimchuk, Nikolai Artemiev.
Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81548?usp=email )
Change subject: erasure_layout: don't copy region buffers if they're null/zero-size
......................................................................
Patch Set 3:
(3 comments)
Commit Message:
https://review.coreboot.org/c/flashrom/+/81548/comment/aefb12d7_ac828f6c :
PS2, Line 10: the pointer is null
> I think from the meaning of the patch, you meant to say […]
Exactly, just a typo! Thanks, done.
File erasure_layout.c:
https://review.coreboot.org/c/flashrom/+/81548/comment/b062f6ac_31ec29d9 :
PS2, Line 263: size_t old_start_len
> This can be const, but also […]
You're right. Done
https://review.coreboot.org/c/flashrom/+/81548/comment/bd303596_142f4d28 :
PS2, Line 264: size_t old_end_len
> And this can be const and `end_buf_len`
Done
--
To view, visit https://review.coreboot.org/c/flashrom/+/81548?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: I99aad4119f9c11bea75e3419926cc833bc1f68c5
Gerrit-Change-Number: 81548
Gerrit-PatchSet: 3
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 13 May 2024 15:16:55 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev.
Hello Aarya, Anastasia Klimchuk, Nikolai Artemiev, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/81548?usp=email
to look at the new patch set (#3).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: erasure_layout: don't copy region buffers if they're null/zero-size
......................................................................
erasure_layout: don't copy region buffers if they're null/zero-size
memcpy() function expects 2nd parameter to be non-null. Make sure that
the pointer is non-null before passing it to the function.
Also move allocations under if conditions to avoid allocating memory for
a potentially 0 size.
Found-by: scan-build, clang v17.0.6
Signed-off-by: Alexander Goncharov <chat(a)joursoir.net>
Change-Id: I99aad4119f9c11bea75e3419926cc833bc1f68c5
---
M erasure_layout.c
1 file changed, 30 insertions(+), 24 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/81548/3
--
To view, visit https://review.coreboot.org/c/flashrom/+/81548?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: I99aad4119f9c11bea75e3419926cc833bc1f68c5
Gerrit-Change-Number: 81548
Gerrit-PatchSet: 3
Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)chromium.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>
Gerrit-MessageType: newpatchset
Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/81664?usp=email )
Change subject: [WIP] tests: Add tests for erasing chip with protected region
......................................................................
Patch Set 1:
(1 comment)
File tests/erase_func_algo.c:
https://review.coreboot.org/c/flashrom/+/81664/comment/bd4afe9a_dc610773 :
PS1, Line 130: TEST_ERASE_INJECTOR
> I have added comment into CB:82251
I've debugged this and think it's all correct, combined with the actual bugfix in CB:82393 because it seems like the fix should include regression tests.
--
To view, visit https://review.coreboot.org/c/flashrom/+/81664?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: I1e7cfd027d37437c917c054f30f3be59b76b0069
Gerrit-Change-Number: 81664
Gerrit-PatchSet: 1
Gerrit-Owner: Anastasia Klimchuk <aklm(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-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Comment-Date: Mon, 13 May 2024 05:28:03 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>
Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Aarya, Anastasia Klimchuk, Nikolai Artemiev.
Peter Marheine has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email )
Change subject: erasure_layout: Fix get_flash_region bug
......................................................................
Patch Set 4:
This change is ready for review.
--
To view, visit https://review.coreboot.org/c/flashrom/+/82393?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: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9
Gerrit-Change-Number: 82393
Gerrit-PatchSet: 4
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 13 May 2024 05:26:46 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Anastasia Klimchuk, Nikolai Artemiev.
Hello Anastasia Klimchuk, Nikolai Artemiev, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/82251?usp=email
to look at the new patch set (#5).
The following approvals got outdated and were removed:
Verified+1 by build bot (Jenkins)
Change subject: tests/erase: record the opcode for each erase
......................................................................
tests/erase: record the opcode for each erase
This allows tests to verify that the correct opcode is used when
erasing, which is required to unit-test the fix to issue #525 where in
some situations an incorrect erase opcode will be used.
BUG=https://ticket.coreboot.org/issues/525
Change-Id: I3983fe42c2e7f06668a1bd20d2db7fafa93b8043
Signed-off-by: Peter Marheine <pmarheine(a)chromium.org>
---
M flashrom.c
M include/flash.h
M tests/chip.c
M tests/erase_func_algo.c
M tests/meson.build
5 files changed, 49 insertions(+), 25 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/51/82251/5
--
To view, visit https://review.coreboot.org/c/flashrom/+/82251?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: I3983fe42c2e7f06668a1bd20d2db7fafa93b8043
Gerrit-Change-Number: 82251
Gerrit-PatchSet: 5
Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com>
Gerrit-MessageType: newpatchset