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

Change in flashrom[master]: helpers.c: use unsigned int for bit shifts (ASAN)
by Anastasia Klimchuk (Code Review) 20 Mar '22

20 Mar '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/62862 ) Change subject: helpers.c: use unsigned int for bit shifts (ASAN) ...................................................................... helpers.c: use unsigned int for bit shifts (ASAN) This change addresses the following ASAN error detected in the chromium tree: * ASAN error detected: * ../flashrom-9999/helpers.c:28:13: runtime error: left shift of 1 by 31 places cannot be represented in type 'int' * #0 0x5589a94bb284 in address_to_bits /build/amd64-generic/tmp/portage/sys-apps/flashrom-9999/work/flashrom-9999-build/../flashrom-9999/helpers.c:28:13 * * SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../flashrom-9999/helpers.c:28:13 in BUG=b:224828279 TEST=./test_build.sh; FEATURES=test emerge-amd64-generic flashrom BRANCH=none Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: Ib595f13c29dd5c0775e074801756e4f920b4daaf Reviewed-on: https://review.coreboot.org/c/flashrom/+/62862 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> Reviewed-by: Paul Menzel <paulepanter(a)mailbox.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M helpers.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Angel Pons: Looks good to me, but someone else must approve Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/helpers.c b/helpers.c index 289848d..5b47b68 100644 --- a/helpers.c +++ b/helpers.c @@ -25,7 +25,7 @@ uint32_t address_to_bits(uint32_t addr) { unsigned int lzb = 0; - while (((1 << (31 - lzb)) & ~addr) != 0) + while (((1u << (31 - lzb)) & ~addr) != 0) lzb++; return 32 - lzb; } -- To view, visit https://review.coreboot.org/c/flashrom/+/62862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib595f13c29dd5c0775e074801756e4f920b4daaf Gerrit-Change-Number: 62862 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: add more mock wrappers
by Anastasia Klimchuk (Code Review) 20 Mar '22

20 Mar '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/62844 ) Change subject: tests: add more mock wrappers ...................................................................... tests: add more mock wrappers This change allows for tests to run when the compiler is inlining some other interfaces. This happens when compiling on the chromium chroot environment. * __fgets_chk() is being used instead of fgets() in get_max_kernel_buf_size() on linux_spi.c * __vfprintf_chk() is being used instead of fprintf() in disable_power_management() on power.c * __open64_2() is being used instead of open() in i2c_open_path() on i2c_helper_linux.c BUG=b:224828279 TEST=./test_build.sh; FEATURES=test emerge-volteer flashrom Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: I9776104d655c37891093da08789d37e5e27700de Reviewed-on: https://review.coreboot.org/c/flashrom/+/62844 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Edward O'Callaghan <quasisec(a)chromium.org> --- M tests/meson.build M tests/tests.c 2 files changed, 27 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/tests/meson.build b/tests/meson.build index 610d1c6..90e0df1 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -36,6 +36,7 @@ '-Wl,--wrap=sio_read', '-Wl,--wrap=open', '-Wl,--wrap=open64', + '-Wl,--wrap=__open64_2', '-Wl,--wrap=ioctl', '-Wl,--wrap=read', '-Wl,--wrap=write', @@ -56,7 +57,9 @@ '-Wl,--wrap=fsync', '-Wl,--wrap=fread', '-Wl,--wrap=fgets', + '-Wl,--wrap=__fgets_chk', '-Wl,--wrap=fprintf', + '-Wl,--wrap=__vfprintf_chk', '-Wl,--wrap=fclose', '-Wl,--wrap=feof', '-Wl,--wrap=ferror', diff --git a/tests/tests.c b/tests/tests.c index f1fb3f3..33b2ea2 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -90,6 +90,14 @@ return NON_ZERO; } +int __wrap___open64_2(const char *pathname, int flags) +{ + LOG_ME; + if (get_io() && get_io()->open) + return get_io()->open(get_io()->state, pathname, flags); + return NON_ZERO; +} + int __wrap_ioctl(int fd, unsigned long int request, ...) { LOG_ME; @@ -198,6 +206,14 @@ return NULL; } +char *__wrap___fgets_chk(char *buf, int len, FILE *fp) +{ + LOG_ME; + if (get_io() && get_io()->fgets) + return get_io()->fgets(get_io()->state, buf, len, fp); + return NULL; +} + size_t __wrap_fread(void *ptr, size_t size, size_t nmemb, FILE *fp) { LOG_ME; @@ -250,6 +266,14 @@ return 0; } +int __wrap___vfprintf_chk(FILE *fp, const char *fmt, va_list args) +{ + LOG_ME; + if (get_io() && get_io()->fprintf) + return get_io()->fprintf(get_io()->state, fp, fmt, args); + return 0; +} + int __wrap_fclose(FILE *fp) { LOG_ME; 3 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/62844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9776104d655c37891093da08789d37e5e27700de Gerrit-Change-Number: 62844 Gerrit-PatchSet: 5 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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-MessageType: merged

1 0

Change in flashrom[master]: tests: Add padding to pci_dev struct for ASAN
by Anastasia Klimchuk (Code Review) 20 Mar '22

20 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Daniel Campello, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62845 ) Change subject: tests: Add padding to pci_dev struct for ASAN ...................................................................... Patch Set 4: Code-Review+2 (1 comment) Patchset: PS4: I wrapped the lines in commit message so that they fit into 72 chars width limit. -- To view, visit https://review.coreboot.org/c/flashrom/+/62845 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8 Gerrit-Change-Number: 62845 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Mar 2022 02:05:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add padding to pci_dev struct for ASAN
by Anastasia Klimchuk (Code Review) 20 Mar '22

20 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Daniel Campello, Angel Pons. Anastasia Klimchuk has uploaded a new patch set (#4) to the change originally created by Daniel Campello. ( https://review.coreboot.org/c/flashrom/+/62845 ) Change subject: tests: Add padding to pci_dev struct for ASAN ...................................................................... tests: Add padding to pci_dev struct for ASAN This change addresses the following ASAN error detected in the chromium tree: * ASAN error detected: * ================================================================= * ==12==ERROR: AddressSanitizer: global-buffer-overflow on address 0x55a8a046c916 at pc 0x55a8a038a21d bp 0x7ffd5dbc9ed0 sp 0x7ffd5dbc9ec8 * READ of size 2 at 0x55a8a046c916 thread T0 * #0 0x55a8a038a21c in nicrealtek_init /build/amd64-generic/tmp/por tage/sys-apps/flashrom-9999/work/flashrom-9999-build/../flashrom-9999/ni crealtek.c:119:15 * #1 0x55a8a032f172 in __sanitizer::BufferedStackTrace::UnwindImpl( unsigned long, unsigned long, void*, bool, unsigned int) ??:0:0 * #2 0x55a8a02b65b8 in __asan::ErrorGeneric::Print() ??:0:0 * #3 0x55a8a03294d5 in __asan::ScopedInErrorReport::~ScopedInErrorR eport() ??:0:0 * #4 0x55a8a032c5ae in __asan::ReportGenericError(unsigned long, un signed long, unsigned long, unsigned long, bool, unsigned long, unsigned int, bool) ??:0:0 * #5 0x55a8a032d0f7 in __asan_report_load2 ??:0:0 * * 0x55a8a046c916 is located 18 bytes to the right of global variable 'm ock_pci_dev' defined in '../flashrom-9999/tests/tests.c:50:16' (0x55a8a0 46c900) of size 4 * SUMMARY: AddressSanitizer: global-buffer-overflow (/tmp/portage/sys-a pps/flashrom-9999/work/flashrom-9999-build/tests/flashrom_unit_tests+0x1 9a21c) * Shadow bytes around the buggy address: * 0x0ab5940858d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 * 0x0ab5940858e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 * 0x0ab5940858f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 * 0x0ab594085900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 * 0x0ab594085910: 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 f9 f9 00 00 * =>0x0ab594085920: 04 f9[f9]f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * 0x0ab594085930: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * 0x0ab594085940: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * 0x0ab594085950: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * 0x0ab594085960: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * 0x0ab594085970: f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 f9 * Shadow byte legend (one shadow byte represents 8 application bytes): * Addressable: 00 * Partially addressable: 01 02 03 04 05 06 07 * Heap left redzone: fa * Freed heap region: fd * Stack left redzone: f1 * Stack mid redzone: f2 * Stack right redzone: f3 * Stack after return: f5 * Stack use after scope: f8 * Global redzone: f9 * Global init order: f6 * Poisoned by user: f7 * Container overflow: fc * Array cookie: ac * Intra object redzone: bb * ASan internal: fe * Left alloca redzone: ca * Right alloca redzone: cb * ==12==ABORTING BUG=b:224828279 TEST=./test_build.sh; FEATURES=test emerge-amd64-generic flashrom BRANCH=none Signed-off-by: Daniel Campello <campello(a)chromium.org> Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8 --- M tests/io_mock.h 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/45/62845/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/62845 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8 Gerrit-Change-Number: 62845 Gerrit-PatchSet: 4 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: tests: Add padding to pci_dev struct for ASAN
by Anastasia Klimchuk (Code Review) 20 Mar '22

20 Mar '22

Attention is currently required from: Nico Huber, Edward O'Callaghan, Daniel Campello, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62845 ) Change subject: tests: Add padding to pci_dev struct for ASAN ...................................................................... Patch Set 3: Code-Review+2 (1 comment) Patchset: PS2: > the three outstanding patches block the enablement of tests in the cq as each of them are addressing […] Okay, I see you are not very enthusiastic to fix this comment :) But I am, especially since the situation was already discussed and addressed previously for tests which are using libusb. I created a patch and put it on the top of this one: CB:62948 I decided that my patch can go on the top of this one for two main reasons: mock struct pci_dev has been added a while ago, so this patch is modifying existing struct; and also my patch CB:62948 is introducing dependency for the tests pci/pci.h and I would prefer this to be carefully reviewed to ensure it's all good. -- To view, visit https://review.coreboot.org/c/flashrom/+/62845 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I47943bf70181a9041f287df3ece0f7067a112de8 Gerrit-Change-Number: 62845 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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: Daniel Campello <campello(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Mon, 21 Mar 2022 01:51:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Daniel Campello <campello(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Remove mock struct pci_dev, use real pci symbols in tests
by Anastasia Klimchuk (Code Review) 20 Mar '22

20 Mar '22

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/62948 ) Change subject: tests: Remove mock struct pci_dev, use real pci symbols in tests ...................................................................... tests: Remove mock struct pci_dev, use real pci symbols in tests As a follow up on CB:62845 this patch removes mock struct pci_dev and now tests are including pci/pci.h header and all real symbols from the header. Importantly, this means libpci now becomes a dependency for tests (similar to how libusb is already a dependency for tests as a result of commit f47ff316ec79b014a5a37898d46e78e61acd6b01 ) BUG=b:181803212 TEST=ninja test Change-Id: I1206fbdca392f190066a364376ce0db28071e53c Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/io_mock.h M tests/tests.c 2 files changed, 3 insertions(+), 8 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/48/62948/1 diff --git a/tests/io_mock.h b/tests/io_mock.h index f8f75ad..95bcc6c 100644 --- a/tests/io_mock.h +++ b/tests/io_mock.h @@ -35,20 +35,15 @@ #include <stdio.h> /* - * Explicitly including the header because some tests are using libusb structs + * Explicitly including the headers because some tests are using structs * in depth, opaque symbols are not sufficient. */ #include <libusb.h> +#include <pci/pci.h> /* Address value needs fit into uint8_t. */ #define USB_DEVICE_ADDRESS 19 -/* Define struct pci_dev to avoid dependency on pci.h */ -struct pci_dev { - char padding[18]; - unsigned int device_id; -}; - /* POSIX open() flags, avoiding dependency on fcntl.h */ #define O_RDONLY 0 #define O_WRONLY 1 diff --git a/tests/tests.c b/tests/tests.c index f1fb3f3..111648c 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -48,7 +48,7 @@ } struct pci_dev mock_pci_dev = { - .device_id = NON_ZERO, + .device_id = (short unsigned int) NON_ZERO, }; struct pci_dev *__wrap_pcidev_init(void *devs, int bar) -- To view, visit https://review.coreboot.org/c/flashrom/+/62948 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1206fbdca392f190066a364376ce0db28071e53c Gerrit-Change-Number: 62948 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: cli_classic.c: Use correct format specifier for `size_t`
by Anastasia Klimchuk (Code Review) 20 Mar '22

20 Mar '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/62707 ) Change subject: cli_classic.c: Use correct format specifier for `size_t` ...................................................................... cli_classic.c: Use correct format specifier for `size_t` The return value of `flashrom_flash_getsize()` is of `size_t` type. Fixes: commit 83c5c50f91806df599fa9eebb654832879381772 Change-Id: I8025c0e046cb61e0e115d3786e6e4184190347f1 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/62707 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Patrick Rudolph <siro(a)das-labor.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M cli_classic.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Patrick Rudolph: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/cli_classic.c b/cli_classic.c index dc3a651..b3930d2 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -715,7 +715,7 @@ } if (flash_size) { - printf("%ld\n", flashrom_flash_getsize(fill_flash)); + printf("%zu\n", flashrom_flash_getsize(fill_flash)); goto out_shutdown; } -- To view, visit https://review.coreboot.org/c/flashrom/+/62707 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8025c0e046cb61e0e115d3786e6e4184190347f1 Gerrit-Change-Number: 62707 Gerrit-PatchSet: 2 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Upgrade linux_spi test to run probe lifecycle
by Anastasia Klimchuk (Code Review) 20 Mar '22

20 Mar '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/59742 ) Change subject: tests: Upgrade linux_spi test to run probe lifecycle ...................................................................... tests: Upgrade linux_spi test to run probe lifecycle This test adds a mock for linux_spi ioctl and mocks it for read request. Read buffer is populated with chip manufacture id and chip model id to emulate successful probing. BUG=b:181803212 TEST=ninja test Change-Id: I32d8e972d99b52c2b18f688aa6aeae75dd170f72 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/59742 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> Reviewed-by: Nico Huber <nico.h(a)gmx.de> --- M tests/lifecycle.c M tests/tests.c M tests/tests.h 3 files changed, 30 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Thomas Heijligen: Looks good to me, approved diff --git a/tests/lifecycle.c b/tests/lifecycle.c index e48d94e..282a6b5 100644 --- a/tests/lifecycle.c +++ b/tests/lifecycle.c @@ -15,6 +15,7 @@ #include <include/test.h> #include <string.h> +#include <linux/spi/spidev.h> #include "libflashrom.h" #include "io_mock.h" @@ -315,6 +316,30 @@ #endif } +static int linux_spi_ioctl(void *state, int fd, unsigned long request, va_list args) { + + if (request == SPI_IOC_MESSAGE(2)) { /* ioctl code for read request */ + struct spi_ioc_transfer* msg = va_arg(args, struct spi_ioc_transfer*); + + /* First message has write array and write count */ + unsigned int writecnt = msg[0].len; + unsigned char *writearr = (unsigned char *)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; + readarr[0] = 0xEF; /* WINBOND_NEX_ID */ + readarr[1] = 0x40; /* WINBOND_NEX_W25Q128_V left byte */ + readarr[2] = 0x18; /* WINBOND_NEX_W25Q128_V right byte */ + } + } + + return 0; +} + char *linux_spi_fgets(void *state, char *buf, int len, FILE *fp) { /* Emulate reading max buffer size from sysfs. */ @@ -323,7 +348,7 @@ return memcpy(buf, max_buf_size, min(len, strlen(max_buf_size) + 1)); } -void linux_spi_basic_lifecycle_test_success(void **state) +void linux_spi_probe_lifecycle_test_success(void **state) { /* * Current implementation tests a particular path of the init procedure. @@ -332,11 +357,12 @@ #if CONFIG_LINUX_SPI == 1 const struct io_mock linux_spi_io = { .fgets = linux_spi_fgets, + .ioctl = linux_spi_ioctl, }; io_mock_register(&linux_spi_io); - run_basic_lifecycle(state, &programmer_linux_spi, "dev=/dev/null"); + run_probe_lifecycle(state, &programmer_linux_spi, "dev=/dev/null", "W25Q128.V"); io_mock_register(NULL); #else diff --git a/tests/tests.c b/tests/tests.c index 34c01ea..f1fb3f3 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -369,7 +369,7 @@ cmocka_unit_test(raiden_debug_basic_lifecycle_test_success), cmocka_unit_test(dediprog_basic_lifecycle_test_success), cmocka_unit_test(linux_mtd_probe_lifecycle_test_success), - cmocka_unit_test(linux_spi_basic_lifecycle_test_success), + cmocka_unit_test(linux_spi_probe_lifecycle_test_success), cmocka_unit_test(realtek_mst_basic_lifecycle_test_success), }; ret |= cmocka_run_group_tests_name("lifecycle.c tests", lifecycle_tests, NULL, NULL); diff --git a/tests/tests.h b/tests/tests.h index 672ec1f..68f7781 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -47,7 +47,7 @@ void raiden_debug_basic_lifecycle_test_success(void **state); void dediprog_basic_lifecycle_test_success(void **state); void linux_mtd_probe_lifecycle_test_success(void **state); -void linux_spi_basic_lifecycle_test_success(void **state); +void linux_spi_probe_lifecycle_test_success(void **state); void realtek_mst_basic_lifecycle_test_success(void **state); /* layout.c */ 6 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/59742 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I32d8e972d99b52c2b18f688aa6aeae75dd170f72 Gerrit-Change-Number: 59742 Gerrit-PatchSet: 9 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Upgrade linux_mtd test to run probe lifecycle
by Anastasia Klimchuk (Code Review) 20 Mar '22

20 Mar '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/59743 ) Change subject: tests: Upgrade linux_mtd test to run probe lifecycle ...................................................................... tests: Upgrade linux_mtd test to run probe lifecycle No additional mocks are needed, because linux_mtd is doing most of the job in init function. BUG=b:181803212 TEST=ninja test Change-Id: I74436f36f628680c22c7225b1584f06464307775 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/59743 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Thomas Heijligen <src(a)posteo.de> --- M tests/lifecycle.c M tests/tests.c M tests/tests.h 3 files changed, 4 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Thomas Heijligen: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/tests/lifecycle.c b/tests/lifecycle.c index c439ff7..e48d94e 100644 --- a/tests/lifecycle.c +++ b/tests/lifecycle.c @@ -294,7 +294,7 @@ return 0; } -void linux_mtd_basic_lifecycle_test_success(void **state) +void linux_mtd_probe_lifecycle_test_success(void **state) { #if CONFIG_LINUX_MTD == 1 struct linux_mtd_io_state linux_mtd_io_state = { NULL }; @@ -307,7 +307,7 @@ io_mock_register(&linux_mtd_io); - run_basic_lifecycle(state, &programmer_linux_mtd, ""); + run_probe_lifecycle(state, &programmer_linux_mtd, "", "Opaque flash chip"); io_mock_register(NULL); #else diff --git a/tests/tests.c b/tests/tests.c index 0f16041..34c01ea 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -368,7 +368,7 @@ cmocka_unit_test(nicrealtek_basic_lifecycle_test_success), cmocka_unit_test(raiden_debug_basic_lifecycle_test_success), cmocka_unit_test(dediprog_basic_lifecycle_test_success), - cmocka_unit_test(linux_mtd_basic_lifecycle_test_success), + cmocka_unit_test(linux_mtd_probe_lifecycle_test_success), cmocka_unit_test(linux_spi_basic_lifecycle_test_success), cmocka_unit_test(realtek_mst_basic_lifecycle_test_success), }; diff --git a/tests/tests.h b/tests/tests.h index 5f05abf..672ec1f 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -46,7 +46,7 @@ void nicrealtek_basic_lifecycle_test_success(void **state); void raiden_debug_basic_lifecycle_test_success(void **state); void dediprog_basic_lifecycle_test_success(void **state); -void linux_mtd_basic_lifecycle_test_success(void **state); +void linux_mtd_probe_lifecycle_test_success(void **state); void linux_spi_basic_lifecycle_test_success(void **state); void realtek_mst_basic_lifecycle_test_success(void **state); 4 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://review.coreboot.org/c/flashrom/+/59743 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I74436f36f628680c22c7225b1584f06464307775 Gerrit-Change-Number: 59743 Gerrit-PatchSet: 7 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in flashrom[master]: tests: Add run_probe_lifecycle and add dummyflasher probe test
by Anastasia Klimchuk (Code Review) 20 Mar '22

20 Mar '22

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/59741 ) Change subject: tests: Add run_probe_lifecycle and add dummyflasher probe test ...................................................................... tests: Add run_probe_lifecycle and add dummyflasher probe test This patch implements run_probe_lifecycle and adds dummyflasher test to run probing lifecycle. A lifecycle consists of 3 steps: 1) init programmer 2) do some action 3) shutdown programmer. Step 2 can be "do nothing", and this is named "basic lifecycle", i.e. the simplest. This patch implements "probe lifecycle" which probes a chip as Step 2. Internally there is one run_lifecycle function which performs steps 1, 2, 3. run_lifecycle is operating via libflashrom API. Long term goal for cli_classic is to operate via libflashrom API, so the test aligns with this approach. BUG=b:181803212 TEST=ninja test Change-Id: I9eb7fe3a436fbba5e70db957139fd26e00efec36 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/59741 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> --- M tests/lifecycle.c M tests/tests.c M tests/tests.h 3 files changed, 56 insertions(+), 7 deletions(-) Approvals: build bot (Jenkins): Verified Thomas Heijligen: Looks good to me, approved diff --git a/tests/lifecycle.c b/tests/lifecycle.c index 8cb604b..c439ff7 100644 --- a/tests/lifecycle.c +++ b/tests/lifecycle.c @@ -16,26 +16,64 @@ #include <include/test.h> #include <string.h> +#include "libflashrom.h" #include "io_mock.h" #include "programmer.h" +#include "spi.h" -static void run_basic_lifecycle(void **state, const struct programmer_entry *prog, const char *param) +static void probe_chip(const struct programmer_entry *prog, + struct flashrom_programmer *flashprog, + const char *const chip_name) +{ + struct flashrom_flashctx *flashctx; + + printf("Testing flashrom_flash_probe for programmer=%s, chip=%s ... \n", prog->name, chip_name); + assert_int_equal(0, flashrom_flash_probe(&flashctx, flashprog, chip_name)); + printf("... flashrom_flash_probe for programmer=%s successful\n", prog->name); + + flashrom_flash_release(flashctx); /* cleanup */ +} + +static void run_lifecycle(void **state, const struct programmer_entry *prog, + const char *param, const char *const chip_name, + void (*action)(const struct programmer_entry *prog, + struct flashrom_programmer *flashprog, + const char *const chip_name)) { (void) state; /* unused */ + struct flashrom_programmer *flashprog; char *param_dup = strdup(param); - printf("Testing programmer_init for programmer=%s ...\n", prog->name); - assert_int_equal(0, programmer_init(prog, param_dup)); - printf("... programmer_init for programmer=%s successful\n", prog->name); + printf("Testing flashrom_programmer_init for programmer=%s ...\n", prog->name); + assert_int_equal(0, flashrom_programmer_init(&flashprog, prog->name, param_dup)); + printf("... flashrom_programmer_init for programmer=%s successful\n", prog->name); - printf("Testing programmer_shutdown for programmer=%s ...\n", prog->name); - assert_int_equal(0, programmer_shutdown()); - printf("... programmer_shutdown for programmer=%s successful\n", prog->name); + if (action) + action(prog, flashprog, chip_name); + + printf("Testing flashrom_programmer_shutdown for programmer=%s ...\n", prog->name); + assert_int_equal(0, flashrom_programmer_shutdown(flashprog)); + printf("... flashrom_programmer_shutdown for programmer=%s successful\n", prog->name); free(param_dup); } +static void run_basic_lifecycle(void **state, 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 */); +} + +static void run_probe_lifecycle(void **state, 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); +} + void dummy_basic_lifecycle_test_success(void **state) { #if CONFIG_DUMMY == 1 @@ -45,6 +83,15 @@ #endif } +void dummy_probe_lifecycle_test_success(void **state) +{ +#if CONFIG_DUMMY == 1 + run_probe_lifecycle(state, &programmer_dummy, "bus=spi,emulate=W25Q128FV", "W25Q128.V"); +#else + skip(); +#endif +} + void nicrealtek_basic_lifecycle_test_success(void **state) { #if CONFIG_NICREALTEK == 1 diff --git a/tests/tests.c b/tests/tests.c index 21cc1ed..0f16041 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -364,6 +364,7 @@ const struct CMUnitTest lifecycle_tests[] = { cmocka_unit_test(dummy_basic_lifecycle_test_success), + cmocka_unit_test(dummy_probe_lifecycle_test_success), cmocka_unit_test(nicrealtek_basic_lifecycle_test_success), cmocka_unit_test(raiden_debug_basic_lifecycle_test_success), cmocka_unit_test(dediprog_basic_lifecycle_test_success), diff --git a/tests/tests.h b/tests/tests.h index 1218a30..5f05abf 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -42,6 +42,7 @@ /* lifecycle.c */ void dummy_basic_lifecycle_test_success(void **state); +void dummy_probe_lifecycle_test_success(void **state); void nicrealtek_basic_lifecycle_test_success(void **state); void raiden_debug_basic_lifecycle_test_success(void **state); void dediprog_basic_lifecycle_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/59741 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I9eb7fe3a436fbba5e70db957139fd26e00efec36 Gerrit-Change-Number: 59741 Gerrit-PatchSet: 6 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2022