Attention is currently required from: Sergii Dmytruk.

View Change

2 comments:

• Patchset:

  • Patch Set #7:

    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:

  • Patch Set #7, 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 change 84439. To unsubscribe, or for help writing mail filters, visit settings.