June 2024 - flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[main]: doc: Add doc for supported boards and laptops
by Anastasia Klimchuk (Code Review) 12 Jun '24

12 Jun '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82271?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: doc: Add doc for supported boards and laptops ...................................................................... doc: Add doc for supported boards and laptops Change-Id: Iaae05ccd138fd8f7760823f867f3c7799018dc2e Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82271 Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M doc/supported_hw/index.rst A doc/supported_hw/supported_boards.rst 2 files changed, 20 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved diff --git a/doc/supported_hw/index.rst b/doc/supported_hw/index.rst index ba77132..eebf394 100644 --- a/doc/supported_hw/index.rst +++ b/doc/supported_hw/index.rst @@ -8,3 +8,4 @@ supported_flashchips supported_prog/index supported_chipsets + supported_boards diff --git a/doc/supported_hw/supported_boards.rst b/doc/supported_hw/supported_boards.rst new file mode 100644 index 0000000..91b2899 --- /dev/null +++ b/doc/supported_hw/supported_boards.rst @@ -0,0 +1,19 @@ +======================== +Supported boards/laptops +======================== + +To see the list of all supported boards or laptops, check either ``struct board_info boards_known[]`` or ``struct board_info laptops_known[]`` +in the ``known_boards.c`` file in the source tree. + +If you have a flashrom repo cloned locally, you can look at the file in your repo, alternatively inspect the file +`on the web UI of our GitHub mirror <https://github.com/flashrom/flashrom/blob/main/known_boards.c#L29>`_. + +If you can run flashrom locally, the command ``flashrom -L`` prints the list of all supported boards and laptops +(see :doc:`/classic_cli_manpage` for more details on command line options). The output of this command is long, so you might +want to save it to file or grep. + +Each board entry is described by the ``struct board_info`` in ``include/programmer.h`` which you can inspect in the same way, either in the local source tree or +`in the GitHub web UI <https://github.com/flashrom/flashrom/blob/main/include/programmer.h#L207>`_. + +Note the ``enum test_state status`` of the board. ``OK`` means board is tested, ``NT`` means not tested, to see all possible +test states check the ``enum test_state`` definition in ``include/flash.h``. -- To view, visit https://review.coreboot.org/c/flashrom/+/82271?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iaae05ccd138fd8f7760823f867f3c7799018dc2e Gerrit-Change-Number: 82271 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in flashrom[main]: doc: Add doc for supported chipsets
by Anastasia Klimchuk (Code Review) 12 Jun '24

12 Jun '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82198?usp=email ) Change subject: doc: Add doc for supported chipsets ...................................................................... doc: Add doc for supported chipsets Change-Id: I9c9edc7deeeb7a783e2ba2fc6b372edb9c61609e Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82198 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M doc/supported_hw/index.rst A doc/supported_hw/supported_chipsets.rst 2 files changed, 22 insertions(+), 0 deletions(-) Approvals: Peter Marheine: Looks good to me, approved build bot (Jenkins): Verified diff --git a/doc/supported_hw/index.rst b/doc/supported_hw/index.rst index aec367b..ba77132 100644 --- a/doc/supported_hw/index.rst +++ b/doc/supported_hw/index.rst @@ -7,3 +7,4 @@ supported_flashchips supported_prog/index + supported_chipsets diff --git a/doc/supported_hw/supported_chipsets.rst b/doc/supported_hw/supported_chipsets.rst new file mode 100644 index 0000000..ca7dc8f --- /dev/null +++ b/doc/supported_hw/supported_chipsets.rst @@ -0,0 +1,21 @@ +================== +Supported chipsets +================== + +To see the list of all supported chipsets, check the ``const struct penable chipset_enables[]`` in ``chipset_enable.c`` file in the source tree. +If you have a flashrom repo cloned locally, you can look at the file in your repo. + +Alternatively inspect the file `on the web UI of our GitHub mirror <https://github.com/flashrom/flashrom/blob/main/chipset_enable.c#L1768>`_. + +If you can run flashrom locally, the command ``flashrom -L`` prints the list of all supported chipsets +(see :doc:`/classic_cli_manpage` for more details on command line options). The output of this command is long, so you might +want to save it to file or grep. + +Each chipset entry is described by ``struct penable`` in ``include/programmer.h`` which you can inspect in the same way, either in the local source tree or +`in the GitHub web UI <https://github.com/flashrom/flashrom/blob/main/include/programmer.h#L149>`_. + +Note the ``enum test_state status`` of the chipset. ``OK`` means chipset is tested, ``NT`` means not tested, to see all possible +test states check the ``enum test_state`` definition in ``include/flash.h``. + +Also note that macros for supported buses are defined in ``chipset_enable.c`` right before ``chipset_enables[]`` array. +For example ``B_S`` means ``BUS_SPI``, check the ``chipset_enable.c`` source code for the rest of macro definitions. -- To view, visit https://review.coreboot.org/c/flashrom/+/82198?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I9c9edc7deeeb7a783e2ba2fc6b372edb9c61609e Gerrit-Change-Number: 82198 Gerrit-PatchSet: 5 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[S] Change in flashrom[main]: doc: Add doc for supported chipsets
by Peter Marheine (Code Review) 12 Jun '24

12 Jun '24

Attention is currently required from: Anastasia Klimchuk, Thomas Heijligen. Peter Marheine has posted comments on this change by Anastasia Klimchuk. ( https://review.coreboot.org/c/flashrom/+/82198?usp=email ) Change subject: doc: Add doc for supported chipsets ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82198?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: I9c9edc7deeeb7a783e2ba2fc6b372edb9c61609e Gerrit-Change-Number: 82198 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 12 Jun 2024 04:05:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in flashrom[main]: flashchips: Add support for chip model GD25LB128E/GD25LR128E
by Victor Lim (Code Review) 12 Jun '24

12 Jun '24

Attention is currently required from: Nikolai Artemiev, Stefan Reinauer, Victor Lim. Hello Anastasia Klimchuk, Nikolai Artemiev, Stefan Reinauer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/83012?usp=email to look at the new patch set (#2). Change subject: flashchips: Add support for chip model GD25LB128E/GD25LR128E ...................................................................... flashchips: Add support for chip model GD25LB128E/GD25LR128E Adding GD25LB128E/GD25LR128E to flashchip.c These part # sharing the same ID, form, fit, function, produced with different process node: 1.8V 128Mbit Datasheet Link: https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20230926/DS-0… https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20220714/DS-0… https://www.gigadevice.com/message-board?cid=39&id=3004&file_type=Datasheet… Change-Id: I8e8ad6bd69e2159b7cc2b64a7a8c7fcb1399294d Signed-off-by: Victor Lim <vlim(a)gigadevice.com> --- M flashchips.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/12/83012/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/83012?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: I8e8ad6bd69e2159b7cc2b64a7a8c7fcb1399294d Gerrit-Change-Number: 83012 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>

1 0

[L] Change in flashrom[main]: [WIP] tests: Add tests for erasing chip with protected region
by Anastasia Klimchuk (Code Review) 12 Jun '24

12 Jun '24

Anastasia Klimchuk has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/81664?usp=email ) Change subject: [WIP] tests: Add tests for erasing chip with protected region ...................................................................... Abandoned Merged as a part of CB:82393 -- 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?usp=email Gerrit-MessageType: abandon 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>

1 0

[M] Change in flashrom[main]: flashchips: add support for chip model Winbond W25Q16JV_M
by Anastasia Klimchuk (Code Review) 12 Jun '24

12 Jun '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82715?usp=email ) Change subject: flashchips: add support for chip model Winbond W25Q16JV_M ...................................................................... flashchips: add support for chip model Winbond W25Q16JV_M This is a 2 MiB model with QE=0 factory setting. Tested with ch341a programmer: probe, read, write, erase Link to datasheet: https://www.winbond.com/resource-files/w25q16jv%20spi%20revh%2004082019%20p… Change-Id: Ida1ceb5fe31411bef647e5133c5bd0bdb02d7704 Signed-off-by: Michael Heimpold <mhei(a)heimpold.de> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82715 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashchips.c M include/flashchips.h 2 files changed, 53 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index a5d9ec2..86ebce3 100644 --- a/flashchips.c +++ b/flashchips.c @@ -19332,6 +19332,58 @@ { .vendor = "Winbond", + .name = "W25Q16JV_M", + .bustype = BUS_SPI, + .manufacture_id = WINBOND_NEX_ID, + .model_id = WINBOND_NEX_W25Q16JV_M, + .total_size = 2048, + .page_size = 256, + /* supports SFDP */ + /* OTP: 3x 256B total; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | + FEATURE_WRSR2 | FEATURE_WRSR3, + .tested = TEST_OK_PREW, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 512} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 64} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 32} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {2 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {2 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_SRWD_SEC_TB_BP2_WELWIP, + .unlock = SPI_DISABLE_BLOCKPROTECT, + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, + .voltage = {2700, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .sec = {STATUS1, 6, RW}, + .tb = {STATUS1, 5, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}}, + .cmp = {STATUS2, 6, RW}, + .srl = {STATUS2, 0, RW}, + .wps = {STATUS3, 2, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, + }, + + { + .vendor = "Winbond", .name = "W25Q16.V", .bustype = BUS_SPI, .manufacture_id = WINBOND_NEX_ID, diff --git a/include/flashchips.h b/include/flashchips.h index c264b04..691ffc5 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -1009,6 +1009,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_W25Q16JV_M 0x7015 /* W25Q16JV_M (QE=0) */ #define WINBOND_NEX_W25Q64JV 0x7017 /* W25Q64JV */ #define WINBOND_NEX_W25Q128_V_M 0x7018 /* W25Q128JVSM */ #define WINBOND_NEX_W25Q256JV_M 0x7019 /* W25Q256JV_M (QE=0) */ -- To view, visit https://review.coreboot.org/c/flashrom/+/82715?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ida1ceb5fe31411bef647e5133c5bd0bdb02d7704 Gerrit-Change-Number: 82715 Gerrit-PatchSet: 3 Gerrit-Owner: Michael Heimpold <mhei(a)heimpold.de> 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>

1 0

[XS] Change in flashrom[main]: flashchips: Add support for chip model GD25LQ128E/GD25LB128E/GD25LR12...
by Anastasia Klimchuk (Code Review) 11 Jun '24

11 Jun '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/+/83012?usp=email ) Change subject: flashchips: Add support for chip model GD25LQ128E/GD25LB128E/GD25LR128E/GD25LQ128D/GD25LQ128C ...................................................................... Patch Set 1: (3 comments) Patchset: PS1: Victor thank you, the patch looks good! I only have comments about commit message. You can modify the commit message in Gerrit, there is an Edit button below commit message box. Also just to check, you have tested the models you are adding, is that right? Do you have the link to datasheet for GD25LR128E ? Thank you! Commit Message: https://review.coreboot.org/c/flashrom/+/83012/comment/7e6ffec5_8d7577c3?us… : PS1, Line 7: GD25LQ128E/GD25LB128E/GD25LR128E/GD25LQ128D/GD25LQ128C You can replace this list with GD25LB128E/GD25LR128E , because you add two models, and the others were here before. So commit title will be > flashchips: Add support for chip model GD25LB128E/GD25LR128E https://review.coreboot.org/c/flashrom/+/83012/comment/253fbd98_60bf60da?us… : PS1, Line 9: Adding GD25LQ128E/GD25LB128E/GD25LR128E/GD25LQ128D/GD25LQ128C to flashchip.c : : These part # sharing the same ID, form, fit, function, produced with different process node: 1.8V 128Mbit You need to wrap the commit message to be 72 chars max width. (links to datasheets are exception, leave them long) Keep the same text, but break the lines so that each line no longer than 72 chars. -- To view, visit https://review.coreboot.org/c/flashrom/+/83012?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: I8e8ad6bd69e2159b7cc2b64a7a8c7fcb1399294d Gerrit-Change-Number: 83012 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: Tue, 11 Jun 2024 07:04:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Peter Marheine (Code Review) 11 Jun '24

11 Jun '24

Peter Marheine has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email ) 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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82393 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M erasure_layout.c M tests/erase_func_algo.c M tests/tests.c M tests/tests.h 4 files changed, 602 insertions(+), 95 deletions(-) Approvals: Anastasia Klimchuk: Looks good to me, approved Peter Marheine: Looks good to me, approved build bot (Jenkins): Verified Nikolai Artemiev: Looks good to me, approved diff --git a/erasure_layout.c b/erasure_layout.c index d15594d..ab54c12 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -238,6 +238,80 @@ } } +static int erase_write_helper(struct flashctx *const flashctx, chipoff_t region_start, chipoff_t region_end, + uint8_t *curcontents, uint8_t *newcontents, + struct erase_layout *erase_layout, bool *all_skipped) +{ + const size_t erasefn_count = count_usable_erasers(flashctx); + + // select erase functions + for (size_t i = 0; i < erase_layout[erasefn_count - 1].block_count; i++) { + if (erase_layout[erasefn_count - 1].layout_list[i].start_addr <= region_end && + region_start <= erase_layout[erasefn_count - 1].layout_list[i].end_addr) + select_erase_functions(flashctx, erase_layout, + erasefn_count - 1, i, + curcontents, newcontents, + region_start, region_end); + } + + // erase + for (size_t i = 0; i < erasefn_count; i++) { + for (size_t j = 0; j < erase_layout[i].block_count; j++) { + if (!erase_layout[i].layout_list[j].selected) + continue; + + chipoff_t start_addr = erase_layout[i].layout_list[j].start_addr; + unsigned int block_len = erase_layout[i].layout_list[j].end_addr - start_addr + 1; + const uint8_t erased_value = ERASED_VALUE(flashctx); + // execute erase + erasefunc_t *erasefn = lookup_erase_func_ptr(erase_layout[i].eraser); + + + if (erasefn(flashctx, start_addr, block_len)) { + return -1; + } + if (check_erased_range(flashctx, start_addr, block_len)) { + return -1; + msg_cerr("ERASE FAILED!\n"); + } + + // adjust curcontents + memset(curcontents+start_addr, erased_value, block_len); + // after erase make it unselected again + erase_layout[i].layout_list[j].selected = false; + msg_cdbg("E(%"PRIx32":%"PRIx32")", start_addr, start_addr + block_len - 1); + + *all_skipped = false; + } + } + + // write + unsigned int start_here = 0, len_here = 0, erase_len = region_end - region_start + 1; + while ((len_here = get_next_write(curcontents + region_start + start_here, + newcontents + region_start + start_here, + erase_len - start_here, &start_here, + flashctx->chip->gran))) { + // execute write + int ret = write_flash(flashctx, + newcontents + region_start + start_here, + region_start + start_here, len_here); + if (ret) { + msg_cerr("Write failed at %#x, Abort.\n", region_start + start_here); + return -1; + } + + // adjust curcontents + memcpy(curcontents + region_start + start_here, + newcontents + region_start + start_here, len_here); + msg_cdbg("W(%"PRIx32":%"PRIx32")", region_start + start_here, region_start + start_here + len_here - 1); + + *all_skipped = false; + } + + return 0; +} + + /* * @brief wrapper to use the erase algorithm * @@ -253,12 +327,15 @@ uint8_t *curcontents, uint8_t *newcontents, struct erase_layout *erase_layout, bool *all_skipped) { - const size_t erasefn_count = count_usable_erasers(flashctx); int ret = 0; - size_t i; chipoff_t old_start = region_start, old_end = region_end; align_region(erase_layout, flashctx, &region_start, &region_end); + if (!flashctx->flags.skip_unwritable_regions) { + if (check_for_unwritable_regions(flashctx, region_start, region_end - region_start + 1)) + return -1; + } + uint8_t *old_start_buf = NULL, *old_end_buf = NULL; const size_t start_buf_len = old_start - region_start; const size_t end_buf_len = region_end - old_end; @@ -287,101 +364,34 @@ memcpy(newcontents + end_offset, curcontents + end_offset, end_buf_len); } - // select erase functions - for (i = 0; i < erase_layout[erasefn_count - 1].block_count; i++) { - if (erase_layout[erasefn_count - 1].layout_list[i].start_addr <= region_end && - region_start <= erase_layout[erasefn_count - 1].layout_list[i].end_addr) - select_erase_functions(flashctx, erase_layout, - erasefn_count - 1, i, - curcontents, newcontents, - region_start, region_end); - } + unsigned int len; + for (unsigned int addr = region_start; addr <= region_end; addr += len) { + struct flash_region region; + get_flash_region(flashctx, addr, &region); + len = min(region_end, region.end) - addr + 1; - for (i = 0; i < erasefn_count; i++) { - for (size_t j = 0; j < erase_layout[i].block_count; j++) { - if (!erase_layout[i].layout_list[j].selected) continue; - - // erase - chipoff_t start_addr = erase_layout[i].layout_list[j].start_addr; - unsigned int block_len = erase_layout[i].layout_list[j].end_addr - start_addr + 1; - const uint8_t erased_value = ERASED_VALUE(flashctx); - // execute erase - erasefunc_t *erasefn = lookup_erase_func_ptr(erase_layout[i].eraser); - - if (!flashctx->flags.skip_unwritable_regions) { - if (check_for_unwritable_regions(flashctx, start_addr, block_len)) - goto _end; - } - - unsigned int len; - for (unsigned int addr = start_addr; addr < start_addr + block_len; addr += len) { - struct flash_region region; - get_flash_region(flashctx, addr, &region); - - len = min(start_addr + block_len, region.end + 1) - addr; - - if (region.write_prot) { - msg_gdbg("%s: cannot erase inside %s " - "region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n", - __func__, region.name, - region.start, region.end, - addr, addr + len - 1); - free(region.name); - continue; - } - - msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is " - "writable, erasing range (%#08x..%#08x).\n", - __func__, region.name, - region.start, region.end, - addr, addr + len - 1); - free(region.name); - - if (erasefn(flashctx, addr, len)) { - ret = -1; - goto _end; - } - if (check_erased_range(flashctx, addr, len)) { - ret = - 1; - msg_cerr("ERASE FAILED!\n"); - goto _end; - } - } - - // adjust curcontents - memset(curcontents+start_addr, erased_value, block_len); - // after erase make it unselected again - erase_layout[i].layout_list[j].selected = false; - msg_cdbg("E(%"PRIx32":%"PRIx32")", start_addr, start_addr + block_len - 1); - - *all_skipped = false; + if (region.write_prot) { + msg_gdbg("%s: cannot erase inside %s " + "region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n", + __func__, region.name, + region.start, region.end, + addr, addr + len - 1); + free(region.name); + continue; } - } - // write - unsigned int start_here = 0, len_here = 0, erase_len = region_end - region_start + 1; - while ((len_here = get_next_write(curcontents + region_start + start_here, - newcontents + region_start + start_here, - erase_len - start_here, &start_here, - flashctx->chip->gran))) { - // execute write - ret = write_flash(flashctx, - newcontents + region_start + start_here, - region_start + start_here, len_here); - if (ret) { - msg_cerr("Write failed at %#zx, Abort.\n", i); - ret = -1; + msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is " + "writable, erasing range (%#08x..%#08x).\n", + __func__, region.name, + region.start, region.end, + addr, addr + len - 1); + free(region.name); + + + ret = erase_write_helper(flashctx, addr, addr + len - 1, curcontents, newcontents, erase_layout, all_skipped); + if (ret) goto _end; - } - - // adjust curcontents - memcpy(curcontents + region_start + start_here, - newcontents + region_start + start_here, len_here); - msg_cdbg("W(%"PRIx32":%"PRIx32")", region_start + start_here, region_start + start_here + len_here - 1); - - *all_skipped = false; } - _end: if (old_start_buf) { memcpy(newcontents + region_start, old_start_buf, start_buf_len); diff --git a/tests/erase_func_algo.c b/tests/erase_func_algo.c index b0969c2..576923f 100644 --- a/tests/erase_func_algo.c +++ b/tests/erase_func_algo.c @@ -22,7 +22,7 @@ #include "libflashrom.h" #include "programmer.h" -#define LOG_ERASE_FUNC printf("Eraser called with blockaddr=0x%x, blocklen=0x%x\n", blockaddr, blocklen) +#define LOG_ERASE_FUNC printf("Eraser called with blockaddr=0x%x, blocklen=0x%x, erase_func=%d\n", blockaddr, blocklen, erase_func - TEST_ERASE_INJECTOR_1 + 1) #define LOG_READ_WRITE_FUNC printf("%s called with start=0x%x, len=0x%x\n", __func__, start, len) #define ERASE_VALUE 0xff @@ -48,10 +48,11 @@ uint8_t initial_buf[MOCK_CHIP_SIZE]; /* Initial state of chip memory. */ uint8_t erased_buf[MOCK_CHIP_SIZE]; /* Expected content after erase. */ uint8_t written_buf[MOCK_CHIP_SIZE]; /* Expected content after write. */ + uint8_t written_protected_buf[MOCK_CHIP_SIZE]; /* Expected content after write with protected region. */ struct erase_invoke eraseblocks_expected[MOCK_CHIP_SIZE]; /* Expected order of eraseblocks invocations. */ unsigned int eraseblocks_expected_ind; /* Expected number of eraseblocks invocations. */ - char erase_test_name[20]; /* Test case display name for testing erase operation. */ - char write_test_name[20]; /* Test case display name for testing write operation. */ + char erase_test_name[40]; /* Test case display name for testing erase operation. */ + char write_test_name[40]; /* Test case display name for testing write operation. */ }; struct all_state { @@ -764,6 +765,181 @@ }, }; + +#define START_PROTECTED_REGION 6 +#define END_PROTECTED_REGION 13 + +/* + * Setup all test cases with protected region. + * Protected region is the same for all test cases, between bytes 8 - 15. + */ +static struct test_case test_cases_protected_region[] = { + { + /* + * Test case #0 + * + * Initial vs written: all 16 bytes are different. + * One layout region for the whole chip. + * Chip with eraseblocks 1, 2, 4, 8, 16. + */ + .chip = &chip_1_2_4_8_16, + .regions = {{0, MIN_REAL_CHIP_SIZE - 1, "whole chip"}}, + .initial_buf = {0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, + 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, 0x6, 0x7, + 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, ERASE_VALUE, ERASE_VALUE}, + .written_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, + 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff}, + .written_protected_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0x6, 0x7, + 0x8, 0x9, 0xa, 0xb, 0xc, 0xd, 0xfe, 0xff}, + .eraseblocks_expected = {{0x4, 0x2, TEST_ERASE_INJECTOR_2}, {0x0, 0x4, TEST_ERASE_INJECTOR_3}, + {0xe, 0x2, TEST_ERASE_INJECTOR_2}}, + .eraseblocks_expected_ind = 3, + .erase_test_name = "Erase protected region test case #0", + .write_test_name = "Write protected region test case #0", + }, { + /* + * Test case #1 + * + * Initial vs written: all 16 bytes are different. + * Two layout regions each one 8 bytes, which is 1/2 size of chip. + * Chip with eraseblocks 1, 2, 4, 8, 16. + */ + .chip = &chip_1_2_4_8_16, + .regions = {{0, MOCK_CHIP_SIZE/2 - 1, "part1"}, {MOCK_CHIP_SIZE/2, MIN_REAL_CHIP_SIZE - 1, "part2"}}, + .initial_buf = {0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, + 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xfe, 0xff}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, 0xf6, 0xf7, + 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, ERASE_VALUE, ERASE_VALUE}, + .written_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, + 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf}, + .written_protected_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xf6, 0xf7, + 0xf8, 0xf9, 0xfa, 0xfb, 0xfc, 0xfd, 0xae, 0xaf}, + .eraseblocks_expected = {{0xe, 0x2, TEST_ERASE_INJECTOR_2}, {0x4, 0x2, TEST_ERASE_INJECTOR_2}, + {0x0, 0x4, TEST_ERASE_INJECTOR_3}}, + .eraseblocks_expected_ind = 3, + .erase_test_name = "Erase protected region test case #1", + .write_test_name = "Write protected region test case #1", + }, { + /* + * Test case #2 + * + * Initial vs written: all 16 bytes are different. + * Three layout regions 8+4+4b + * Chip with eraseblocks 1, 2, 4, 8, 16. + */ + .chip = &chip_1_2_4_8_16, + .regions = {{0, 7, "odd1"}, {8, 11, "odd2"}, {12, 15, "odd3"}, + {MOCK_CHIP_SIZE, MIN_REAL_CHIP_SIZE - 1, "longtail"}}, + .initial_buf = {0xff, 0xff, 0x0, 0xff, 0x0, 0xff, 0x0, 0xff, + 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, 0xff, 0xff}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, 0x0, 0xff, + 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, ERASE_VALUE, ERASE_VALUE}, + .written_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, + 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf}, + .written_protected_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0x0, 0xff, + 0x0, 0xff, 0x0, 0xff, 0xff, 0xff, 0xae, 0xaf}, + .eraseblocks_expected = {{0xe, 0x2, TEST_ERASE_INJECTOR_2}, {0x4, 0x2, TEST_ERASE_INJECTOR_2}, + {0x0, 0x4, TEST_ERASE_INJECTOR_3}}, + .eraseblocks_expected_ind = 3, + .erase_test_name = "Erase protected region test case #2", + .write_test_name = "Write protected region test case #2", + }, { + /* + * Test case #3 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned regions 2+4+9+1b which require use of the 1-byte erase block. + * Chip with eraseblocks 1, 2, 4, 8, 16. + */ + .chip = &chip_1_2_4_8_16, + .regions = {{0, 1, "reg2"}, {2, 5, "reg4"}, {6, 14, "reg9"}, + {15, MIN_REAL_CHIP_SIZE - 1, "reg1"}}, + .initial_buf = {0x4, 0x4, 0x5, 0x5, 0x5, 0x5, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x7}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, ERASE_VALUE, ERASE_VALUE}, + .written_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, + 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf}, + .written_protected_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0xae, 0xaf}, + .eraseblocks_expected = {{0xf, 0x1, TEST_ERASE_INJECTOR_1}, {0xe, 0x1, TEST_ERASE_INJECTOR_1}, + {0x2, 0x2, TEST_ERASE_INJECTOR_2}, {0x4, 0x2, TEST_ERASE_INJECTOR_2}, + {0x0, 0x2, TEST_ERASE_INJECTOR_2}}, + .eraseblocks_expected_ind = 5, + .erase_test_name = "Erase protected region test case #3", + .write_test_name = "Write protected region test case #3", + }, { + /* + * Test case #4 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned region 3+13b which require use of the 1-byte erase block. + * Chip with eraseblocks 1, 8, 16. + */ + .chip = &chip_1_8_16, + .regions = {{0, 2, "reg3"}, {3, MIN_REAL_CHIP_SIZE - 1, "tail"}}, + .initial_buf = {0x4, 0x4, 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, ERASE_VALUE, ERASE_VALUE}, + .written_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, + 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf}, + .written_protected_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0x6, 0x6, + 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0xae, 0xaf}, + .eraseblocks_expected = { + {0x3, 0x1, TEST_ERASE_INJECTOR_1}, + {0x4, 0x1, TEST_ERASE_INJECTOR_1}, + {0x5, 0x1, TEST_ERASE_INJECTOR_1}, + {0xe, 0x1, TEST_ERASE_INJECTOR_1}, + {0xf, 0x1, TEST_ERASE_INJECTOR_1}, + {0x0, 0x1, TEST_ERASE_INJECTOR_1}, + {0x1, 0x1, TEST_ERASE_INJECTOR_1}, + {0x2, 0x1, TEST_ERASE_INJECTOR_1}, + }, + .eraseblocks_expected_ind = 8, + .erase_test_name = "Erase protected region test case #4", + .write_test_name = "Write protected region test case #4", + }, { + /* + * Test case #5 + * + * Initial vs written: all 16 bytes are different. + * Layout with unaligned region 9+7b. + * Chip with eraseblocks 1, 8, 16. + */ + .chip = &chip_1_8_16, + .regions = {{0, 8, "reg9"}, {9, MIN_REAL_CHIP_SIZE - 1, "tail"}}, + .initial_buf = {0x4, 0x4, 0x4, 0x4, 0x4, 0x4, 0x4, 0x4, + 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6, 0x6}, + .erased_buf = {ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, ERASE_VALUE, + ERASE_VALUE, ERASE_VALUE, 0x4, 0x4, + 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, ERASE_VALUE, ERASE_VALUE}, + .written_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0xa6, 0xa7, + 0xa8, 0xa9, 0xaa, 0xab, 0xac, 0xad, 0xae, 0xaf}, + .written_protected_buf = {0xa0, 0xa1, 0xa2, 0xa3, 0xa4, 0xa5, 0x4, 0x4, + 0x4, 0x6, 0x6, 0x6, 0x6, 0x6, 0xae, 0xaf}, + .eraseblocks_expected = { + {0xe, 0x1, TEST_ERASE_INJECTOR_1}, + {0xf, 0x1, TEST_ERASE_INJECTOR_1}, + {0x0, 0x1, TEST_ERASE_INJECTOR_1}, + {0x1, 0x1, TEST_ERASE_INJECTOR_1}, + {0x2, 0x1, TEST_ERASE_INJECTOR_1}, + {0x3, 0x1, TEST_ERASE_INJECTOR_1}, + {0x4, 0x1, TEST_ERASE_INJECTOR_1}, + {0x5, 0x1, TEST_ERASE_INJECTOR_1}, + }, + .eraseblocks_expected_ind = 8, + .erase_test_name = "Erase protected region test case #5", + .write_test_name = "Write protected region test case #5", + }, +}; + static int setup(void **state) { struct test_case *current_test_case = *state; g_state.current_test_case = current_test_case; @@ -806,6 +982,55 @@ return all_cases; } +static void test_erase_fails_for_unwritable_region(void **); +static void erase_unwritable_regions_skipflag_on_test_success(void **); +static void write_unwritable_regions_skipflag_on_test_success(void **); + +/* + * Creates the array of tests for each test case in test_cases_protected_region[]. + * The caller needs to free the allocated memory. + */ +struct CMUnitTest *get_erase_protected_region_algo_tests(size_t *num_tests) { + const size_t num_parameterized = ARRAY_SIZE(test_cases_protected_region); + const size_t num_unparameterized = 1; + // Twice the number of parameterized test cases, because each test case is run twice: + // for erase and write. + const size_t num_cases = num_parameterized * 2 + num_unparameterized; + + struct CMUnitTest *all_cases = calloc(num_cases, sizeof(struct CMUnitTest)); + *num_tests = num_cases; + + for (size_t i = 0; i < num_parameterized; i++) { + all_cases[i] = (struct CMUnitTest) { + .name = test_cases_protected_region[i].erase_test_name, + .setup_func = setup, + .teardown_func = teardown, + .initial_state = &test_cases_protected_region[i], + .test_func = erase_unwritable_regions_skipflag_on_test_success, + }; + all_cases[i + num_parameterized] = (struct CMUnitTest) { + .name = test_cases_protected_region[i].write_test_name, + .setup_func = setup, + .teardown_func = teardown, + .initial_state = &test_cases_protected_region[i], + .test_func = write_unwritable_regions_skipflag_on_test_success, + }; + } + + memcpy( + &all_cases[num_parameterized * 2], + (const struct CMUnitTest[]){ + (const struct CMUnitTest) { + .name = "erase failure for unskipped unwritable regions", + .test_func = test_erase_fails_for_unwritable_region, + } + }, + sizeof(*all_cases) * num_unparameterized + ); + + return all_cases; +} + /* * This function is invoked for every test case in test_cases[], * current test case is passed as an argument. @@ -906,3 +1131,264 @@ assert_int_equal(0, all_write_test_result); } + +static void get_protected_region(const struct flashctx *flash, unsigned int addr, struct flash_region *region) +{ + if (addr < 20) + printf("Inside test get_protected_region for addr=0x%x\n", addr); + + if (addr < START_PROTECTED_REGION) { + region->name = strdup("not protected"); + region->start = 0; + region->end = START_PROTECTED_REGION - 1; + region->read_prot = false; + region->write_prot = false; + } else if (addr <= END_PROTECTED_REGION) { + region->name = strdup("protected"); + region->start = START_PROTECTED_REGION; + region->end = END_PROTECTED_REGION; + region->read_prot = true; + region->write_prot = true; + } else { + region->name = strdup("tail"); + region->start = END_PROTECTED_REGION + 1; + region->end = flashrom_flash_getsize(flash) - 1; + region->read_prot = false; + region->write_prot = false; + } +} + +static int block_erase_chip_with_protected_region(struct flashctx *flash, enum block_erase_func erase_func, unsigned int blockaddr, unsigned int blocklen) +{ + if (blockaddr + blocklen <= MOCK_CHIP_SIZE) { + LOG_ERASE_FUNC; + + /* Register eraseblock invocation. */ + g_state.eraseblocks_actual[g_state.eraseblocks_actual_ind] = (struct erase_invoke){ + .erase_func = erase_func, + .blocklen = blocklen, + .blockaddr = blockaddr, + }; + g_state.eraseblocks_actual_ind++; + } + + assert_in_range(blockaddr + blocklen, 0, MIN_REAL_CHIP_SIZE); + + // Check we are not trying to erase protected region. This should not happen, + // because the logic should handle protected regions and never invoke erasefn + // for them. If this happens, means there is a bug in erasure logic, and test fails. + // + // Note: returning 1 instead of assert, so that the flow goes back to erasure code + // to clean up the memory after failed erase. Memory leaks are also tested by unit tests. + const unsigned int erase_op_size = 1 << (erase_func - TEST_ERASE_INJECTOR_1); + if (blocklen < erase_op_size) { + printf("Error: block length %d is smaller than erase_func length %d\n", blocklen, erase_op_size); + return 1; + } + + if ((blockaddr >= START_PROTECTED_REGION && blockaddr <= END_PROTECTED_REGION) + || (blockaddr + blocklen - 1 >= START_PROTECTED_REGION + && blockaddr + blocklen - 1 <= END_PROTECTED_REGION) + || (blockaddr < START_PROTECTED_REGION + && blockaddr + blocklen + 1 > END_PROTECTED_REGION)) { + printf("Error: block with start=%d, len=%d overlaps protected region %d-%d\n", + blockaddr, blocklen, START_PROTECTED_REGION, END_PROTECTED_REGION); + return 1; + } + + memset(&g_state.buf[blockaddr], ERASE_VALUE, blocklen); + return 0; +} + +#define BLOCK_ERASE_PROTECTED_FUNC(i) static int block_erase_chip_with_protected_region_ ## i \ + (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen) { \ + return block_erase_chip_with_protected_region(flash, TEST_ERASE_INJECTOR_ ## i, blockaddr, blocklen); \ + } +BLOCK_ERASE_PROTECTED_FUNC(1) +BLOCK_ERASE_PROTECTED_FUNC(2) +BLOCK_ERASE_PROTECTED_FUNC(3) +BLOCK_ERASE_PROTECTED_FUNC(4) +BLOCK_ERASE_PROTECTED_FUNC(5) + +/* + * Runs the test cases that use protected flash regions (regions returned from + * get_flash_region() where write_prot is true) when the runtime flag to avoid + * writing to those regions is enabled. + * + * These tests verify that no protected region is erased, and that the erase + * commands used match the expected erase size (ensuring for example that a + * command erasing 16 bytes is not used when only 8 should be erased). + */ +static void erase_unwritable_regions_skipflag_on_test_success(void **state) +{ + struct test_case* current_test_case = *state; + + int all_erase_tests_result = 0; + struct flashrom_flashctx flashctx = { 0 }; + const char *param = ""; /* Default values for all params. */ + + struct flashrom_layout *layout; + + setup_chip(&flashctx, &layout, param, current_test_case); + + // This test needs special block erase to emulate protected regions. + memcpy(g_test_erase_injector, + (erasefunc_t *const[]){ + block_erase_chip_with_protected_region_1, + block_erase_chip_with_protected_region_2, + block_erase_chip_with_protected_region_3, + block_erase_chip_with_protected_region_4, + block_erase_chip_with_protected_region_5, + }, + sizeof(g_test_erase_injector) + ); + + flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS, true); + + // We use dummyflasher programmer in tests, but for this test we need to + // replace dummyflasher's default get_region fn with test one. + // The rest of master struct is fine for this test. + // Note dummyflasher registers multiple masters by default, so replace + // get_region for each of them. + flashctx.mst->spi.get_region = &get_protected_region; + flashctx.mst->opaque.get_region = &get_protected_region; + + printf("%s started.\n", current_test_case->erase_test_name); + int ret = flashrom_flash_erase(&flashctx); + printf("%s returned %d.\n", current_test_case->erase_test_name, ret); + + int chip_erased = !memcmp(g_state.buf, current_test_case->erased_buf, MOCK_CHIP_SIZE); + + int eraseblocks_in_order = !memcmp(g_state.eraseblocks_actual, + current_test_case->eraseblocks_expected, + current_test_case->eraseblocks_expected_ind * sizeof(struct erase_invoke)); + + int eraseblocks_invocations = (g_state.eraseblocks_actual_ind == + current_test_case->eraseblocks_expected_ind); + + if (chip_erased) + printf("Erased chip memory state for %s is CORRECT\n", + current_test_case->erase_test_name); + else + printf("Erased chip memory state for %s is WRONG\n", + current_test_case->erase_test_name); + + if (eraseblocks_in_order) + printf("Eraseblocks order of invocation for %s is CORRECT\n", + current_test_case->erase_test_name); + else + printf("Eraseblocks order of invocation for %s is WRONG\n", + current_test_case->erase_test_name); + + if (eraseblocks_invocations) + printf("Eraseblocks number of invocations for %s is CORRECT\n", + current_test_case->erase_test_name); + else + printf("Eraseblocks number of invocations for %s is WRONG, expected %d actual %d\n", + current_test_case->erase_test_name, + current_test_case->eraseblocks_expected_ind, + g_state.eraseblocks_actual_ind); + + all_erase_tests_result |= ret; + all_erase_tests_result |= !chip_erased; + all_erase_tests_result |= !eraseblocks_in_order; + all_erase_tests_result |= !eraseblocks_invocations; + + teardown_chip(&layout); + + assert_int_equal(0, all_erase_tests_result); +} + +/* + * Runs the test cases that use protected flash regions (regions returned from + * get_flash_region() where write_prot is true) when the runtime flag to avoid + * writing to those regions is enabled. + * + * These tests verify that no protected region is written, i.e. protected region + * memory state stays untouched. + */ +static void write_unwritable_regions_skipflag_on_test_success(void **state) { + struct test_case* current_test_case = *state; + + int all_write_tests_result = 0; + struct flashrom_flashctx flashctx = { 0 }; + uint8_t newcontents[MIN_BUF_SIZE]; + const char *param = ""; /* Default values for all params. */ + + struct flashrom_layout *layout; + + setup_chip(&flashctx, &layout, param, current_test_case); + memcpy(&newcontents, current_test_case->written_buf, MOCK_CHIP_SIZE); + + // This test needs special block erase to emulate protected regions. + memcpy(g_test_erase_injector, + (erasefunc_t *const[]){ + block_erase_chip_with_protected_region_1, + block_erase_chip_with_protected_region_2, + block_erase_chip_with_protected_region_3, + block_erase_chip_with_protected_region_4, + block_erase_chip_with_protected_region_5, + }, + sizeof(g_test_erase_injector) + ); + + flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS, true); + flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNREADABLE_REGIONS, true); + + // We use dummyflasher programmer in tests, but for this test we need to + // replace dummyflasher's default get_region fn with test one. + // The rest of master struct is fine for this test. + // Note dummyflasher registers multiple masters by default, so replace + // get_region for each of them. + flashctx.mst->spi.get_region = &get_protected_region; + flashctx.mst->opaque.get_region = &get_protected_region; + + printf("%s started.\n", current_test_case->write_test_name); + int ret = flashrom_image_write(&flashctx, &newcontents, MIN_BUF_SIZE, NULL); + printf("%s returned %d.\n", current_test_case->write_test_name, ret); + + int chip_written = !memcmp(g_state.buf, current_test_case->written_protected_buf, MOCK_CHIP_SIZE); + + if (chip_written) + printf("Written chip memory state for %s is CORRECT\n", + current_test_case->write_test_name); + else + printf("Written chip memory state for %s is WRONG\n", + current_test_case->write_test_name); + + all_write_tests_result |= ret; + all_write_tests_result |= !chip_written; + + teardown_chip(&layout); + + assert_int_equal(0, all_write_tests_result); +} + +static void test_erase_fails_for_unwritable_region(void **state) { + struct flashrom_flashctx flashctx = { + .chip = &chip_1_2_4_8_16, + }; + assert_int_equal(0, programmer_init(&programmer_dummy, "")); + flashctx.mst = &registered_masters[0]; + + flashctx.mst->spi.get_region = &get_protected_region; + flashctx.mst->opaque.get_region = &get_protected_region; + flashrom_flag_set(&flashctx, FLASHROM_FLAG_SKIP_UNWRITABLE_REGIONS, false); + + /* Ask to erase one byte at the end of the unprotected region and one byte + * at the beginning of the protected one. If the check for unwritable regions + * wrongly treats the upper bound as exclusive, it will incorrectly try + * to erase inside the protected region. */ + struct flashrom_layout *layout; + flashrom_layout_new(&layout); + flashrom_layout_add_region(layout, 7, 8, "protected"); + flashrom_layout_include_region(layout, "protected"); + flashrom_layout_set(&flashctx, layout); + + int ret = flashrom_flash_erase(&flashctx); + + assert_int_equal(0, programmer_shutdown()); + flashrom_layout_release(layout); + + assert_int_not_equal(ret, 0); +} diff --git a/tests/tests.c b/tests/tests.c index aa8353a..165390a 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -514,6 +514,16 @@ ret |= _cmocka_run_group_tests("erase_func_algo.c tests", erase_func_algo_tests, n_erase_tests, NULL, NULL); free(erase_func_algo_tests); + size_t n_erase_protected_region_tests; + struct CMUnitTest *erase_protected_region_algo_tests + = get_erase_protected_region_algo_tests(&n_erase_protected_region_tests); + ret |= _cmocka_run_group_tests("erase_func_algo.c protected region tests", + erase_protected_region_algo_tests, + n_erase_protected_region_tests, + NULL, + NULL); + free(erase_protected_region_algo_tests); + // Write-protect group should run last. const struct CMUnitTest chip_wp_tests[] = { cmocka_unit_test(invalid_wp_range_dummyflasher_test_success), diff --git a/tests/tests.h b/tests/tests.h index c22da12..8cf8be0 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -107,6 +107,7 @@ /* erase_func_algo.c */ struct CMUnitTest *get_erase_func_algo_tests(size_t *num_tests); +struct CMUnitTest *get_erase_protected_region_algo_tests(size_t *num_tests); void erase_function_algo_test_success(void **state); void write_function_algo_test_success(void **state); -- 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?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 18 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: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in flashrom[main]: flashchips: Add support for chip model GD25LQ128E/GD25LB128E/GD25LR12...
by Victor Lim (Code Review) 09 Jun '24

09 Jun '24

Victor Lim has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/83012?usp=email ) Change subject: flashchips: Add support for chip model GD25LQ128E/GD25LB128E/GD25LR128E/GD25LQ128D/GD25LQ128C ...................................................................... flashchips: Add support for chip model GD25LQ128E/GD25LB128E/GD25LR128E/GD25LQ128D/GD25LQ128C Adding GD25LQ128E/GD25LB128E/GD25LR128E/GD25LQ128D/GD25LQ128C to flashchip.c These part # sharing the same ID, form, fit, function, produced with different process node: 1.8V 128Mbit Datasheet Link: https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20230926/DS-0… https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20220714/DS-0… Change-Id: I8e8ad6bd69e2159b7cc2b64a7a8c7fcb1399294d Signed-off-by: Victor Lim <vlim(a)gigadevice.com> --- M flashchips.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/12/83012/1 diff --git a/flashchips.c b/flashchips.c index a5d9ec2..cc9c9da 100644 --- a/flashchips.c +++ b/flashchips.c @@ -6587,7 +6587,7 @@ { .vendor = "GigaDevice", - .name = "GD25LQ128C/GD25LQ128D/GD25LQ128E", + .name = "GD25LQ128E/GD25LB128E/GD25LR128E/GD25LQ128D/GD25LQ128C", .bustype = BUS_SPI, .manufacture_id = GIGADEVICE_ID, .model_id = GIGADEVICE_GD25LQ128CD, -- To view, visit https://review.coreboot.org/c/flashrom/+/83012?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: I8e8ad6bd69e2159b7cc2b64a7a8c7fcb1399294d Gerrit-Change-Number: 83012 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com>

1 0

[M] Change in flashrom[main]: flashchips: add support for chip model Winbond W25Q16JV_M
by Anastasia Klimchuk (Code Review) 08 Jun '24

08 Jun '24

Attention is currently required from: Michael Heimpold, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change by Michael Heimpold. ( https://review.coreboot.org/c/flashrom/+/82715?usp=email ) Change subject: flashchips: add support for chip model Winbond W25Q16JV_M ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/82715?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: Ida1ceb5fe31411bef647e5133c5bd0bdb02d7704 Gerrit-Change-Number: 82715 Gerrit-PatchSet: 2 Gerrit-Owner: Michael Heimpold <mhei(a)heimpold.de> 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: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Michael Heimpold <mhei(a)heimpold.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Sat, 08 Jun 2024 08:47:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2024