November 2021 - flashrom-gerrit

Change in flashrom[master]: cli_classic: add writeprotect CLI
by Sergii Dmytruk (Code Review) 01 Dec '21

01 Dec '21

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 ) Change subject: cli_classic: add writeprotect CLI ...................................................................... Patch Set 18: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/58738/comment/c9108ecb_0a6f01ef PS17, Line 220: ret = flashrom_wp_set_range(flash, &cfg, &range); : if (ret) { : msg_gerr("The chip does not support the requested range.\n"); : return ret; : } : : /* Write range before other operations (i.e. enabling HW protection) */ : ret = write_wp_config(flash, &cfg); : if (ret) : return ret; > It's good to know there is no harm, but is there any effect of doing 1,2 without 3? What would be the most common usage, 1-2-3 or 1-2? 1-2 case is harmless and quite useless since WP config is essentially opaque. > The usual intention is that a user would try to set the range (read/set_range/write), check that it succeeded, and only then set the mode (read/set_mode/write). So we could possibly change the set_mode and set_range functions to also write to the chip, but there are still the problems. In this case making calls do more work will make more sense if config gets completely removed from the API as a result, but that needs some kind of error object that can be converted to a string or just `char **error` that gets set to a descriptive error message. -- To view, visit https://review.coreboot.org/c/flashrom/+/58738 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I499f521781ee8999921996517802c0c0c641d869 Gerrit-Change-Number: 58738 Gerrit-PatchSet: 18 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 30 Nov 2021 23:10:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: cli_classic: add writeprotect CLI
by Anastasia Klimchuk (Code Review) 30 Nov '21

30 Nov '21

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Sergii Dmytruk. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 ) Change subject: cli_classic: add writeprotect CLI ...................................................................... Patch Set 18: (2 comments) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/58738/comment/4b0e52e1_6e8ef603 PS15, Line 66: " --wp-range=<start>,<len> set write protect range\n" : " --wp-region <region> set write protect region\n" > You unset it by setting an empty range (it's included in --wp-list), --wp-disable doesn't change the […] Cool thanks! Your explanation: "You unset it by setting an empty range (it's included in --wp-list)" seems like a great addition to command line help! My intention is that people can answer the question "how do I unset" just by reading help, without looking into the source code. File cli_classic.c: https://review.coreboot.org/c/flashrom/+/58738/comment/bcce3e99_4c234454 PS17, Line 220: ret = flashrom_wp_set_range(flash, &cfg, &range); : if (ret) { : msg_gerr("The chip does not support the requested range.\n"); : return ret; : } : : /* Write range before other operations (i.e. enabling HW protection) */ : ret = write_wp_config(flash, &cfg); : if (ret) : return ret; > The structure of the API is: […] It's good to know there is no harm, but is there any effect of doing 1,2 without 3? What would be the most common usage, 1-2-3 or 1-2? If 1-2 is the most common usage then code is fine. I don't suggest to ignore errors from the library, of course not. Errors should be checked in the same way, after every call. But they can be checked without printing a message. If `flashrom_wp_set_range` returns error, then just return 2 (or something else) without calling `write_wp_config`. Just to be clear: I was thinking of a function like `mega_flashrom_wp_set_range` which does 1-2-3 and checks errors on every step. -- To view, visit https://review.coreboot.org/c/flashrom/+/58738 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I499f521781ee8999921996517802c0c0c641d869 Gerrit-Change-Number: 58738 Gerrit-PatchSet: 18 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 30 Nov 2021 22:34:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: cli_classic: add writeprotect CLI
by Nikolai Artemiev (Code Review) 30 Nov '21

30 Nov '21

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 ) Change subject: cli_classic: add writeprotect CLI ...................................................................... Patch Set 18: (1 comment) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/58738/comment/7ff9e7c3_ab124f8c PS17, Line 220: ret = flashrom_wp_set_range(flash, &cfg, &range); : if (ret) { : msg_gerr("The chip does not support the requested range.\n"); : return ret; : } : : /* Write range before other operations (i.e. enabling HW protection) */ : ret = write_wp_config(flash, &cfg); : if (ret) : return ret; > I understand we need to print extra messages here. To achieve that, flashrom_wp_set_range need to return error codes, can be 1,2,3 etc. Here, we can print messages depending on error code. I thought return codes could be a good option too, but there were other issues with the design and ultimately I thought the approach of separate read/modify/write functions was nicer, but there could be a better option. The main problems were: - If writing the config fails, the CLI should print some kind of helpful error message. Right now this involves converting the expected chip state and actual chip state to strings and printing them (roughly lines 175-185). If we put the verification logic in writeprotect.c, we have to pass back the expected/actual config objects so the CLI code can print them. That's doable but a bit messy. - If we call write_wp_config() from the set_range() and set_mode() then we would also have to pass the expected/actual config objects there as well. > The API in its current form allows fetching state, setting both mode and range and writing it in one go. If it's changed to perform writes on every operation and you incorrectly set mode before the range, setting range might fail (possibly permanently). Current design seems safer: you make changes, handle errors caught by the library and only then try to change state of the chip. The usual intention is that a user would try to set the range (read/set_range/write), check that it succeeded, and only then set the mode (read/set_mode/write). So we could possibly change the set_mode and set_range functions to also write to the chip, but there are still the problems. -- To view, visit https://review.coreboot.org/c/flashrom/+/58738 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I499f521781ee8999921996517802c0c0c641d869 Gerrit-Change-Number: 58738 Gerrit-PatchSet: 18 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 30 Nov 2021 22:31:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: [FIX ME] tests: Add test for extract operation
by Daniel Campello (Code Review) 30 Nov '21

30 Nov '21

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Peter Marheine. Daniel Campello has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59532 ) Change subject: [FIX ME] tests: Add test for extract operation ...................................................................... Patch Set 2: (1 comment) Patchset: PS1: > Failing looks like correct behavior to me, since `read_flash_to_file` will never have a meaningful r […] I believe Peter is right and there is some issues due to the differences between the Chromium fork and here (differences on https://review.coreboot.org/c/flashrom/+/52362). With respect on how the read actually happens (we are not throwing away the results), this is done via the logic in prepare_layout_for_extraction() coupled with write_buf_to_include_args() in read_flash_to_file() -- To view, visit https://review.coreboot.org/c/flashrom/+/59532 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I13107c0e095d53184e32a41fa72227cf7dc1d449 Gerrit-Change-Number: 59532 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> 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-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Tue, 30 Nov 2021 17:44:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: cli_classic: add writeprotect CLI
by Sergii Dmytruk (Code Review) 30 Nov '21

30 Nov '21

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk, Nikolai Artemiev. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58738 ) Change subject: cli_classic: add writeprotect CLI ...................................................................... Patch Set 18: (2 comments) File cli_classic.c: https://review.coreboot.org/c/flashrom/+/58738/comment/cb7f99e4_76b86565 PS15, Line 66: " --wp-range=<start>,<len> set write protect range\n" : " --wp-region <region> set write protect region\n" > I am just wondering, there are commands to set wp range/region, but how to unset? Or is it --wp-disa […] You unset it by setting an empty range (it's included in --wp-list), --wp-disable doesn't change the range. File cli_classic.c: https://review.coreboot.org/c/flashrom/+/58738/comment/a1f1b022_aebf4e70 PS17, Line 220: ret = flashrom_wp_set_range(flash, &cfg, &range); : if (ret) { : msg_gerr("The chip does not support the requested range.\n"); : return ret; : } : : /* Write range before other operations (i.e. enabling HW protection) */ : ret = write_wp_config(flash, &cfg); : if (ret) : return ret; > Continuing the conversation about libflashrom vs cli. […] The structure of the API is: 1. Read state from chip 2. Modify state in memory 3. Write state to chip So no harm in forgetting to state. The API in its current form allows fetching state, setting both mode and range and writing it in one go. If it's changed to perform writes on every operation and you incorrectly set mode before the range, setting range might fail (possibly permanently). Current design seems safer: you make changes, handle errors caught by the library and only then try to change state of the chip. -- To view, visit https://review.coreboot.org/c/flashrom/+/58738 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I499f521781ee8999921996517802c0c0c641d869 Gerrit-Change-Number: 58738 Gerrit-PatchSet: 18 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> 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: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Tue, 30 Nov 2021 13:57:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Add run_probe_lifecycle and use it in dummyflasher test
by Edward O'Callaghan (Code Review) 30 Nov '21

30 Nov '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59741 ) Change subject: tests: Add run_probe_lifecycle and use it in dummyflasher test ...................................................................... Patch Set 1: (1 comment) File tests/lifecycle.c: https://review.coreboot.org/c/flashrom/+/59741/comment/59ff7b54_880583e6 PS1, Line 68: "bus=spi,emulate=W25Q128FV", "W25Q128.V" this is changing the behavior of the test in a rename commit. -- 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: 1 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 30 Nov 2021 08:26:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Rename run_lifecycle into run_basic_lifecycle
by Edward O'Callaghan (Code Review) 30 Nov '21

30 Nov '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59740 ) Change subject: tests: Rename run_lifecycle into run_basic_lifecycle ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/59740/comment/5a0d744f_3acdb42f PS1, Line 7: basic but what does "basic" mean, seems like a bit of a spectrum of meaning? -- To view, visit https://review.coreboot.org/c/flashrom/+/59740 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2771921ae2bd37f4b3f49342e03d9abb5ee36ea0 Gerrit-Change-Number: 59740 Gerrit-PatchSet: 1 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 30 Nov 2021 06:59:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Rename init_shutdown.c into lifecycle.c
by Edward O'Callaghan (Code Review) 30 Nov '21

30 Nov '21

Attention is currently required from: Nico Huber, Angel Pons, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59739 ) Change subject: tests: Rename init_shutdown.c into lifecycle.c ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/59739 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8d734c43cc15c7ec1055d3fb5bdcdca8c90d0987 Gerrit-Change-Number: 59739 Gerrit-PatchSet: 1 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Tue, 30 Nov 2021 06:58:26 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Set up mock chip memory in consistent and predictable way
by Anastasia Klimchuk (Code Review) 30 Nov '21

30 Nov '21

Attention is currently required from: Nico Huber, Paul Menzel, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59237 ) Change subject: tests: Set up mock chip memory in consistent and predictable way ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/59237/comment/bdeb0fdf_5b05a38a PS3, Line 7: Setup > Set up Done. You are welcome to do a review for the rest of the chain! -- To view, visit https://review.coreboot.org/c/flashrom/+/59237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871 Gerrit-Change-Number: 59237 Gerrit-PatchSet: 4 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 30 Nov 2021 06:55:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: tests: Set up mock chip memory in consistent and predictable way
by Anastasia Klimchuk (Code Review) 30 Nov '21

30 Nov '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello build bot (Jenkins), Nico Huber, Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59237 to look at the new patch set (#4). Change subject: tests: Set up mock chip memory in consistent and predictable way ...................................................................... tests: Set up mock chip memory in consistent and predictable way This patch adds a macro MOCK_CHIP_CONTENT which represents a memory state of a mock chip. The macro is used to initialise mock chip memory at the beginning of a test (in setup_chip function). Previously mock chip memory was not reset between tests. For existing tests that did not matter, however new test for verify operation (added later in this chain) needs mock chip memory to be setup in a predictable way. BUG=b:181803212 TEST=ninja test Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M tests/chip.c 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/59237/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/59237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0d7623a601c207bfc62d54ab89d94cda56d85871 Gerrit-Change-Number: 59237 Gerrit-PatchSet: 4 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> 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-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2021