January 2023 - flashrom-gerrit

[S] Change in flashrom[master]: tests: Fix warnings for 32-bit Linux
by Anastasia Klimchuk (Code Review) 17 Jan '23

17 Jan '23

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/72038 ) Change subject: tests: Fix warnings for 32-bit Linux ...................................................................... tests: Fix warnings for 32-bit Linux Unit tests had int-to-pointer-cast warnings for 32-bit Linux environment, and since warnings are treated as errors this failed build with unit tests. Ticket: https://ticket.coreboot.org/issues/407 Change-Id: I9ec5d37cc038171afc67a69ea9a6885deb8fa4a8 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/linux_spi.c 1 file changed, 18 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/72038/1 diff --git a/tests/linux_spi.c b/tests/linux_spi.c index 1009634..1b653fe 100644 --- a/tests/linux_spi.c +++ b/tests/linux_spi.c @@ -23,14 +23,14 @@ /* First message has write array and write count */ unsigned int writecnt = msg[0].len; - unsigned char *writearr = (unsigned char *)msg[0].tx_buf; + unsigned char *writearr = (unsigned char *)(uintptr_t)msg[0].tx_buf; /* Second message has read array and read count */ unsigned int readcnt = msg[1].len; /* Detect probing */ if (writecnt == 1 && writearr[0] == JEDEC_RDID && readcnt == 3) { /* We need to populate read array. */ - unsigned char *readarr = (unsigned char *)msg[1].rx_buf; + unsigned char *readarr = (unsigned char *)(uintptr_t)msg[1].rx_buf; readarr[0] = 0xEF; /* WINBOND_NEX_ID */ readarr[1] = 0x40; /* WINBOND_NEX_W25Q128_V left byte */ readarr[2] = 0x18; /* WINBOND_NEX_W25Q128_V right byte */ -- To view, visit https://review.coreboot.org/c/flashrom/+/72038 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9ec5d37cc038171afc67a69ea9a6885deb8fa4a8 Gerrit-Change-Number: 72038 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: opaque: make opaque programmers expose register read/write capability
by Nikolai Artemiev (Code Review) 17 Jan '23

17 Jan '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66032 ) Change subject: opaque: make opaque programmers expose register read/write capability ...................................................................... Patch Set 3: (1 comment) File ichspi.c: https://review.coreboot.org/c/flashrom/+/66032/comment/d7b8cf43_906bceb6 PS2, Line 1413: static bool ich_hwseq_can_read_register(const struct flashctx *flash, enum flash_reg reg) : { : return reg == STATUS1; : } : : static bool ich_hwseq_can_write_register(const struct flashctx *flash, enum flash_reg reg) : { : return reg == STATUS1; : } > Possibly the programmer fills a instance of `struct reg_bit_info` and the writeprotect implementatio […] It might be better to use functions instead of structs, given the many-to-one relationship between devices and drivers. I.e. we could have multiple devices with different register access capabilities that use the same driver. This doesn't apply to the ich hwseq master afaik, but it could if we need to use this abstraction for another programmer. -- To view, visit https://review.coreboot.org/c/flashrom/+/66032 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9ec34a8d6f55a76f890e0066d0b896560331ede0 Gerrit-Change-Number: 66032 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 17 Jan 2023 03:05:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: opaque: make opaque programmers expose register read/write capability
by Nikolai Artemiev (Code Review) 17 Jan '23

17 Jan '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66032 ) Change subject: opaque: make opaque programmers expose register read/write capability ...................................................................... Patch Set 3: (1 comment) This change is ready for review. File writeprotect.c: https://review.coreboot.org/c/flashrom/+/66032/comment/9b8003bb_86c55cd5 PS2, Line 231: * TODO: check if the programmer supports writing the register that the : * bit is in. For example, some chipsets may only allow SR1 to be : * written. > maybe keep the comment just slightly reworded. Done -- To view, visit https://review.coreboot.org/c/flashrom/+/66032 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9ec34a8d6f55a76f890e0066d0b896560331ede0 Gerrit-Change-Number: 66032 Gerrit-PatchSet: 3 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 17 Jan 2023 02:57:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: ch347t_spi.c: Move ch347_device struct to flashctx
by Anastasia Klimchuk (Code Review) 17 Jan '23

17 Jan '23

Attention is currently required from: Angel Pons, Nicholas Chin. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71066 ) Change subject: ch347t_spi.c: Move ch347_device struct to flashctx ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Nicholas, thank you so much for removing global state! I have a question: is there any way this can be squashed with the previous patch? So that new programmer would be introduced without global state from the very beginning? We are currently actively removing global state from everywhere in flashrom (although some bits left still). For new programmer, I would try as much as possible to introduce it without global state. You can use `Co-authored-by` tag in commit message to document that the patch is a collaboration of two engineers. In fact, I think you can use `Co-authored-by` anyway: it is already a collaboration ;) -- To view, visit https://review.coreboot.org/c/flashrom/+/71066 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If3ff96dedd2b591200dd69f54df753668853beca Gerrit-Change-Number: 71066 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Tue, 17 Jan 2023 02:51:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: Add initial CH347T SPI programmer
by Anastasia Klimchuk (Code Review) 17 Jan '23

17 Jan '23

Attention is currently required from: Angel Pons, qianfan, Nicholas Chin. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70529 ) Change subject: Add initial CH347T SPI programmer ...................................................................... Patch Set 9: (1 comment) Patchset: PS9: There a few things need to be added to this patch: 1) add new programmer to meson build system (currently only added to make) 2) add man page entry for new programmer 3) add to test_build.sh 4) if you guys agree: MAINTAINERS file (see also my comment in CB:70573). It would be great to have both of you there! This is a coincidence, but: there is one more new programmer being under review at the moment in CB:71801 . The programmer is independent of this one, but it can be a useful example as of all the places that need to be updated when introducing new programmer. Thank you for your work! -- To view, visit https://review.coreboot.org/c/flashrom/+/70529 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6da5e308af76693e60308c8165349128e517e09a Gerrit-Change-Number: 70529 Gerrit-PatchSet: 9 Gerrit-Owner: qianfan <qianfanguijin(a)163.com> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Comment-Date: Tue, 17 Jan 2023 02:44:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: ch347_spi.c: Add initial support for the WCH CH347
by Anastasia Klimchuk (Code Review) 17 Jan '23

17 Jan '23

Attention is currently required from: Thomas Heijligen, qianfan, Nicholas Chin. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70573 ) Change subject: ch347_spi.c: Add initial support for the WCH CH347 ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > Giving this -1 for now to make sure this doesn't get submitted without CB:70529 (Qianfan's port whic […] Nicholas, just to check: is this patch still needed? It seems like CB:70529 + CB:71066 add new programmer? And I see CB:70529 is signed-off by both of you (and tested by both of you). So I am wondering is this patch still needed. -- To view, visit https://review.coreboot.org/c/flashrom/+/70573 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I31b86c41076cc45d4a416a73fa1131350fb745ba Gerrit-Change-Number: 70573 Gerrit-PatchSet: 3 Gerrit-Owner: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Alexander Goncharov <chat(a)joursoir.net> Gerrit-CC: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: qianfan <qianfanguijin(a)163.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Tue, 17 Jan 2023 02:38:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Change chip restore data type from uint8_t to void ptr
by Nikolai Artemiev (Code Review) 17 Jan '23

17 Jan '23

Attention is currently required from: Nikolai Artemiev. Hello build bot (Jenkins), Edward O'Callaghan, Sergii Dmytruk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70349 to look at the new patch set (#8). Change subject: tree/: Change chip restore data type from uint8_t to void ptr ...................................................................... tree/: Change chip restore data type from uint8_t to void ptr Chip restore callbacks currently are used by - spi25_statusreg.c unlock functions to restore status register 1. - s25f.c to restore config register 3. Both of these cases only need to save a single uint8_t value to restore the original chip state, however storing a void pointer will allow more flexible chip restore behaviour. In particular, it will allow flashrom_wp_cfg objects to be saved and restored, enabling writeprotect-based unlocking. BUG=b:237485865,b:247421511 BRANCH=none TEST=Tested on grunt DUT (prog: sb600spi, flash: W25Q128.W): `flashrom --wp-range 0x0,0x1000000 \ flashrom --wp-status # Result: range=0x0,0x1000000 \ flashrom -w random.bin # Result: success \ flashrom -v random.bin # Result: success \ flashrom --wp-status # Result: range=0x0,0x1000000` Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> --- M flashrom.c M include/flash.h M s25f.c M spi25_statusreg.c 4 files changed, 57 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/49/70349/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/70349 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I311b468a4b0349f4da9584c12b36af6ec2394527 Gerrit-Change-Number: 70349 Gerrit-PatchSet: 8 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: programmer: Add bitbanging programmer driver for Linux libgpiod
by Anastasia Klimchuk (Code Review) 17 Jan '23

17 Jan '23

Attention is currently required from: Felix Singer, Thomas Heijligen, Angel Pons, Steve Markgraf. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71801 ) Change subject: programmer: Add bitbanging programmer driver for Linux libgpiod ...................................................................... Patch Set 4: Code-Review+1 (7 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/71801/comment/a333903b_67f8cf6e PS4, Line 9: old smartphone This is cool! :) https://review.coreboot.org/c/flashrom/+/71801/comment/a9680929_3e85fee5 PS4, Line 17: All arguments are required. I think this can be mentioned in the manpage too. https://review.coreboot.org/c/flashrom/+/71801/comment/9c368954_14d50915 PS4, Line 19: Reading of a 2048 kB flash chip on a Qualcomm MSM8916 SoC @800 MHz: I suggest a little re-phrase of this, just to make clear what was the testing scenario that you ran. Maybe: Tested by reading of a 2048 kB flash chip on a Qualcomm MSM8916 SoC @800 MHz, ran the following command: <exact command line you ran> Output: Found GigaDevice flash chip .... If you tested other scenarios (write, erase, verify) it would be great to mention here too. Thanks! Patchset: PS3: > What I had missed so far is that there is a second build system. […] You are right that we have two build systems [still], and those are make and meson. You actually covered them both in your patch! just one small comment left (see my comment in README). `test_build.sh` is a script used for CI, and the script is running builds + tests for both build systems. Yes, you need to add new programmer to `test_build.sh` into both `make_programmer_opts` and `meson_programmer_opts` and once you add it, Jenkins will start building your patch after you push it to Gerrit. As you see, every time you push a patchset, Jenkins responds with +1 (or -1) and gives a link to build logs. If by any chance you get -1, you can look into the logs why. The bit I am not sure about: how to install libgpiod for CI environment. It would be great if Thomas and Felix could comment on this? Thank you! Patchset: PS4: Thomas, Felix, it would be great if you could have a look, thanks! File MAINTAINERS: https://review.coreboot.org/c/flashrom/+/71801/comment/750588f1_e50262dc PS4, Line 137: LINUX GPIOD We keep this in abc order, so you need to swap with previous entry (LINUX MTD). File README: https://review.coreboot.org/c/flashrom/+/71801/comment/8e8381b9_3c1748a8 PS4, Line 76: libgpiod-dev (if you want support for Linux GPIO devices) Could you please add this info to meson doc too? Here: Documentation/building.md Thank you! -- To view, visit https://review.coreboot.org/c/flashrom/+/71801 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icad3eb7764f28feaea51bda3a7893da724c86d06 Gerrit-Change-Number: 71801 Gerrit-PatchSet: 4 Gerrit-Owner: Steve Markgraf <steve(a)steve-m.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Steve Markgraf <steve(a)steve-m.de> Gerrit-Comment-Date: Tue, 17 Jan 2023 02:03:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Steve Markgraf <steve(a)steve-m.de> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tests/chip{_wp}.c: Avoid unnecessary heap allocations
by Edward O'Callaghan (Code Review) 17 Jan '23

17 Jan '23

Attention is currently required from: Anastasia Klimchuk. Hello build bot (Jenkins), Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71917 to look at the new patch set (#2). Change subject: tests/chip{_wp}.c: Avoid unnecessary heap allocations ...................................................................... tests/chip{_wp}.c: Avoid unnecessary heap allocations Just use a static string on the stack. Change-Id: Ic6cb4f32094ae5868912ebcffc8ab21026c48d32 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/chip.c M tests/chip_wp.c 2 files changed, 23 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/17/71917/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71917 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic6cb4f32094ae5868912ebcffc8ab21026c48d32 Gerrit-Change-Number: 71917 Gerrit-PatchSet: 2 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tests/chip: Add non-aligned write within a region unit-test
by Edward O'Callaghan (Code Review) 17 Jan '23

17 Jan '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71659 ) ( 5 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tests/chip: Add non-aligned write within a region unit-test ...................................................................... tests/chip: Add non-aligned write within a region unit-test A written region that is sized below that of the erasure granularity can result in a incorrectly read region that does not include prior content within the region before the write op. This was dealt with in ChromeOS downstream by expanding out the read to match the erase granularity however does not seem to impact upstream. Add a unit-test to avoid regression as this is important behaviour to cover. Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/71659 Reviewed-by: Sam McNally <sammc(a)google.com> Reviewed-by: Evan Benn <evanbenn(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M tests/chip.c M tests/tests.c M tests/tests.h 3 files changed, 114 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Sam McNally: Looks good to me, approved Evan Benn: Looks good to me, but someone else must approve diff --git a/tests/chip.c b/tests/chip.c index 580f4ea..e82719e 100644 --- a/tests/chip.c +++ b/tests/chip.c @@ -418,6 +418,97 @@ free(newcontents); } +void write_nonaligned_region_with_dummyflasher_test_success(void **state) +{ + (void) state; /* unused */ + + static struct io_mock_fallback_open_state data = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock chip_io = { + .fallback_open_state = &data, + }; + + struct flashrom_flashctx flashctx = { 0 }; + struct flashrom_layout *layout; + struct flashchip mock_chip = chip_W25Q128_V; + const uint32_t mock_chip_size = mock_chip.total_size * KiB; + /* + * Dummyflasher is capable to emulate W25Q128.V, so we ask it to do this. + * Nothing to mock, dummy is taking care of this already. + */ + char *param_dup = strdup("bus=spi,emulate=W25Q128FV"); + + /* FIXME: MOCK_CHIP_CONTENT is buggy within setup_chip, it should also + * not be either 0x00 or 0xFF as those are specific values related to + * either a erased chip or zero'ed heap thus ambigous. + */ +#define MOCK_CHIP_SUBREGION_CONTENTS 0xCC + /** + * Step 0 - Prepare newcontents as contiguous sample data bytes as follows: + * {MOCK_CHIP_SUBREGION_CONTENTS, [..]}. + */ + uint8_t *newcontents = calloc(1, mock_chip_size); + assert_non_null(newcontents); + memset(newcontents, MOCK_CHIP_SUBREGION_CONTENTS, mock_chip_size); + + setup_chip(&flashctx, &layout, &mock_chip, param_dup, &chip_io); + /* Expect to verify only the non-aligned write operation within the region. */ + flashrom_flag_set(&flashctx, FLASHROM_FLAG_VERIFY_AFTER_WRITE, true); + flashrom_flag_set(&flashctx, FLASHROM_FLAG_VERIFY_WHOLE_CHIP, false); + + /** + * Prepare mock chip content and release setup_chip() layout for our + * custom ones. + */ + assert_int_equal(0, flashrom_image_write(&flashctx, newcontents, mock_chip_size, NULL)); + flashrom_layout_release(layout); + + /** + * Create region smaller than erase granularity of chip. + */ + printf("Creating custom region layout... "); + assert_int_equal(0, flashrom_layout_new(&layout)); + printf("Adding and including region0... "); + assert_int_equal(0, flashrom_layout_add_region(layout, 0, (1 * KiB), "region0")); + assert_int_equal(0, flashrom_layout_include_region(layout, "region0")); + flashrom_layout_set(&flashctx, layout); + printf("Subregion layout configuration done.\n"); + + /** + * Step 1 - Modify newcontents as non-contiguous sample data bytes as follows: + * 0xAA 0xAA {MOCK_CHIP_SUBREGION_CONTENTS}, [..]}. + */ + printf("Subregion chip write op..\n"); + memset(newcontents, 0xAA, 2); + assert_int_equal(0, flashrom_image_write(&flashctx, newcontents, mock_chip_size, NULL)); + printf("Subregion chip write op done.\n"); + + /** + * FIXME: A 'NULL' layout should indicate a default layout however this + * causes a crash for a unknown reason. For now prepare a new default + * layout of the entire chip. flashrom_layout_set(&flashctx, NULL); // use default layout. + */ + flashrom_layout_release(layout); + assert_int_equal(0, flashrom_layout_new(&layout)); + assert_int_equal(0, flashrom_layout_add_region(layout, 0, mock_chip_size - 1, "entire")); + assert_int_equal(0, flashrom_layout_include_region(layout, "entire")); + flashrom_layout_set(&flashctx, layout); + + /** + * Expect a verification pass that the previous content within the region, however + * outside the region write length, is untouched. + */ + printf("Entire chip verify op..\n"); + assert_int_equal(0, flashrom_image_verify(&flashctx, newcontents, mock_chip_size)); + printf("Entire chip verify op done.\n"); + + teardown(&layout); + free(param_dup); + free(newcontents); +} + static size_t verify_chip_fread(void *state, void *buf, size_t size, size_t len, FILE *fp) { /* diff --git a/tests/tests.c b/tests/tests.c index a1dcaca..0912f35 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -480,6 +480,7 @@ cmocka_unit_test(read_chip_with_dummyflasher_test_success), cmocka_unit_test(write_chip_test_success), cmocka_unit_test(write_chip_with_dummyflasher_test_success), + cmocka_unit_test(write_nonaligned_region_with_dummyflasher_test_success), cmocka_unit_test(verify_chip_test_success), cmocka_unit_test(verify_chip_with_dummyflasher_test_success), }; diff --git a/tests/tests.h b/tests/tests.h index 6a12fdb..bdcfdae 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -80,6 +80,7 @@ void read_chip_with_dummyflasher_test_success(void **state); void write_chip_test_success(void **state); void write_chip_with_dummyflasher_test_success(void **state); +void write_nonaligned_region_with_dummyflasher_test_success(void **state); void verify_chip_test_success(void **state); void verify_chip_with_dummyflasher_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 7 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2023