October 2024 - flashrom-gerrit

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

07 Oct '24

Attention is currently required from: Aarya, Bill XIE. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84686?usp=email ) Change subject: erase/write: Deselect all smaller blocks when large block is selected ...................................................................... Patch Set 1: (1 comment) File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/84686/comment/7b1c80a5_34ed9df3?us… : PS1, Line 237: /* Deselect all smaller blocks covering the same region. */ I still need to add a test for this in `tests/erase_func_algo.c` -- 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: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Icfc18d5c090b1dcb92ab157e2c139be71af59300 Gerrit-Change-Number: 84686 Gerrit-PatchSet: 1 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-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 07 Oct 2024 07:45:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

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

07 Oct '24

Attention is currently required from: Aarya, Bill XIE. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84686?usp=email ) Change subject: erase/write: Deselect all smaller blocks when large block is selected ...................................................................... Patch Set 1: (2 comments) Patchset: PS1: Run the test scenario from CB:84614 and that's the result: double-erase seems gone Reading old flash chip contents... read_flash: region (00000000..0xffffff) is readable, reading range (00000000..0xffffff). done. erase_write: region (00000000..0xffffff) is writable, erasing range (00000000..0xffffff). 000000..0xffffff verify_range: Verifying region (00000000..0xffffff) read_flash: region (00000000..0xffffff) is readable, reading range (00000000..0xffffff). E(0:ffffff)write_flash: region (00000000..0xffffff) is writable, writing range (00000000..0xffffff). W(0:ffffff)Erase/write done from 0 to ffffff Verifying flash... read_flash: region (00000000..0xffffff) is readable, reading range (00000000..0xffffff). VERIFIED. Writing chip.rom File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/84686/comment/127814fe_df505a33?us… : PS1, Line 242: while (true) I want to re-write this as recursive fn `deselect_erase_functions` -- 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: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Icfc18d5c090b1dcb92ab157e2c139be71af59300 Gerrit-Change-Number: 84686 Gerrit-PatchSet: 1 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-Attention: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 07 Oct 2024 07:44:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

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

07 Oct '24

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84686?usp=email ) 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. Change-Id: Icfc18d5c090b1dcb92ab157e2c139be71af59300 Spotted-by: persmule <persmule(a)hardenedlinux.org> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M erasure_layout.c 1 file changed, 22 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/86/84686/1 diff --git a/erasure_layout.c b/erasure_layout.c index c3a415b..ad333eb 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -229,9 +229,29 @@ const int total_blocks = sub_block_end - sub_block_start + 1; if (count && count > total_blocks/2) { + /* More than a half of the region is covered by smaller blocks already. + We are selecting one large block instead, to send opcode once + instead of sending many smaller once. */ if (ll->start_addr >= rstart && ll->end_addr <= rend) { - for (int j = sub_block_start; j <= sub_block_end; j++) - layout[findex - 1].layout_list[j].selected = false; + + /* Deselect all smaller blocks covering the same region. */ + size_t index_to_deselect = findex - 1; // represents size of the block + int block_start_to_deselect = sub_block_start; + int block_end_to_deselect = sub_block_end; + + while (true) { + for (int j = block_start_to_deselect; j <= block_end_to_deselect; j++) + layout[index_to_deselect].layout_list[j].selected = false; + + block_start_to_deselect = layout[index_to_deselect].layout_list[block_start_to_deselect].first_sub_block_index; + block_end_to_deselect = layout[index_to_deselect].layout_list[block_end_to_deselect].last_sub_block_index; + if (index_to_deselect) + index_to_deselect--; + else + break; // index_to_deselect has already reached 0, the smallest size of block. we are done. + } + + /* Select large block. */ ll->selected = true; } } -- 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: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Icfc18d5c090b1dcb92ab157e2c139be71af59300 Gerrit-Change-Number: 84686 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

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

06 Oct '24

Attention is currently required from: Aarya, Anastasia Klimchuk, Nikolai Artemiev. Sergii Dmytruk 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 7: (1 comment) Patchset: PS7: > Sergii, maybe you could have a look? Especially at the code which decides whether to replace previou […] Oh, and I don't think this needs to be tied to CB:84439. -- 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: 7 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sun, 06 Oct 2024 17:07:09 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

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

06 Oct '24

Attention is currently required from: Aarya, Anastasia Klimchuk, Nikolai Artemiev. Sergii Dmytruk 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 7: (8 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/84102/comment/8db69e36_d01915c2?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 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. ``` File cli_output.c: https://review.coreboot.org/c/flashrom/+/84102/comment/a76f2895_9a4d0ebd?us… : PS4, Line 119: 16 * FLASHROM_PROGRESS_NR > Oh this is a backspace that is not deleting characters! Sorry I got so confused. […] Just saw that "..." is also printed below which needs to be erased as well. File cli_output.c: https://review.coreboot.org/c/flashrom/+/84102/comment/d5fabcff_c745576a?us… : PS7, Line 157: progress_printed = false; Those extra newlines are quite annoying. I think most of them can be deal with quite easily. The flag could be an enumeration: - progress - newline - midline This simple check should address absolute majority of prints: ```c last_line = (fmt[strlen(fmt) - 1] == '\n' ? NEWLINE : MIDLINE); ``` Then both `msg_ginfo("\n");` above can be skipped if we know we're at the new line to turn ``` flashrom 1.5.0-devel (git:v1.4.0-57-g83954aa8) on Linux 6.10.11 (x86_64) flashrom is free software, get the source code at https://flashrom.org Found Winbond flash chip "W25Q128.V" (16384 kB, SPI) on dummy. [READ: 100%][ERASE: 100%]...Erase/write done from 0 to ffffff ``` into ``` flashrom 1.5.0-devel (git:v1.4.0-57-g83954aa8) on Linux 6.10.11 (x86_64) flashrom is free software, get the source code at https://flashrom.org Found Winbond flash chip "W25Q128.V" (16384 kB, SPI) on dummy. [READ: 100%][ERASE: 100%]...Erase/write done from 0 to ffffff ``` File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/84102/comment/ef44a75f_2c8b2286?us… : PS7, Line 275: msg_cerr("ERASE FAILED!\n"); Not added here, but this line is unreachable. File libflashrom.c: https://review.coreboot.org/c/flashrom/+/84102/comment/7ae97ec7_a6be4eed?us… : PS7, Line 82: update_progress(flashctx, stage, 0); Probably worth commenting that this is used to trigger callback invocation. File realtek_mst_i2c_spi.c: https://review.coreboot.org/c/flashrom/+/84102/comment/6b5b0fe2_0f696d61?us… : PS7, Line 400: RTK_PAGE_SIZE Looks like this should actually be `page_len`. File spi25.c: https://review.coreboot.org/c/flashrom/+/84102/comment/57f2adcc_0b5ced68?us… : PS7, Line 733: chunksize This should be `towrite`. File tests/chip.c: https://review.coreboot.org/c/flashrom/+/84102/comment/faa1aa14_d02457cd?us… : PS7, Line 84: } Doesn't look like this can catch progress going backward. If you want to check for that, this could verify that stage progress is either reset to 0 or increases while total amount changes only when current value is reset. Needs creating a state in tests to keep track of stages, basically what cli code does. -- 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: 7 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: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sun, 06 Oct 2024 17:06:04 +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

[S] Change in flashrom[main]: doc/contact: Add note to IRC section and calm down the formatting
by Stefan Reinauer (Code Review) 06 Oct '24

06 Oct '24

Attention is currently required from: Anastasia Klimchuk. Stefan Reinauer has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84679?usp=email ) Change subject: doc/contact: Add note to IRC section and calm down the formatting ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/84679?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: Ic808508b5da431d6c0b88a9b2847c34c7b02cfe0 Gerrit-Change-Number: 84679 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 06 Oct 2024 16:57:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: doc/contact: Add note to IRC section and calm down the formatting
by Anastasia Klimchuk (Code Review) 06 Oct '24

06 Oct '24

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/84679?usp=email ) Change subject: doc/contact: Add note to IRC section and calm down the formatting ...................................................................... doc/contact: Add note to IRC section and calm down the formatting Change-Id: Ic808508b5da431d6c0b88a9b2847c34c7b02cfe0 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/contact.rst 1 file changed, 12 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/84679/1 diff --git a/doc/contact.rst b/doc/contact.rst index 79cdb91..38888fb 100644 --- a/doc/contact.rst +++ b/doc/contact.rst @@ -43,8 +43,8 @@ You are welcome to join and discuss current and future flashrom development, ideas and contributions. -If you have a problem and would like to get help, don't ask for help. Instead, just **explain** your problem right away, -and make sure to **describe** the situation as much as possible, so that other people can understand you and provide meaningful answers. +If you have a problem and would like to get help, don't ask for help. Instead, just explain your problem right away, +and make sure to describe the situation as much as possible, so that other people can understand you and provide meaningful answers. Otherwise, others have to ask or guess the details of your problem, which is frustrating for both parties. Should you need to paste lots of text (more than three lines), please use a `paste service <https://en.wikipedia.org/wiki/Pastebin>`_. @@ -54,6 +54,12 @@ Questions on `coreboot <https://coreboot.org>`_, `OpenBIOS <http://www.openbios.info/>`_, firmware and related topics are welcome in **#coreboot** on the same server. +Discord +""""""" + +Flashrom Discord channel is hosted on coreboot's server. Once you join, you will be able to see all coreboot's and flashrom's channels in one place. +To join, use the `invite link <https://discord.gg/dgcrkwVyeR>`_. + IRC """ @@ -68,10 +74,10 @@ with many different cultures and timezones. Most people are in the `CET timezone <https://en.wikipedia.org/wiki/Central_European_Time>`_, so the channel may be very quiet during `CET nighttime <https://time.is/CET>`_. -If you receive no replies, **please be patient**. +If you receive no replies, *please be patient*. After all, silence is better than getting replied with `"IDK" <https://en.wiktionary.org/wiki/IDK>`_. -Frequently, somebody knows the answer, but hasn't checked IRC yet. In any case, please **do not leave the channel while waiting for an answer!** -Since IRC does not store messages, replying to somebody who left the channel is **impossible**. +Frequently, somebody knows the answer, but hasn't checked IRC yet. In any case, please *do not leave the channel while waiting for an answer!* +Since IRC does not store messages, replying to somebody who left the channel is *impossible*. To have persistence on IRC, you can set up an `IRC bouncer <https://en.wikipedia.org/wiki/Internet_Relay_Chat#Bouncer>`_ like `ZNC <https://en.wikipedia.org/wiki/ZNC>`_, or use `IRCCloud <https://www.irccloud.com/>`_. @@ -81,11 +87,7 @@ Instead of sending lots of tiny messages with only about two words, prefer using longer sentences, spaces and punctuation symbols. If reading and understanding your messages is easy, replying to them is also easy. -Discord -""""""" - -Flashrom Discord channel is hosted on coreboot's server. Once you join, you will be able to see all coreboot's and flashrom's channels in one place. -To join, use the `invite link <https://discord.gg/dgcrkwVyeR>`_. +*Note: the channel is not moderated or monitored by any of the current active maintainers.* Dev meeting ----------- -- To view, visit https://review.coreboot.org/c/flashrom/+/84679?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: Ic808508b5da431d6c0b88a9b2847c34c7b02cfe0 Gerrit-Change-Number: 84679 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

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

06 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 7: (1 comment) Patchset: PS7: Sergii, maybe you could have a look? Especially at the code which decides whether to replace previous output or print new line. I think it's the last remaining thing to do here? I was thinking this patch can go ahead first (provided it's all good and approved), and it does not necessarily need to wait for CB:84439 ? While I am still working on CB:84439 Thank you! -- 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: 7 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: Sun, 06 Oct 2024 09:01:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in flashrom[main]: Display progress for what is actually erased/written
by Anastasia Klimchuk (Code Review) 06 Oct '24

06 Oct '24

Attention is currently required from: Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/84439?usp=email ) Change subject: Display progress for what is actually erased/written ...................................................................... Patch Set 7: (2 comments) Patchset: PS7: Sergii, I forgot to add you in this patch too! It's not fully ready yet but I am working on it. I fixed the tests and added one more test. Also now looking into FEATURE_NO_ERASE (made a long comment about it) File flashrom.c: https://review.coreboot.org/c/flashrom/+/84439/comment/773f0652_6bd16650?us… : PS7, Line 1265: // TODO: take FEATURE_NO_ERASE into account? I found something to fix for FEATURE_NO_ERASE, although not here. This code here is fine I think, `need_erase` just compares buffers, this is fine. However, there is another thing on FEATURE_NO_ERASE. I added a test in this patch, which runs on chip with FEATURE_NO_ERASE and progress enabled, and I can debug. What happens is: The emulation path calls the progress for WRITE, even though code is on the ERASE path: Thread 1 "flashrom_unit_t" hit Breakpoint 3, progress_callback (flash=0x7fffffffc618) at ../tests/chip.c:83 83 printf("Progress almost done for stage %d\n", flash->progress_state->stage); (gdb) p flash->progress_state->stage $6 = FLASHROM_PROGRESS_WRITE (gdb) bt #0 progress_callback (flash=0x7fffffffc618) at ../tests/chip.c:83 #1 0x000055555559f2a5 in spi_write_chunked (flash=flash@entry=0x7fffffffc618, buf=buf@entry=0x7ffff2a00070 "", start=start@entry=0, len=len@entry=16777216, chunksize=256) at ../spi25.c:733 #2 0x00005555555a1293 in spi_block_erase_emulation (flash=0x7fffffffc618, addr=<optimised out>, blocklen=16777216) at ../spi95.c:69 #3 0x0000555555592cf6 in erase_write_helper (all_skipped=0x7fffffffc4af, erase_layout=0x555555e7d690, newcontents=0x7ffff4e00070 '\272' <repeats 200 times>..., curcontents=0x7ffff3c00070 '\377' <repeats 200 times>..., region_end=16777215, region_start=0, flashrom_flashctx=0x7fffffffc618) at ../erasure_layout.c:270 #4 erase_write (flashrom_flashctx=flashrom_flashctx@entry=0x7fffffffc618, region_start=0, region_end=16777215, curcontents=curcontents@entry=0x7ffff3c00070 '\377' <repeats 200 times>..., newcontents=newcontents@entry=0x7ffff4e00070 '\272' <repeats 200 times>..., erase_layout=0x555555e7d690, all_skipped=0x7fffffffc4af) at ../erasure_layout.c:393 #5 0x0000555555595938 in write_by_layout (all_skipped=0x7fffffffc4af, newcontents=0x7ffff4e00070, curcontents=0x7ffff3c00070, flashrom_flashctx=0x7fffffffc618) at ../flashrom.c:1470 #6 flashrom_image_write (flashrom_flashctx=flashrom_flashctx@entry=0x7fffffffc618, buffer=buffer@entry=0x7ffff4e00070, buffer_len=buffer_len@entry=16777216, refbuffer=refbuffer@entry=0x0) at ../flashrom.c:2023 #7 0x000055555558ac67 in write_chip_feature_no_erase_with_progress (state=<optimised out>) at ../tests/chip.c:605 #8 0x00007ffff7fa44e1 in ?? () from /lib/x86_64-linux-gnu/libcmocka.so.0 #9 0x00007ffff7fa4ec5 in _cmocka_run_group_tests () from /lib/x86_64-linux-gnu/libcmocka.so.0 #10 0x0000555555586170 in main (argc=<optimised out>, argv=<optimised out>) at ../tests/tests.c:512 So when no_erase chips are erased, there is `spi_write_chunked` invoked under the hood, which is updating progress for write operation (even though actual operation is erase). By the time erase completes (it does), current progress for write is already at 100% (because it has been updated during erase). So, when writing starts, it keeps going further and increases total on the fly in libflashrom.c:95 and total eventually becomes twice more than it should. I will look at this. -- To view, visit https://review.coreboot.org/c/flashrom/+/84439?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: I88ac4d40f1b6ccc1636b1efb690d8d68bdebec08 Gerrit-Change-Number: 84439 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Sun, 06 Oct 2024 08:52:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in flashrom[main]: Display progress for what is actually erased/written
by Anastasia Klimchuk (Code Review) 06 Oct '24

06 Oct '24

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/84439?usp=email to look at the new patch set (#7). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: Display progress for what is actually erased/written ...................................................................... Display progress for what is actually erased/written Change-Id: I88ac4d40f1b6ccc1636b1efb690d8d68bdebec08 Co-developed-by: Anastasia Klimchuk <aklm(a)flashrom.org> Co-developed-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> --- M doc/classic_cli_manpage.rst M flashrom.c M tests/chip.c M tests/tests.c M tests/tests.h 5 files changed, 104 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/84439/7 -- To view, visit https://review.coreboot.org/c/flashrom/+/84439?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: I88ac4d40f1b6ccc1636b1efb690d8d68bdebec08 Gerrit-Change-Number: 84439 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit October 2024