October 2024 - flashrom-gerrit

[S] Change in flashrom[main]: WIP: flashchips: add Winbond W25R512NWEIQ
by Alexandru Stan (Code Review) 12 Oct '24

12 Oct '24

Alexandru Stan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84752?usp=email ) Change subject: WIP: flashchips: add Winbond W25R512NWEIQ ...................................................................... WIP: flashchips: add Winbond W25R512NWEIQ I don't actually have a datasheet for this (so feel free to reject this CL), I just bruteforced it by looking at the chip IDs and knowing it's a 64M chip. I used W25Q256JW as a template and just increased every erase size calculation. I tested it by running: dd if=/dev/urandom of=/tmp/random.bin bs=1M count=64 sudo /tmp/flashrom/build/flashrom -p ft2232_spi:type=2232H -w /tmp/random.bin --progress sudo /tmp/flashrom/build/flashrom -p ft2232_spi:type=2232H -v /tmp/random.bin And I saw "Verifying flash... VERIFIED." PS: Yes I know I probably overvolted the chip by giving it 3.3V Change-Id: Ibf670e4014a22e4636789768b759cb51f75cd046 Signed-off-by: Alexandru M Stan <ams(a)frame.work> --- M flashchips.c M include/flashchips.h 2 files changed, 47 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/52/84752/1 diff --git a/flashchips.c b/flashchips.c index 02eefa4..c294e63 100644 --- a/flashchips.c +++ b/flashchips.c @@ -20227,6 +20227,52 @@ { .vendor = "Winbond", + .name = "W25R512NWEIQ", + .bustype = BUS_SPI, + .manufacture_id = WINBOND_NEX_ID, + .model_id = WINBOND_NEX_W25R512NWEIQ, + .total_size = 65536, + .page_size = 256, + /* supports SFDP */ + /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA, + .tested = TEST_OK_PREW, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = SPI_BLOCK_ERASE_21, + }, { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 2048} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_DC, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {64 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {64 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, + .unlock = SPI_DISABLE_BLOCKPROTECT, + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, + .voltage = {1650, 1950}, + }, + + { + .vendor = "Winbond", .name = "W25Q256JW_DTR", .bustype = BUS_SPI, .manufacture_id = WINBOND_NEX_ID, diff --git a/include/flashchips.h b/include/flashchips.h index 5f3bed0..3c781c7 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -1024,6 +1024,7 @@ #define WINBOND_NEX_W25Q64_W 0x6017 /* W25Q64DW; W25Q64FV in QPI mode */ #define WINBOND_NEX_W25Q128_W 0x6018 /* W25Q128FW; W25Q128FV in QPI mode */ #define WINBOND_NEX_W25Q256_W 0x6019 /* W25Q256JW */ +#define WINBOND_NEX_W25R512NWEIQ 0x6020 #define WINBOND_NEX_W25Q16JV_M 0x7015 /* W25Q16JV_M (QE=0) */ #define WINBOND_NEX_W25Q32JV_M 0x7016 /* W25Q32JV_M (QE=0) */ #define WINBOND_NEX_W25Q64JV 0x7017 /* W25Q64JV */ -- To view, visit https://review.coreboot.org/c/flashrom/+/84752?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ibf670e4014a22e4636789768b759cb51f75cd046 Gerrit-Change-Number: 84752 Gerrit-PatchSet: 1 Gerrit-Owner: Alexandru Stan <ams(a)frame.work>

1 0

[S] Change in flashrom[main]: erasure_layout: Add an option to sacrifice unchanged blocks for speed
by Bill XIE (Code Review) 11 Oct '24

11 Oct '24

Attention is currently required from: Aarya. Hello Aarya, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84721?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: erasure_layout: Add an option to sacrifice unchanged blocks for speed ...................................................................... erasure_layout: Add an option to sacrifice unchanged blocks for speed Someone may prefer to sacrifice sub-blocks needless to change within a larger erase block for programming speed. These people who are okay to fry their chip sooner can set this option. If the percentage of sub-blocks needless to change within a larger erase block is lower than the given value, the whole larger erase block will be erased. Change-Id: I154e8a713f626c37dbbe118db700055b96d24803 Signed-off-by: persmule <persmule(a)hardenedlinux.org> --- M cli_classic.c M erasure_layout.c M include/flash.h 3 files changed, 25 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/84721/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84721?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I154e8a713f626c37dbbe118db700055b96d24803 Gerrit-Change-Number: 84721 Gerrit-PatchSet: 2 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>

1 0

[L] Change in flashrom[main]: Complete and fix progress feature implementation for all operations
by Anastasia Klimchuk (Code Review) 11 Oct '24

11 Oct '24

Attention is currently required from: Aarya, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/flashrom/+/84102?usp=email ) Change subject: Complete and fix progress feature implementation for all operations ...................................................................... Patch Set 9: (7 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/84102/comment/d4f2fa6b_9142155a?us… : PS7, Line 42: The progress doesn't just dump lots of stuff on the screen which : probably won't fly because of CB:64668, but it's not hard to adjust : this. > ```suggestion […] Done File cli_output.c: https://review.coreboot.org/c/flashrom/+/84102/comment/29613789_74bdf5df?us… : PS4, Line 119: 16 * FLASHROM_PROGRESS_NR > Just saw that "..." is also printed below which needs to be erased as well. Maybe this is where your extra 3 chars went! (you said: it will be 13 chars which I rounded to 16). Because, when running, `...` does not multiply, it seems to be erased as well. File cli_output.c: https://review.coreboot.org/c/flashrom/+/84102/comment/10e17709_05349c1f?us… : PS7, Line 157: progress_printed = false; > Those extra newlines are quite annoying. I think most of them can be deal with quite easily. […] Yes that's a good improvement. I re-wrote, and did the same tests with dummy (with normal level of verbosity and -VVV), seems to work. File libflashrom.c: https://review.coreboot.org/c/flashrom/+/84102/comment/452e1f9a_e15a1294?us… : PS7, Line 82: update_progress(flashctx, stage, 0); > Probably worth commenting that this is used to trigger callback invocation. Done File realtek_mst_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/84102/comment/6f5378d1_657ce56e?us… : PS7, Line 400: RTK_PAGE_SIZE > Looks like this should actually be `page_len`. Done File spi25.c: https://review.coreboot.org/c/flashrom/+/84102/comment/88215ffc_4b229a2a?us… : PS7, Line 733: chunksize > This should be `towrite`. Done File tests/chip.c: https://review.coreboot.org/c/flashrom/+/84102/comment/aa31decd_fbe8fefe?us… : PS7, Line 84: } > Doesn't look like this can catch progress going backward. […] Yeah this does not assert anything, just prints for a human to read in the logs. I meant this test as a starting point, to at least run the code (which is a bit better than just compiling). It checks that there are no segfaults or infinite loops, memory leaks, things like that - but no asserting for logic. I would do it later (perhaps in the next patch), what do you think? -- To view, visit https://review.coreboot.org/c/flashrom/+/84102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051 Gerrit-Change-Number: 84102 Gerrit-PatchSet: 9 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> 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: Peter Marheine <pmarheine(a)chromium.org> 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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Fri, 11 Oct 2024 11:52:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>

1 0

[L] Change in flashrom[main]: Complete and fix progress feature implementation for all operations
by Anastasia Klimchuk (Code Review) 11 Oct '24

11 Oct '24

Attention is currently required from: Aarya, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has uploaded a new patch set (#9) to the change originally created by Sergii Dmytruk. ( https://review.coreboot.org/c/flashrom/+/84102?usp=email ) The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: Complete and fix progress feature implementation for all operations ...................................................................... Complete and fix progress feature implementation for all operations Original progress reporting implemented in CB:49643 and it has some issues, for example: size_t start_address = start; size_t end_address = len - start; End address is anything but length minus start address. update_progress(flash, FLASHROM_PROGRESS_READ, /*current*/ start - start_address + to_read, /*total*/ end_address); Total should just be length if that's how current value is computed. --- libflashrom needs to know total size ahead of time. That's init_progress() and changed update_progress(). It also needs to store the last current value to be able to update it. That's stage_progress in flashrom_flashctx. Measuring accurately amount of data which will be read/erased/written isn't easy because things can be skipped as optimizations. The next patch in the chain aims to address this, there are TODO/FIXME comments there. --- CLI shares terminal with the rest of the code and has to maintain more state to handle that reasonably well. Similar to CB:64668, an effort is made to keep the progress on a single line. Non-progress output is kept track of to know when moving to a new line cannot be avoided. --- A script to test the CLI: \#!/bin/bash t=${1:-rewW} shift if [[ $t =~ r ]]; then echo ">>> READ" ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -r dump.rom --progress "$@" echo fi if [[ $t =~ e ]]; then echo ">>> ERASE" ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -E --progress "$@" echo fi if [[ $t =~ w ]]; then echo ">>> WRITE (without erase)" dd if=/dev/zero of=zero.rom bs=1M count=16 2> /dev/null ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -w zero.rom --progress "$@" echo fi if [[ $t =~ W ]]; then echo ">>> WRITE (with erase)" dd if=/dev/zero of=zero.rom bs=1M count=16 2> /dev/null dd if=/dev/random of=random.rom bs=1M count=16 2> /dev/null ./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz,image=random.rom -w zero.rom --progress "$@" echo fi Co-developed-by: Anastasia Klimchuk <aklm(a)flashrom.org> Co-developed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051 Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M 82802ab.c M at45db.c M cli_classic.c M cli_output.c M dediprog.c M doc/release_notes/devel.rst M en29lv640b.c M erasure_layout.c M flashrom.c M include/flash.h M it87spi.c M jedec.c M libflashrom.c M linux_mtd.c M parade_lspcon.c M realtek_mst_i2c_spi.c M spi.c M spi25.c M sst28sf040.c M tests/chip.c M tests/spi25.c M tests/tests.c M tests/tests.h 23 files changed, 304 insertions(+), 50 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/84102/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/84102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051 Gerrit-Change-Number: 84102 Gerrit-PatchSet: 9 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> 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: Peter Marheine <pmarheine(a)chromium.org> 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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>

1 0

[L] Change in flashrom[main]: Complete and fix progress feature implementation for all operations
by Anastasia Klimchuk (Code Review) 11 Oct '24

11 Oct '24

Attention is currently required from: Aarya, Nikolai Artemiev, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change by Sergii Dmytruk. ( https://review.coreboot.org/c/flashrom/+/84102?usp=email ) Change subject: Complete and fix progress feature implementation for all operations ...................................................................... Patch Set 8: (1 comment) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/84102/comment/0b1b701a_4a865fee?us… : PS7, Line 275: msg_cerr("ERASE FAILED!\n"); > Not added here, but this line is unreachable. Oh yes, I made CB:84725 -- To view, visit https://review.coreboot.org/c/flashrom/+/84102?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051 Gerrit-Change-Number: 84102 Gerrit-PatchSet: 8 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> 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: Peter Marheine <pmarheine(a)chromium.org> 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: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Fri, 11 Oct 2024 08:23:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com>

1 0

[XS] Change in flashrom[main]: erasure_layout: Fix unreachable error message
by Anastasia Klimchuk (Code Review) 11 Oct '24

11 Oct '24

Attention is currently required from: Aarya. Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84725?usp=email ) Change subject: erasure_layout: Fix unreachable error message ...................................................................... erasure_layout: Fix unreachable error message Change-Id: I11d2d1359f74475cb20a1c91bddb380b4952a704 Spotted-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M erasure_layout.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/84725/1 diff --git a/erasure_layout.c b/erasure_layout.c index ee90afa..7c76c7a 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -271,8 +271,8 @@ return -1; } if (check_erased_range(flashctx, start_addr, block_len)) { - return -1; msg_cerr("ERASE FAILED!\n"); + return -1; } update_progress(flashctx, FLASHROM_PROGRESS_ERASE, block_len); -- To view, visit https://review.coreboot.org/c/flashrom/+/84725?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I11d2d1359f74475cb20a1c91bddb380b4952a704 Gerrit-Change-Number: 84725 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>

1 0

[M] Change in flashrom[main]: flashchips: add GD25F64F
by Anastasia Klimchuk (Code Review) 11 Oct '24

11 Oct '24

Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim. Anastasia Klimchuk has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/84705?usp=email ) Change subject: flashchips: add GD25F64F ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/84705/comment/868ce68e_8f7cd4ed?us… : PS1, Line 10: Will send datasheet through email > All good, I will wait for you to send me and then review! Got the email, I did the review. You can remove this line from commit message. Thanks! -- To view, visit https://review.coreboot.org/c/flashrom/+/84705?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I07005f1589b76c8a61a1a744b16dc6b0c9020e11 Gerrit-Change-Number: 84705 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> 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: Victor Lim <vlim(a)gigadevice.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 11 Oct 2024 06:53:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: flashchips: add GD25F256F
by Anastasia Klimchuk (Code Review) 11 Oct '24

11 Oct '24

Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim. Anastasia Klimchuk has posted comments on this change by Victor Lim. ( https://review.coreboot.org/c/flashrom/+/84090?usp=email ) Change subject: flashchips: add GD25F256F ...................................................................... Patch Set 2: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/84090/comment/e1ad1602_618f5733?us… : PS2, Line 8056: reg_bits > We should leave the srp bits out of reg_bits if the chip doesn't have them, but I'm not sure how the […] The exact definition in the datasheet for STATUS1 bit 7 is "Reserved". Does it mean the value in this bit can be any? Victor maybe you know, what happens when we read STATUS1, what will be in bit 7? Or, how this model handles Status register protection bit ? -- To view, visit https://review.coreboot.org/c/flashrom/+/84090?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ibbbbb8a55adbcbc2ee1785782c4eb3771d50c167 Gerrit-Change-Number: 84090 Gerrit-PatchSet: 2 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com> 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: Victor Lim <vlim(a)gigadevice.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 11 Oct 2024 06:21:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[S] Change in flashrom[main]: erasure_layout: Add an option to sacrifice unchanged blocks for speed
by Bill XIE (Code Review) 11 Oct '24

11 Oct '24

Attention is currently required from: Aarya. Bill XIE has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84721?usp=email ) Change subject: erasure_layout: Add an option to sacrifice unchanged blocks for speed ...................................................................... erasure_layout: Add an option to sacrifice unchanged blocks for speed Someone may prefer to sacrifice sub-blocks needless to change within a larger erase block for programming speed. These people who are okay to fry their chip sooner can set this option. If the percentage of sub-blocks needless to change within a larger erase block is lower than the given value, the whole larger erase block will be erased. Change-Id: I154e8a713f626c37dbbe118db700055b96d24803 Signed-off-by: persmule <persmule(a)hardenedlinux.org> --- M cli_classic.c M erasure_layout.c M include/flash.h 3 files changed, 25 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/84721/1 diff --git a/cli_classic.c b/cli_classic.c index 3343438..dc51259 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -45,6 +45,7 @@ OPTION_WP_DISABLE, OPTION_WP_LIST, OPTION_PROGRESS, + OPTION_SACRIFICE_RATIO, }; struct cli_options { @@ -73,6 +74,7 @@ char *logfile; char *referencefile; const char *chip_to_probe; + int sacrifice_ratio; }; static void cli_classic_usage(const char *name) @@ -119,6 +121,10 @@ " --flash-contents <ref-file> assume flash contents to be <ref-file>\n" " -L | --list-supported print supported devices\n" " --progress show progress percentage on the standard output\n" + " --sacrifice-ratio <ratio> The percentage of overall sub-blocks needless to\n" + " change within a larger erase block to be sacrificed\n" + " for programming speed, default 0, max 100\n" + " DANGEROUS! It wears your chip harder!\n" " -p | --programmer <name>[:<param>] specify the programmer device. One of\n"); list_programmers_linebreak(4, 80, 0); printf(".\n\nYou can specify one of -h, -R, -L, " @@ -810,6 +816,10 @@ case OPTION_PROGRESS: options->show_progress = true; break; + case OPTION_SACRIFICE_RATIO: + /* It is okay to convert invalid input to 0. */ + options->sacrifice_ratio = atoi(optarg); + break; default: cli_classic_abort_usage(NULL); break; @@ -879,6 +889,7 @@ {"version", 0, NULL, 'R'}, {"output", 1, NULL, 'o'}, {"progress", 0, NULL, OPTION_PROGRESS}, + {"sacrifice-ratio", 1, NULL, OPTION_SACRIFICE_RATIO}, {NULL, 0, NULL, 0}, }; @@ -1125,6 +1136,14 @@ goto out_shutdown; } + if (options.sacrifice_ratio) { + if (options.sacrifice_ratio > 100) { + msg_ginfo("Invalid input, use default.\n"); + options.sacrifice_ratio = 0; + } + fill_flash->sacrifice_ratio = options.sacrifice_ratio; + } + if (options.ifd && (flashrom_layout_read_from_ifd(&options.layout, fill_flash, NULL, 0) || process_include_args(options.layout, options.include_args))) { ret = 1; diff --git a/erasure_layout.c b/erasure_layout.c index 7f3bb46..56da450 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -249,9 +249,10 @@ } const int total_blocks = sub_block_end - sub_block_start + 1; - if (count == total_blocks) { - /* We are selecting one large block instead, so send opcode once - * instead of sending many smaller ones. + if (total_blocks - count <= total_blocks * flashctx->sacrifice_ratio / 100) { + /* Number of smaller blocks needless to change is lower than the + * sacrifice ratio, we are selecting one large block instead, so + * send opcode once instead of sending many smaller ones. */ if (ll->start_addr >= rstart && ll->end_addr <= rend) { /* Deselect all smaller blocks covering the same region. */ diff --git a/include/flash.h b/include/flash.h index d0e55af..90a9201 100644 --- a/include/flash.h +++ b/include/flash.h @@ -587,6 +587,8 @@ /* Progress reporting */ flashrom_progress_callback *progress_callback; struct flashrom_progress *progress_state; + /* Sacrifice ratio */ + int sacrifice_ratio; }; /* Timing used in probe routines. ZERO is -2 to differentiate between an unset -- To view, visit https://review.coreboot.org/c/flashrom/+/84721?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I154e8a713f626c37dbbe118db700055b96d24803 Gerrit-Change-Number: 84721 Gerrit-PatchSet: 1 Gerrit-Owner: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com>

1 0

[S] Change in flashrom[main]: erase/write: Deselect all smaller blocks when large block is selected
by Bill XIE (Code Review) 11 Oct '24

11 Oct '24

Attention is currently required from: Aarya, Anastasia Klimchuk, Bill XIE. Bill XIE has uploaded a new patch set (#3) to the change originally created by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84686?usp=email ) The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: erase/write: Deselect all smaller blocks when large block is selected ...................................................................... erase/write: Deselect all smaller blocks when large block is selected Previously the logic which selected large block did deselect of smaller blocks, but only one level below. So some even smaller blocks could still remain selected, and this would result in duplicate erase. This patch deselects all smaller blocks of all levels below, down to the smallest size. If the area is covered by one large block, no other smaller blocks inside are needed. Change-Id: Icfc18d5c090b1dcb92ab157e2c139be71af59300 Spotted-by: persmule <persmule(a)hardenedlinux.org> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Co-authored-by: persmule <persmule(a)hardenedlinux.org> --- M erasure_layout.c 1 file changed, 31 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/84686/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/84686?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Icfc18d5c090b1dcb92ab157e2c139be71af59300 Gerrit-Change-Number: 84686 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2024