January 2023 - flashrom-gerrit

[L] Change in flashrom[master]: flashrom.c:Add helper functions needed for new erase selction algorithm
by Aarya (Code Review) 16 Jan '23

16 Jan '23

Attention is currently required from: Simon Buhrow, Edward O'Callaghan. Hello build bot (Jenkins), Simon Buhrow, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/65844 to look at the new patch set (#80). Change subject: flashrom.c:Add helper functions needed for new erase selction algorithm ...................................................................... flashrom.c:Add helper functions needed for new erase selction algorithm 1)Add a function to flatten out the addresses of the flash chip as per the different erase functions. This function will return a list of layouts which is dynamically allocated. So after use all the layouts as well as the list itself should be freed. The free_erase_layout function does that. 2)Add a function to align start and end address of the region (in struct walk_info) to some erase sector boundaries and modify the region start and end addresses to match nearest erase sector boundaries. This function will be used in the new algorithm for erase function selection. 3)Add a function that returns a list of sectors (as seen by the first erase function) that need erasing. Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- A erasure_layout.c A erasure_layout.h 2 files changed, 289 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/65844/80 -- To view, visit https://review.coreboot.org/c/flashrom/+/65844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Gerrit-Change-Number: 65844 Gerrit-PatchSet: 80 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: util: add bash completion script
by Anastasia Klimchuk (Code Review) 16 Jan '23

16 Jan '23

Attention is currently required from: Felix Singer, Nico Huber, Thomas Heijligen, Alexander Goncharov. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68247 ) Change subject: util: add bash completion script ...................................................................... Patch Set 12: Code-Review+2 (3 comments) Patchset: PS12: I went through the patch and comments once again, seems like it can go ahead. Sorry Joursoir that it was hanging for so long. Thank you for your work! I will leave it for few days, if everything fine will merge. I understand you are using this feature locally for months already :) File Makefile: https://review.coreboot.org/c/flashrom/+/68247/comment/05dce103_c237e464 PS9, Line 1041: .bash > From [Bash Completion FAQ](https://github.com/scop/bash-completion/blob/master/README.md#faq): […] Okay, so my understanding we have to have a suffix, and the suffix has to be `bash`. Which means leaving as is? and I mark the comment as resolved. File meson.build: https://review.coreboot.org/c/flashrom/+/68247/comment/3f54c17a_c2779bd3 PS9, Line 646: .bash > Only option I could find is to provide a custom install_script. Same situation as with other comment as I understand. Custom install script is probably too much for this... let's leave as is. I am marking as resolved. -- To view, visit https://review.coreboot.org/c/flashrom/+/68247 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie68bc91c3cea4de2ffdbeffd07e48edd8d5590e1 Gerrit-Change-Number: 68247 Gerrit-PatchSet: 12 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Mon, 16 Jan 2023 07:10:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[L] Change in flashrom[master]: flashrom.c:Add helper functions needed for new erase selection algorithm
by Aarya (Code Review) 16 Jan '23

16 Jan '23

Attention is currently required from: Simon Buhrow, Edward O'Callaghan. Hello build bot (Jenkins), Simon Buhrow, Thomas Heijligen, Edward O'Callaghan, Anastasia Klimchuk, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/65844 to look at the new patch set (#79). Change subject: flashrom.c:Add helper functions needed for new erase selection algorithm ...................................................................... flashrom.c:Add helper functions needed for new erase selection algorithm 1)Add a function to flatten out the addresses of the flash chip as per the different erase functions. This function will return a list of layouts which is dynamically allocated. So after use all the layouts as well as the list itself should be freed. The free_erase_layout function does that. 2)Add a function to align start and end address of the region (in struct walk_info) to some erase sector boundaries and modify the region start and end addresses to match nearest erase sector boundaries. This function will be used in the new algorithm for erase function selection. 3)Add a function that returns a list of sectors (as seen by the first erase function) that need erasing. Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- A erasure_layout.c A erasure_layout.h 2 files changed, 289 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/44/65844/79 -- To view, visit https://review.coreboot.org/c/flashrom/+/65844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Gerrit-Change-Number: 65844 Gerrit-PatchSet: 79 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[L] Change in flashrom[master]: flashrom.c:Add auxillary functions needed for new erase selction algo...
by Aarya (Code Review) 16 Jan '23

16 Jan '23

Attention is currently required from: Simon Buhrow, Edward O'Callaghan. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/65844 ) Change subject: flashrom.c:Add auxillary functions needed for new erase selction algorithm ...................................................................... Patch Set 78: (1 comment) Patchset: PS78: > Squash into CB:65879 This patch already contains the code from CB:65879. I think all the other patches should be squashed into this one. -- To view, visit https://review.coreboot.org/c/flashrom/+/65844 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic57ca1cca3d1646543f6b5939ba9c35db8d08256 Gerrit-Change-Number: 65844 Gerrit-PatchSet: 78 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Mon, 16 Jan 2023 03:54:12 +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]: flashrom.c: Add wrapper function to use the erase algorithm
by Aarya (Code Review) 16 Jan '23

16 Jan '23

Attention is currently required from: Simon Buhrow, Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Aarya has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/66104 ) Change subject: flashrom.c: Add wrapper function to use the erase algorithm ...................................................................... Patch Set 97: (1 comment) Patchset: PS97: > Squash into CB:65879 Do you mean CB:65844 -- To view, visit https://review.coreboot.org/c/flashrom/+/66104 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I29e3f2bd796759794184b125741a5abaac6f3ce8 Gerrit-Change-Number: 66104 Gerrit-PatchSet: 97 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Simon Buhrow 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Mon, 16 Jan 2023 03:52:34 +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]: flashrom_tester: Remove --output log redirect option
by Edward O'Callaghan (Code Review) 16 Jan '23

16 Jan '23

Attention is currently required from: Nikolai Artemiev, Evan Benn. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71773 ) Change subject: flashrom_tester: Remove --output log redirect option ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: > I support removing this, we get weird diffs with the `--output` option. […] ACK, thanks for the context. -- To view, visit https://review.coreboot.org/c/flashrom/+/71773 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5eab8169644a16ba31b203e8607853c459f92978 Gerrit-Change-Number: 71773 Gerrit-PatchSet: 2 Gerrit-Owner: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Mon, 16 Jan 2023 03:24:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Add switch for legacy impl of erasure path
by Anastasia Klimchuk (Code Review) 16 Jan '23

16 Jan '23

Attention is currently required from: Felix Singer, Thomas Heijligen, Edward O'Callaghan, Aarya. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71119 ) Change subject: flashrom.c: Add switch for legacy impl of erasure path ...................................................................... Patch Set 10: Code-Review+2 (1 comment) Patchset: PS10: Aarya, I just wanted to say: please don't be scared of -2, it is just to avoid confusion, and not to accidentally merge before the rest is done. The patch is all good by itself! :) -- To view, visit https://review.coreboot.org/c/flashrom/+/71119 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib5660db0067c1c799dcb5c8e83b4a4826b236442 Gerrit-Change-Number: 71119 Gerrit-PatchSet: 10 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 16 Jan 2023 03:05:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Add switch for legacy impl of erasure path
by Edward O'Callaghan (Code Review) 16 Jan '23

16 Jan '23

Attention is currently required from: Felix Singer, Thomas Heijligen, Aarya. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71119 ) Change subject: flashrom.c: Add switch for legacy impl of erasure path ...................................................................... Patch Set 10: Code-Review-2 (1 comment) Patchset: PS10: pending the rest of the gsoc work to be ready to merge. I would be inclined to even squash this into the gsoc work. -- To view, visit https://review.coreboot.org/c/flashrom/+/71119 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib5660db0067c1c799dcb5c8e83b4a4826b236442 Gerrit-Change-Number: 71119 Gerrit-PatchSet: 10 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Mon, 16 Jan 2023 02:57:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flash.h: Make functions global that will be used for new erase algorithm
by Anastasia Klimchuk (Code Review) 16 Jan '23

16 Jan '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71174 ) Change subject: flash.h: Make functions global that will be used for new erase algorithm ...................................................................... flash.h: Make functions global that will be used for new erase algorithm The new erase algorithm uses some of the functions which are static to `flashrom.c`. So make these functions global and add prototypes to `include\flash.h` and `include\layout.h'. Change-Id: I7ee7e208948337b88467935fd2861b5f9ad6af9d Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/71174 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashrom.c M include/flash.h M include/layout.h 3 files changed, 35 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 8da2c42..957d4aa 100644 --- a/flashrom.c +++ b/flashrom.c @@ -356,7 +356,7 @@ return extract_param(&cfg->params, param_name, ","); } -static void get_flash_region(const struct flashctx *flash, int addr, struct flash_region *region) +void get_flash_region(const struct flashctx *flash, int addr, struct flash_region *region) { if ((flash->mst->buses_supported & BUS_PROG) && flash->mst->opaque.get_region) { flash->mst->opaque.get_region(flash, addr, region); @@ -371,7 +371,7 @@ } } -static int check_for_unwritable_regions(const struct flashctx *flash, unsigned int start, unsigned int len) +int check_for_unwritable_regions(const struct flashctx *flash, unsigned int start, unsigned int len) { struct flash_region region; for (unsigned int addr = start; addr < start + len; addr = region.end) { @@ -391,7 +391,7 @@ /* special unit-test hook */ erasefunc_t *g_test_erase_injector; -static erasefunc_t *lookup_erase_func_ptr(const struct block_eraser *const eraser) +erasefunc_t *lookup_erase_func_ptr(const struct block_eraser *const eraser) { switch (eraser->block_erase) { case SPI_BLOCK_ERASE_EMULATION: return &spi_block_erase_emulation; @@ -439,7 +439,7 @@ return NULL; } -static int check_block_eraser(const struct flashctx *flash, int k, int log) +int check_block_eraser(const struct flashctx *flash, int k, int log) { struct block_eraser eraser = flash->chip->block_erasers[k]; @@ -477,7 +477,7 @@ } /* Returns the number of well-defined erasers for a chip. */ -static unsigned int count_usable_erasers(const struct flashctx *flash) +unsigned int count_usable_erasers(const struct flashctx *flash) { unsigned int usable_erasefunctions = 0; int k; @@ -509,7 +509,7 @@ } /* start is an offset to the base address of the flash chip */ -static int check_erased_range(struct flashctx *flash, unsigned int start, unsigned int len) +int check_erased_range(struct flashctx *flash, unsigned int start, unsigned int len) { int ret; const uint8_t erased_value = ERASED_VALUE(flash); @@ -710,7 +710,7 @@ * @gran write granularity (enum, not count) * @return 0 if no erase is needed, 1 otherwise */ -static int need_erase(const uint8_t *have, const uint8_t *want, unsigned int len, +int need_erase(const uint8_t *have, const uint8_t *want, unsigned int len, enum write_granularity gran, const uint8_t erased_value) { int result = 0; @@ -786,7 +786,7 @@ * in relation to the max write length of the programmer and the max write * length of the chip. */ -static unsigned int get_next_write(const uint8_t *have, const uint8_t *want, unsigned int len, +unsigned int get_next_write(const uint8_t *have, const uint8_t *want, unsigned int len, unsigned int *first_start, enum write_granularity gran) { @@ -993,7 +993,7 @@ * This wrapper simplifies most cases when the flash chip needs to be written * since policy decisions such as non-fatal error handling is centralized. */ -static int write_flash(struct flashctx *flash, const uint8_t *buf, +int write_flash(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len) { if (!flash->flags.skip_unwritable_regions) { diff --git a/include/flash.h b/include/flash.h index 2ea9c86..fcefc42 100644 --- a/include/flash.h +++ b/include/flash.h @@ -619,6 +619,13 @@ int prepare_flash_access(struct flashctx *, bool read_it, bool write_it, bool erase_it, bool verify_it); void finalize_flash_access(struct flashctx *); int register_chip_restore(chip_restore_fn_cb_t func, struct flashctx *flash, uint8_t status); +int check_block_eraser(const struct flashctx *flash, int k, int log); +unsigned int count_usable_erasers(const struct flashctx *flash); +int need_erase(const uint8_t *have, const uint8_t *want, unsigned int len, enum write_granularity gran, const uint8_t erased_value); +erasefunc_t *lookup_erase_func_ptr(const struct block_eraser *const eraser); +int check_erased_range(struct flashctx *flash, unsigned int start, unsigned int len); +unsigned int get_next_write(const uint8_t *have, const uint8_t *want, unsigned int len, unsigned int *first_start, enum write_granularity gran); +int write_flash(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); /* Something happened that shouldn't happen, but we can go on. */ #define ERROR_FLASHROM_NONFATAL 0x100 diff --git a/include/layout.h b/include/layout.h index 70d99cb..5614bea 100644 --- a/include/layout.h +++ b/include/layout.h @@ -72,5 +72,7 @@ int included_regions_overlap(const struct flashrom_layout *); void prepare_layout_for_extraction(struct flashrom_flashctx *); int layout_sanity_checks(const struct flashrom_flashctx *); +int check_for_unwritable_regions(const struct flashrom_flashctx *flash, unsigned int start, unsigned int len); +void get_flash_region(const struct flashrom_flashctx *flash, int addr, struct flash_region *region); #endif /* !__LAYOUT_H__ */ -- To view, visit https://review.coreboot.org/c/flashrom/+/71174 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7ee7e208948337b88467935fd2861b5f9ad6af9d Gerrit-Change-Number: 71174 Gerrit-PatchSet: 6 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: flashrom.c: Plumb 'all_skipped' global state into func param
by Anastasia Klimchuk (Code Review) 16 Jan '23

16 Jan '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/67093 ) ( 39 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: flashrom.c: Plumb 'all_skipped' global state into func param ...................................................................... flashrom.c: Plumb 'all_skipped' global state into func param The 'all_skipped' global state can be made into a function parameter if one just follows though the CFG. Running `flashrom -p dummy:emulate=SST25VF032B,image=r.bin -w r.bin` displays the message "Warning: Chip content is identical to the requested image." Change-Id: I2346c869c47b48604360b0facf9313aae086c8dd Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Co-authored-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/67093 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 flashrom.c 1 file changed, 43 insertions(+), 19 deletions(-) Approvals: build bot (Jenkins): Verified Edward O'Callaghan: Looks good to me, but someone else must approve Anastasia Klimchuk: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 822594b..8da2c42 100644 --- a/flashrom.c +++ b/flashrom.c @@ -62,9 +62,6 @@ */ static bool may_register_shutdown = false; -/* Did we change something or was every erase/write skipped (if any)? */ -static bool all_skipped = true; - struct programmer_cfg { char *params; }; @@ -1314,11 +1311,12 @@ chipoff_t erase_end; }; /* returns 0 on success, 1 to retry with another erase function, 2 for immediate abort */ -typedef int (*per_blockfn_t)(struct flashctx *, const struct walk_info *, erasefn_t); +typedef int (*per_blockfn_t)(struct flashctx *, const struct walk_info *, erasefn_t, bool *); static int walk_eraseblocks(struct flashctx *const flashctx, struct walk_info *const info, - const size_t erasefunction, const per_blockfn_t per_blockfn) + const size_t erasefunction, const per_blockfn_t per_blockfn, + bool *all_skipped) { int ret; size_t i, j; @@ -1346,7 +1344,7 @@ msg_cdbg("0x%06x-0x%06x:", info->erase_start, info->erase_end); erasefunc_t *erase_func = lookup_erase_func_ptr(eraser); - ret = per_blockfn(flashctx, info, erase_func); + ret = per_blockfn(flashctx, info, erase_func, all_skipped); if (ret) return ret; } @@ -1358,12 +1356,12 @@ } static int walk_by_layout(struct flashctx *const flashctx, struct walk_info *const info, - const per_blockfn_t per_blockfn) + const per_blockfn_t per_blockfn, bool *all_skipped) { const struct flashrom_layout *const layout = get_layout(flashctx); const struct romentry *entry = NULL; - all_skipped = true; + *all_skipped = true; msg_cinfo("Erasing and writing flash chip... "); while ((entry = layout_next_included(layout, entry))) { @@ -1380,7 +1378,7 @@ if (check_block_eraser(flashctx, j, 1)) continue; - error = walk_eraseblocks(flashctx, info, j, per_blockfn); + error = walk_eraseblocks(flashctx, info, j, per_blockfn, all_skipped); if (error != 1) break; @@ -1404,14 +1402,15 @@ return 1; } } - if (all_skipped) + if (*all_skipped) msg_cinfo("\nWarning: Chip content is identical to the requested image.\n"); msg_cinfo("Erase/write done.\n"); return 0; } static int erase_block(struct flashctx *const flashctx, - const struct walk_info *const info, const erasefn_t erasefn) + const struct walk_info *const info, const erasefn_t erasefn, + bool *all_skipped) { const unsigned int erase_len = info->erase_end + 1 - info->erase_start; const bool region_unaligned = info->region_start > info->erase_start || @@ -1457,7 +1456,7 @@ } ret = 1; - all_skipped = false; + *all_skipped = false; msg_cdbg("E"); @@ -1529,11 +1528,13 @@ static int erase_by_layout(struct flashctx *const flashctx) { struct walk_info info = { 0 }; - return walk_by_layout(flashctx, &info, &erase_block); + bool all_skipped = true; + return walk_by_layout(flashctx, &info, &erase_block, &all_skipped); } static int read_erase_write_block(struct flashctx *const flashctx, - const struct walk_info *const info, const erasefn_t erasefn) + const struct walk_info *const info, const erasefn_t erasefn, + bool *all_skipped) { const chipsize_t erase_len = info->erase_end + 1 - info->erase_start; const bool region_unaligned = info->region_start > info->erase_start || @@ -1591,7 +1592,7 @@ const uint8_t erased_value = ERASED_VALUE(flashctx); if (!(flashctx->chip->feature_bits & FEATURE_NO_ERASE) && need_erase(curcontents, newcontents, erase_len, flashctx->chip->gran, erased_value)) { - if (erase_block(flashctx, info, erasefn)) + if (erase_block(flashctx, info, erasefn, all_skipped)) goto _free_ret; /* Erase was successful. Adjust curcontents. */ memset(curcontents, erased_value, erase_len); @@ -1614,7 +1615,7 @@ if (skipped) msg_cdbg("S"); else - all_skipped = false; + *all_skipped = false; /* Update curcontents, other regions with overlapping erase blocks might rely on this. */ @@ -1640,12 +1641,13 @@ * 1 if anything has gone wrong. */ static int write_by_layout(struct flashctx *const flashctx, - void *const curcontents, const void *const newcontents) + void *const curcontents, const void *const newcontents, + bool *all_skipped) { struct walk_info info; info.curcontents = curcontents; info.newcontents = newcontents; - return walk_by_layout(flashctx, &info, read_erase_write_block); + return walk_by_layout(flashctx, &info, read_erase_write_block, all_skipped); } /** @@ -2097,7 +2099,9 @@ msg_cinfo("done.\n"); } - if (write_by_layout(flashctx, curcontents, newcontents)) { + bool all_skipped = true; + + if (write_by_layout(flashctx, curcontents, newcontents, &all_skipped)) { msg_cerr("Uh oh. Erase/write failed. "); ret = 2; if (verify_all) { -- To view, visit https://review.coreboot.org/c/flashrom/+/67093 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I2346c869c47b48604360b0facf9313aae086c8dd Gerrit-Change-Number: 67093 Gerrit-PatchSet: 41 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2023