March 2022 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: flashrom.c: Implement file-based locking semantics
by Daniel Campello (Code Review) March 30, 2022

March 30, 2022

Attention is currently required from: Hung-Te Lin, Nico Huber, Subrata Banik, Edward O'Callaghan. Daniel Campello has uploaded a new patch set (#14) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/62213 ) Change subject: flashrom.c: Implement file-based locking semantics ...................................................................... flashrom.c: Implement file-based locking semantics This upstreams the ChromiumOS implementation of file-based locking for multiple instances of flashrom that could be spawned either from libflashrom (perhaps by fwupd for example) and user cli as another example. Since flashrom is programming singleton state of hardware from userspace there is no way to exclusively own the address space and therefore a file-based locking semantic is considered here. BUG=b:217629892,b:215255210 BRANCH=none TEST=nm -gD /build/brya/usr/lib64/libflashrom.so | grep "flock" Test futility update with multiple instances of flashrom running. Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M Makefile A big_lock.c A big_lock.h A file_lock.c M flashrom.c A ipc_lock.h M meson.build M tests/chip.c M tests/include/test.h M tests/io_mock.h M tests/lifecycle.c 11 files changed, 462 insertions(+), 12 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/13/62213/14 -- To view, visit https://review.coreboot.org/c/flashrom/+/62213 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I19cb4e3bf14caeb67c3e8100a20395b264c5113a Gerrit-Change-Number: 62213 Gerrit-PatchSet: 14 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests/lifecycle.c: Deduce out io-setup-teardown do-pattern
by Daniel Campello (Code Review) March 30, 2022

March 30, 2022

Attention is currently required from: Edward O'Callaghan. Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63231 ) Change subject: tests/lifecycle.c: Deduce out io-setup-teardown do-pattern ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/63231 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icc00acd980a027337acb079f5afc3cccdfe4c765 Gerrit-Change-Number: 63231 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Thu, 31 Mar 2022 00:37:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: ichspi: Add Jasper Lake support
by Sam McNally (Code Review) March 30, 2022

March 30, 2022

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62282 ) Change subject: ichspi: Add Jasper Lake support ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/62282 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib942d0b8942fe0a991b2af0b187414818485153d Gerrit-Change-Number: 62282 Gerrit-PatchSet: 8 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 31 Mar 2022 00:20:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests/lifecycle.c: Deduce out io-setup-teardown do-pattern
by Edward O'Callaghan (Code Review) March 30, 2022

March 30, 2022

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/63231 ) Change subject: tests/lifecycle.c: Deduce out io-setup-teardown do-pattern ...................................................................... tests/lifecycle.c: Deduce out io-setup-teardown do-pattern The following do-block is quite error prone to do manually, ``` io_mock_register(&XXX_io); run_probe_lifecycle(state, &XXX, "", ".."); io_mock_register(NULL); ```. Hence, deduce out the common pattern and fold up into the common worker function to handle state machine setup and teardown in a consistent way. BUG=b:227521116 TEST=`ninja test`. Change-Id: Icc00acd980a027337acb079f5afc3cccdfe4c765 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/lifecycle.c 1 file changed, 19 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/31/63231/1 diff --git a/tests/lifecycle.c b/tests/lifecycle.c index 282a6b5..4f7f248 100644 --- a/tests/lifecycle.c +++ b/tests/lifecycle.c @@ -35,7 +35,7 @@ flashrom_flash_release(flashctx); /* cleanup */ } -static void run_lifecycle(void **state, const struct programmer_entry *prog, +static void run_lifecycle(void **state, const struct io_mock *io, const struct programmer_entry *prog, const char *param, const char *const chip_name, void (*action)(const struct programmer_entry *prog, struct flashrom_programmer *flashprog, @@ -43,6 +43,8 @@ { (void) state; /* unused */ + io_mock_register(io); + struct flashrom_programmer *flashprog; char *param_dup = strdup(param); @@ -58,27 +60,30 @@ printf("... flashrom_programmer_shutdown for programmer=%s successful\n", prog->name); free(param_dup); + + io_mock_register(NULL); } -static void run_basic_lifecycle(void **state, const struct programmer_entry *prog, const char *param) +static void run_basic_lifecycle(void **state, const struct io_mock *io, + const struct programmer_entry *prog, const char *param) { /* Basic lifecycle only does init and shutdown, * so neither chip name nor action is needed. */ - run_lifecycle(state, prog, param, NULL /* chip_name */, NULL /* action */); + run_lifecycle(state, io, prog, param, NULL /* chip_name */, NULL /* action */); } -static void run_probe_lifecycle(void **state, const struct programmer_entry *prog, - const char *param, const char *const chip_name) +static void run_probe_lifecycle(void **state, const struct io_mock *io, + const struct programmer_entry *prog, const char *param, const char *const chip_name) { /* Each probe lifecycle should run independently, without cache. */ clear_spi_id_cache(); - run_lifecycle(state, prog, param, chip_name, &probe_chip); + run_lifecycle(state, io, prog, param, chip_name, &probe_chip); } void dummy_basic_lifecycle_test_success(void **state) { #if CONFIG_DUMMY == 1 - run_basic_lifecycle(state, &programmer_dummy, "bus=parallel+lpc+fwh+spi"); + run_basic_lifecycle(state, NULL, &programmer_dummy, "bus=parallel+lpc+fwh+spi"); #else skip(); #endif @@ -87,7 +92,7 @@ void dummy_probe_lifecycle_test_success(void **state) { #if CONFIG_DUMMY == 1 - run_probe_lifecycle(state, &programmer_dummy, "bus=spi,emulate=W25Q128FV", "W25Q128.V"); + run_probe_lifecycle(state, NULL, &programmer_dummy, "bus=spi,emulate=W25Q128FV", "W25Q128.V"); #else skip(); #endif @@ -96,7 +101,7 @@ void nicrealtek_basic_lifecycle_test_success(void **state) { #if CONFIG_NICREALTEK == 1 - run_basic_lifecycle(state, &programmer_nicrealtek, ""); + run_basic_lifecycle(state, NULL, &programmer_nicrealtek, ""); #else skip(); #endif @@ -188,11 +193,7 @@ char raiden_debug_param[12]; snprintf(raiden_debug_param, 12, "address=%d", USB_DEVICE_ADDRESS); - io_mock_register(&raiden_debug_io); - - run_basic_lifecycle(state, &programmer_raiden_debug_spi, raiden_debug_param); - - io_mock_register(NULL); + run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param); #else skip(); #endif @@ -229,11 +230,7 @@ .libusb_control_transfer = dediprog_libusb_control_transfer, }; - io_mock_register(&dediprog_io); - - run_basic_lifecycle(state, &programmer_dediprog, "voltage=3.5V"); - - io_mock_register(NULL); + run_basic_lifecycle(state, &dediprog_io, &programmer_dediprog, "voltage=3.5V"); #else skip(); #endif @@ -306,11 +303,7 @@ .fclose = linux_mtd_fclose, }; - io_mock_register(&linux_mtd_io); - - run_probe_lifecycle(state, &programmer_linux_mtd, "", "Opaque flash chip"); - - io_mock_register(NULL); + run_probe_lifecycle(state, &linux_mtd_io, &programmer_linux_mtd, "", "Opaque flash chip"); #else skip(); #endif @@ -360,11 +353,7 @@ .ioctl = linux_spi_ioctl, }; - io_mock_register(&linux_spi_io); - - run_probe_lifecycle(state, &programmer_linux_spi, "dev=/dev/null", "W25Q128.V"); - - io_mock_register(NULL); + run_probe_lifecycle(state, &linux_spi_io, &programmer_linux_spi, "dev=/dev/null", "W25Q128.V"); #else skip(); #endif @@ -414,11 +403,8 @@ .read = realtek_mst_read, .write = realtek_mst_write, }; - io_mock_register(&realtek_mst_io); - run_basic_lifecycle(state, &programmer_realtek_mst_i2c_spi, "bus=254,enter-isp=0"); - - io_mock_register(NULL); + run_basic_lifecycle(state, &realtek_mst_io, &programmer_realtek_mst_i2c_spi, "bus=254,enter-isp=0"); #else skip(); #endif /* CONFIG_REALTEK_I2C_SPI */ -- To view, visit https://review.coreboot.org/c/flashrom/+/63231 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icc00acd980a027337acb079f5afc3cccdfe4c765 Gerrit-Change-Number: 63231 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: tests: assert pathname and flags when calling open()
by Edward O'Callaghan (Code Review) March 30, 2022

March 30, 2022

Attention is currently required from: Daniel Campello. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63227 ) Change subject: tests: assert pathname and flags when calling open() ...................................................................... Patch Set 4: Code-Review+2 (1 comment) File tests/tests.c: https://review.coreboot.org/c/flashrom/+/63227/comment/ff46978d_57caac24 PS3, Line 85: //assert_non_null(io_state->paths[io_state->noc]); remove? -- To view, visit https://review.coreboot.org/c/flashrom/+/63227 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib46ca5b854c8453ec02ae09f3151cd4d25f988eb Gerrit-Change-Number: 63227 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Wed, 30 Mar 2022 23:04:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests/chips.c: Mock out open() for lock file
by Daniel Campello (Code Review) March 30, 2022

March 30, 2022

Attention is currently required from: Daniel Campello, Anastasia Klimchuk. Daniel Campello has uploaded a new patch set (#5) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/62324 ) Change subject: tests/chips.c: Mock out open() for lock file ...................................................................... tests/chips.c: Mock out open() for lock file Change-Id: I527a266233648b0eef11a89108e82d0a008eeb8d Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Co-Author: Daniel Campello <campello(a)chromium.org> Signed-off-by: Daniel Campello <campello(a)chromium.org> --- M tests/chip.c 1 file changed, 99 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/62324/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/62324 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I527a266233648b0eef11a89108e82d0a008eeb8d Gerrit-Change-Number: 62324 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: assert pathname and flags when calling open()
by Daniel Campello (Code Review) March 30, 2022

March 30, 2022

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/63227 to look at the new patch set (#4). Change subject: tests: assert pathname and flags when calling open() ...................................................................... tests: assert pathname and flags when calling open() With this change the wrappers for mock and friends are able to take an optional io_mock_open_state struct to assert expected pathnames and flags whenever an open operation is called. Based partially on https://review.coreboot.org/c/flashrom/+/62319/5 BUG=b:227404721,b:217629892,b:215255210 TEST=./test_build.sh; FEATURES=test emerge-amd64-generic flashrom BRANCH=none Signed-off-by: Daniel Campello <campello(a)chromium.org> Co-Author: Edward O'Callaghan <quasisec(a)google.com> Change-Id: Ib46ca5b854c8453ec02ae09f3151cd4d25f988eb --- M tests/io_mock.h M tests/lifecycle.c M tests/tests.c 3 files changed, 37 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/63227/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/63227 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib46ca5b854c8453ec02ae09f3151cd4d25f988eb Gerrit-Change-Number: 63227 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: use MOCK_FD instead of NON_ZERO
by Edward O'Callaghan (Code Review) March 30, 2022

March 30, 2022

Attention is currently required from: Daniel Campello. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/63193 ) Change subject: tests: use MOCK_FD instead of NON_ZERO ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/63193 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib6bac051852aea2465665a6fd669b7f5e3772985 Gerrit-Change-Number: 63193 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Daniel Campello <campello(a)chromium.org> Gerrit-Comment-Date: Wed, 30 Mar 2022 23:00:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: assert pathname and flags when calling open()
by Daniel Campello (Code Review) March 30, 2022

March 30, 2022

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/63227 to look at the new patch set (#3). Change subject: tests: assert pathname and flags when calling open() ...................................................................... tests: assert pathname and flags when calling open() With this change the wrappers for mock and friends are able to take an optional io_mock_open_state struct to assert expected pathnames and flags whenever an open operation is called. Based partially on https://review.coreboot.org/c/flashrom/+/62319/5 BUG=b:227404721,b:217629892,b:215255210 TEST=./test_build.sh; FEATURES=test emerge-amd64-generic flashrom BRANCH=none Signed-off-by: Daniel Campello <campello(a)chromium.org> Co-Author: Edward O'Callaghan <quasisec(a)google.com> Change-Id: Ib46ca5b854c8453ec02ae09f3151cd4d25f988eb --- M tests/io_mock.h M tests/lifecycle.c M tests/tests.c 3 files changed, 37 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/27/63227/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/63227 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib46ca5b854c8453ec02ae09f3151cd4d25f988eb Gerrit-Change-Number: 63227 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests/linux_spi: Validate param file path
by Daniel Campello (Code Review) March 30, 2022

March 30, 2022

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Daniel Campello has uploaded a new patch set (#6) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/62319 ) Change subject: tests/linux_spi: Validate param file path ...................................................................... tests/linux_spi: Validate param file path Add path validation of '/dev/null' to open operation. BUG=b:217629892,b:215255210 TEST=`ninja test`. Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Co-Author: Daniel Campello <campello(a)chromium.org> Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: If5d24c65f291c53a35509fea5d2f5b3fdb51c306 --- M tests/lifecycle.c 1 file changed, 6 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/19/62319/6 -- To view, visit https://review.coreboot.org/c/flashrom/+/62319 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: If5d24c65f291c53a35509fea5d2f5b3fdb51c306 Gerrit-Change-Number: 62319 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0