June 2018 - flashrom-gerrit - mail.coreboot.org

Change in flashrom[master]: [WIP]layout.c: Use a linked lists for layout
by Arthur Heymans (Code Review) 14 Jun '18

14 Jun '18

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/27106 to look at the new patch set (#2). Change subject: [WIP]layout.c: Use a linked lists for layout ...................................................................... [WIP]layout.c: Use a linked lists for layout This uses sys/queue.h to use a linked list for the struct flashrom_layout instead of using an fix sized array. This also removes the global variable in the layout.c file. Change-Id: Id7f48ac613da1590f30247bee979034cf1f74e9e Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M cli_classic.c M flash.h M flashrom.c M ich_descriptors.c M ich_descriptors.h M layout.c M layout.h M libflashrom.c 8 files changed, 123 insertions(+), 103 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/27106/2 -- To view, visit https://review.coreboot.org/27106 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Id7f48ac613da1590f30247bee979034cf1f74e9e Gerrit-Change-Number: 27106 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: [WIP]layout.c: Use a linked lists for layout
by build bot (Jenkins) (Code Review) 14 Jun '18

14 Jun '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/27106 ) Change subject: [WIP]layout.c: Use a linked lists for layout ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1331/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/1100/ : SUCCESS -- To view, visit https://review.coreboot.org/27106 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id7f48ac613da1590f30247bee979034cf1f74e9e Gerrit-Change-Number: 27106 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 14 Jun 2018 17:26:18 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: [WIP]layout: Use a linked list for layouts
by Arthur Heymans (Code Review) 14 Jun '18

14 Jun '18

Arthur Heymans has abandoned this change. ( https://review.coreboot.org/27102 ) Change subject: [WIP]layout: Use a linked list for layouts ...................................................................... Abandoned made a better version using sys/queue.h -- To view, visit https://review.coreboot.org/27102 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: abandon Gerrit-Change-Id: I049657805ce5676ea4c4b5e49f26d0e8a1f26a3e Gerrit-Change-Number: 27102 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: [WIP]layout.c: Use a linked lists for layout
by Arthur Heymans (Code Review) 14 Jun '18

14 Jun '18

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/27106 Change subject: [WIP]layout.c: Use a linked lists for layout ...................................................................... [WIP]layout.c: Use a linked lists for layout This uses sys/queue.h to use a linked list for the struct flashrom_layout instead of using an fix sized array. This also removes the global variable in the layout.c file. Change-Id: Id7f48ac613da1590f30247bee979034cf1f74e9e Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M cli_classic.c M flash.h M flashrom.c M ich_descriptors.c M ich_descriptors.h M layout.c M layout.h M libflashrom.c 8 files changed, 117 insertions(+), 100 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/27106/1 diff --git a/cli_classic.c b/cli_classic.c index 119c6f8..3d5e612 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -103,7 +103,8 @@ #endif int read_it = 0, write_it = 0, erase_it = 0, verify_it = 0; int dont_verify_it = 0, dont_verify_all = 0, list_supported = 0, operation_specified = 0; - struct flashrom_layout *layout = NULL; + struct flashrom_layout *layout = malloc(sizeof(struct flashrom_layout)); + enum programmer prog = PROGRAMMER_INVALID; enum { OPTION_IFD = 0x0100, @@ -395,11 +396,11 @@ } msg_gdbg("\n"); - if (layoutfile && read_romlayout(layoutfile)) { + if (layoutfile && read_romlayout(layout, layoutfile)) { ret = 1; goto out; } - if (!ifd && process_include_args(get_global_layout())) { + if (!ifd && process_include_args(layout)) { ret = 1; goto out; } @@ -552,9 +553,9 @@ goto out_shutdown; } - if (layoutfile) { - layout = get_global_layout(); - } else if (ifd && (flashrom_layout_read_from_ifd(&layout, fill_flash, NULL, 0) || + /* if (layoutfile) { */ + /* layout = get_global_layout(); */ + /* } else */ if (ifd && (flashrom_layout_read_from_ifd(&layout, fill_flash, NULL, 0) || process_include_args(layout))) { ret = 1; goto out_shutdown; @@ -590,7 +591,7 @@ for (i = 0; i < chipcount; i++) free(flashes[i].chip); - layout_cleanup(); + layout_cleanup(layout); free(filename); free(referencefile); free(layoutfile); diff --git a/flash.h b/flash.h index fca3775..2cd09c2 100644 --- a/flash.h +++ b/flash.h @@ -385,9 +385,9 @@ /* layout.c */ int register_include_arg(char *name); -int read_romlayout(const char *name); -int normalize_romentries(const struct flashctx *flash); -void layout_cleanup(void); +int read_romlayout(struct flashrom_layout *layout, const char *name); +int normalize_romentries(const struct flashrom_layout *layout, const struct flashctx *flash); +void layout_cleanup(struct flashrom_layout *layout); /* spi.c */ struct spi_command { diff --git a/flashrom.c b/flashrom.c index f85dbb1..b972233 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1298,12 +1298,13 @@ /* Fill fallback layout covering the whole chip. */ struct single_layout *const fallback = &flash->fallback_layout; - fallback->base.entries = &fallback->entry; - fallback->base.num_entries = 1; - fallback->entry.start = 0; - fallback->entry.end = flash->chip->total_size * 1024 - 1; - fallback->entry.included = true; - strcpy(fallback->entry.name, "complete flash"); + struct romentry entry; + init_layout(&fallback->base); + entry.start = 0; + entry.end = flash->chip->total_size * 1024 - 1; + entry.included = true; + strcpy(entry.name, "complete flash"); + append_romentry(&fallback->base, &entry); tmp = flashbuses_to_text(flash->chip->bustype); msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found", @@ -1559,13 +1560,14 @@ { const struct flashrom_layout *const layout = get_layout(flashctx); - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (!layout->entries[i].included) + struct flashrom_layout_entry *var; + SLIST_FOREACH(var, layout, next) { + struct romentry entry = get_romentry(var); + if (!entry.included) continue; - const chipoff_t region_start = layout->entries[i].start; - const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1; + const chipoff_t region_start = entry.start; + const chipsize_t region_len = entry.end - entry.start + 1; if (flashctx->chip->read(flashctx, buffer + region_start, region_start, region_len)) return 1; @@ -1646,13 +1648,14 @@ all_skipped = true; msg_cinfo("Erasing and writing flash chip... "); - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (!layout->entries[i].included) + struct flashrom_layout_entry *var; + SLIST_FOREACH(var, layout, next) { + struct romentry entry = get_romentry(var); + if (!entry.included) continue; - info->region_start = layout->entries[i].start; - info->region_end = layout->entries[i].end; + info->region_start = entry.start; + info->region_end = entry.end; size_t j; int error = 1; /* retry as long as it's 1 */ @@ -1861,13 +1864,14 @@ { const struct flashrom_layout *const layout = get_layout(flashctx); - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (!layout->entries[i].included) + struct flashrom_layout_entry *var; + SLIST_FOREACH(var, layout, next) { + struct romentry entry = get_romentry(var); + if (!entry.included) continue; - const chipoff_t region_start = layout->entries[i].start; - const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1; + const chipoff_t region_start = entry.start; + const chipsize_t region_len = entry.end - entry.start + 1; if (flashctx->chip->read(flashctx, curcontents + region_start, region_start, region_len)) return 1; @@ -2219,7 +2223,7 @@ return 1; } - if (flash->layout == get_global_layout() && normalize_romentries(flash)) { + if (normalize_romentries(flash->layout, flash)) { msg_cerr("Requested regions can not be handled. Aborting.\n"); return 1; } @@ -2333,13 +2337,14 @@ { const struct flashrom_layout *const layout = get_layout(flashctx); - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (layout->entries[i].included) + struct flashrom_layout_entry *var; + SLIST_FOREACH(var, layout, next) { + struct romentry entry = get_romentry(var); + if (!entry.included) continue; - const chipoff_t region_start = layout->entries[i].start; - const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1; + const chipoff_t region_start = entry.start; + const chipsize_t region_len = entry.end - entry.start + 1; memcpy(newcontents + region_start, oldcontents + region_start, region_len); } diff --git a/ich_descriptors.c b/ich_descriptors.c index 56e846f..4c9d007 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -1179,8 +1179,6 @@ snprintf(layout->entries[j].name, sizeof(layout->entries[j].name), "%s", regions[i]); ++j; } - layout->base.entries = layout->entries; - layout->base.num_entries = j; return 0; } diff --git a/ich_descriptors.h b/ich_descriptors.h index fa1990d..7849914 100644 --- a/ich_descriptors.h +++ b/ich_descriptors.h @@ -563,7 +563,7 @@ }; struct ich_layout { - struct flashrom_layout base; + int num_entries; struct romentry entries[MAX_NUM_FLREGS]; }; diff --git a/layout.c b/layout.c index fa66238..55bed12 100644 --- a/layout.c +++ b/layout.c @@ -23,33 +23,41 @@ #include "programmer.h" #include "layout.h" -struct romentry entries[MAX_ROMLAYOUT]; -static struct flashrom_layout layout = { entries, 0 }; - /* include_args holds the arguments specified at the command line with -i. They must be processed at some point * so that desired regions are marked as "included" in the layout. */ static char *include_args[MAX_ROMLAYOUT]; static int num_include_args = 0; /* the number of valid include_args. */ -struct flashrom_layout *get_global_layout(void) +void init_layout(struct flashrom_layout *layout) { - return &layout; + SLIST_INIT(layout); +} + +void append_romentry(struct flashrom_layout *layout, struct romentry *entry) +{ + struct flashrom_layout_entry *var = malloc(sizeof(struct flashrom_layout_entry)); + var->entry = *entry; + SLIST_INSERT_HEAD(layout, var, next); +} + +struct romentry get_romentry(struct flashrom_layout_entry *var) +{ + return var->entry; } const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx) { - if (flashctx->layout && flashctx->layout->num_entries) + if (flashctx->layout) return flashctx->layout; else return &flashctx->fallback_layout.base; } #ifndef __LIBPAYLOAD__ -int read_romlayout(const char *name) +int read_romlayout(struct flashrom_layout *layout, const char *name) { FILE *romlayout; char tempstr[256]; - int i; romlayout = fopen(name, "r"); @@ -60,15 +68,10 @@ } while (!feof(romlayout)) { + struct romentry entry; char *tstr1, *tstr2; - if (layout.num_entries >= MAX_ROMLAYOUT) { - msg_gerr("Maximum number of ROM images (%i) in layout " - "file reached.\n", MAX_ROMLAYOUT); - (void)fclose(romlayout); - return 1; - } - if (2 != fscanf(romlayout, "%255s %255s\n", tempstr, layout.entries[layout.num_entries].name)) + if (2 != fscanf(romlayout, "%255s %255s\n", tempstr, entry.name)) continue; #if 0 // fscanf does not like arbitrary comments like that :( later @@ -83,16 +86,17 @@ (void)fclose(romlayout); return 1; } - layout.entries[layout.num_entries].start = strtol(tstr1, (char **)NULL, 16); - layout.entries[layout.num_entries].end = strtol(tstr2, (char **)NULL, 16); - layout.entries[layout.num_entries].included = 0; - layout.num_entries++; + entry.start = strtol(tstr1, (char **)NULL, 16); + entry.end = strtol(tstr2, (char **)NULL, 16); + entry.included = 0; + append_romentry(layout, &entry); } - for (i = 0; i < layout.num_entries; i++) { + struct flashrom_layout_entry *var; + SLIST_FOREACH(var, layout, next) { msg_gdbg("romlayout %08x - %08x named %s\n", - layout.entries[i].start, - layout.entries[i].end, layout.entries[i].name); + var->entry.start, var->entry.end, + var->entry.name); } (void)fclose(romlayout); @@ -135,20 +139,20 @@ return 0; } -/* returns the index of the entry (or a negative value if it is not found) */ +/* returns 0 if it is found or a negative value if it is not found */ static int find_romentry(struct flashrom_layout *const l, char *name) { - int i; - - if (l->num_entries == 0) + if (SLIST_EMPTY(l)) return -1; msg_gspew("Looking for region \"%s\"... ", name); - for (i = 0; i < l->num_entries; i++) { - if (!strcmp(l->entries[i].name, name)) { - l->entries[i].included = 1; + + struct flashrom_layout_entry *var; + SLIST_FOREACH(var, l, next) { + if (!strcmp(var->entry.name, name)) { + var->entry.included = 1; msg_gspew("found.\n"); - return i; + return 0; } } msg_gspew("not found.\n"); @@ -167,7 +171,7 @@ return 0; /* User has specified an area, but no layout file is loaded. */ - if (l->num_entries == 0) { + if (SLIST_EMPTY(l)) { msg_gerr("Region requested (with -i \"%s\"), " "but no layout data is available.\n", include_args[0]); @@ -191,7 +195,7 @@ return 0; } -void layout_cleanup(void) +void layout_cleanup(struct flashrom_layout *layout) { int i; for (i = 0; i < num_include_args; i++) { @@ -200,29 +204,31 @@ } num_include_args = 0; - for (i = 0; i < layout.num_entries; i++) { - layout.entries[i].included = 0; + struct flashrom_layout_entry *var; + while (!SLIST_EMPTY(layout)) { /* List Deletion. */ + var = SLIST_FIRST(layout); + SLIST_REMOVE_HEAD(layout, next); + free(var); } - layout.num_entries = 0; } /* Validate and - if needed - normalize layout entries. */ -int normalize_romentries(const struct flashctx *flash) +int normalize_romentries(const struct flashrom_layout *layout, const struct flashctx *flash) { chipsize_t total_size = flash->chip->total_size * 1024; int ret = 0; - int i; - for (i = 0; i < layout.num_entries; i++) { - if (layout.entries[i].start >= total_size || layout.entries[i].end >= total_size) { + struct flashrom_layout_entry *var; + SLIST_FOREACH(var, layout, next) { + if (var->entry.start >= total_size || var->entry.end >= total_size) { msg_gwarn("Warning: Address range of region \"%s\" exceeds the current chip's " - "address space.\n", layout.entries[i].name); - if (layout.entries[i].included) + "address space.\n", var->entry.name); + if (var->entry.included) ret = 1; } - if (layout.entries[i].start > layout.entries[i].end) { + if (var->entry.start > var->entry.end) { msg_gerr("Error: Size of the address range of region \"%s\" is not positive.\n", - layout.entries[i].name); + var->entry.name); ret = 1; } } diff --git a/layout.h b/layout.h index eb54a4f..e87d285 100644 --- a/layout.h +++ b/layout.h @@ -23,6 +23,7 @@ #include <stddef.h> #include <stdint.h> +#include <sys/queue.h> /* Types and macros regarding the maximum flash space size supported by generic code. */ typedef uint32_t chipoff_t; /* Able to store any addressable offset within a supported flash memory. */ @@ -41,19 +42,21 @@ char name[256]; }; -struct flashrom_layout { - /* entries store the entries specified in a layout file and associated run-time data */ - struct romentry *entries; - /* the number of successfully parsed entries */ - size_t num_entries; +SLIST_HEAD(flashrom_layout, flashrom_layout_entry); + +struct flashrom_layout_entry { + SLIST_ENTRY(flashrom_layout_entry) next; + struct romentry entry; }; struct single_layout { struct flashrom_layout base; - struct romentry entry; +// struct romentry entry; }; -struct flashrom_layout *get_global_layout(void); +void init_layout(struct flashrom_layout *layout); +void append_romentry(struct flashrom_layout *layout, struct romentry *entry); +struct romentry get_romentry(struct flashrom_layout_entry *var); struct flashrom_flashctx; const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx); diff --git a/libflashrom.c b/libflashrom.c index 34e881a..6cd919b 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -292,10 +292,10 @@ */ int flashrom_layout_include_region(struct flashrom_layout *const layout, const char *name) { - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (!strcmp(layout->entries[i].name, name)) { - layout->entries[i].included = true; + struct flashrom_layout_entry *var; + SLIST_FOREACH(var, layout, next) { + if (!strcmp(var->entry.name, name)) { + var->entry.included = true; return 0; } } @@ -330,6 +330,7 @@ return 6; #else struct ich_layout dump_layout; + int i; int ret = 1; void *const desc = malloc(0x1000); @@ -362,14 +363,15 @@ goto _finalize_ret; } - if (chip_layout->base.num_entries != dump_layout.base.num_entries || + if (chip_layout->num_entries != dump_layout.num_entries || memcmp(chip_layout->entries, dump_layout.entries, sizeof(dump_layout.entries))) { ret = 5; goto _finalize_ret; } } - *layout = (struct flashrom_layout *)chip_layout; + for (i = 0; i < chip_layout->num_entries; i++) + append_romentry(*layout, &chip_layout->entries[i]); ret = 0; _finalize_ret: @@ -389,10 +391,12 @@ */ void flashrom_layout_release(struct flashrom_layout *const layout) { - if (layout == get_global_layout()) - return; - - free(layout); + struct flashrom_layout_entry *var; + while (!SLIST_EMPTY(layout)) { /* List Deletion. */ + var = SLIST_FIRST(layout); + SLIST_REMOVE_HEAD(layout, next); + free(var); + } } /** -- To view, visit https://review.coreboot.org/27106 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id7f48ac613da1590f30247bee979034cf1f74e9e Gerrit-Change-Number: 27106 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

Change in flashrom[master]: [WIP]layout: Use a linked list for layouts
by build bot (Jenkins) (Code Review) 14 Jun '18

14 Jun '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/27102 ) Change subject: [WIP]layout: Use a linked list for layouts ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1330/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/1099/ : SUCCESS -- To view, visit https://review.coreboot.org/27102 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I049657805ce5676ea4c4b5e49f26d0e8a1f26a3e Gerrit-Change-Number: 27102 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 14 Jun 2018 13:01:51 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: [WIP]layout: Use a linked list for layouts
by Arthur Heymans (Code Review) 14 Jun '18

14 Jun '18

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/27102 Change subject: [WIP]layout: Use a linked list for layouts ...................................................................... [WIP]layout: Use a linked list for layouts This simply replaces the use of a fixed length array for layouts with a linked list. This will make it easier to use multiple methods of getting layouts at the same time (layoutfile, ifd, fmap, ...) and can also easily be extended to do further checks on whether the controller can read/write to regions. Change-Id: I049657805ce5676ea4c4b5e49f26d0e8a1f26a3e Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M flashrom.c M ich_descriptors.c M layout.c M layout.h M libflashrom.c 5 files changed, 116 insertions(+), 81 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/02/27102/1 diff --git a/flashrom.c b/flashrom.c index f85dbb1..f27ad84 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1298,12 +1298,11 @@ /* Fill fallback layout covering the whole chip. */ struct single_layout *const fallback = &flash->fallback_layout; - fallback->base.entries = &fallback->entry; - fallback->base.num_entries = 1; fallback->entry.start = 0; fallback->entry.end = flash->chip->total_size * 1024 - 1; fallback->entry.included = true; strcpy(fallback->entry.name, "complete flash"); + fallback->base.entry = fallback->entry; tmp = flashbuses_to_text(flash->chip->bustype); msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found", @@ -1557,18 +1556,20 @@ */ static int read_by_layout(struct flashctx *const flashctx, uint8_t *const buffer) { - const struct flashrom_layout *const layout = get_layout(flashctx); + const struct flashrom_layout *layout = get_layout(flashctx); - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (!layout->entries[i].included) + while (layout != NULL) { + if (!layout->entry.included) { + layout = layout->next; continue; + } - const chipoff_t region_start = layout->entries[i].start; - const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1; + const chipoff_t region_start = layout->entry.start; + const chipsize_t region_len = layout->entry.end - layout->entry.start + 1; if (flashctx->chip->read(flashctx, buffer + region_start, region_start, region_len)) return 1; + layout = layout->next; } return 0; } @@ -1641,18 +1642,19 @@ static int walk_by_layout(struct flashctx *const flashctx, struct walk_info *const info, const per_blockfn_t per_blockfn) { - const struct flashrom_layout *const layout = get_layout(flashctx); + const struct flashrom_layout *layout = get_layout(flashctx); all_skipped = true; msg_cinfo("Erasing and writing flash chip... "); - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (!layout->entries[i].included) + while (layout != NULL) { + if (!layout->entry.included) { + layout = layout->next; continue; + } - info->region_start = layout->entries[i].start; - info->region_end = layout->entries[i].end; + info->region_start = layout->entry.start; + info->region_end = layout->entry.end; size_t j; int error = 1; /* retry as long as it's 1 */ @@ -1686,6 +1688,7 @@ msg_cerr("FAILED!\n"); return 1; } + layout = layout->next; } if (all_skipped) msg_cinfo("\nWarning: Chip content is identical to the requested image.\n"); @@ -1859,21 +1862,23 @@ static int verify_by_layout(struct flashctx *const flashctx, void *const curcontents, const uint8_t *const newcontents) { - const struct flashrom_layout *const layout = get_layout(flashctx); + const struct flashrom_layout *layout = get_layout(flashctx); - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (!layout->entries[i].included) + while (layout != NULL) { + if (!layout->entry.included) { + layout = layout->next; continue; + } - const chipoff_t region_start = layout->entries[i].start; - const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1; + const chipoff_t region_start = layout->entry.start; + const chipsize_t region_len = layout->entry.end - layout->entry.start + 1; if (flashctx->chip->read(flashctx, curcontents + region_start, region_start, region_len)) return 1; if (compare_range(newcontents + region_start, curcontents + region_start, region_start, region_len)) return 3; + layout = layout->next; } return 0; } @@ -2331,17 +2336,19 @@ static void combine_image_by_layout(const struct flashctx *const flashctx, uint8_t *const newcontents, const uint8_t *const oldcontents) { - const struct flashrom_layout *const layout = get_layout(flashctx); + const struct flashrom_layout *layout = get_layout(flashctx); - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (layout->entries[i].included) + while (layout != NULL) { + if (layout->entry.included) { + layout = layout->next; continue; + } - const chipoff_t region_start = layout->entries[i].start; - const chipsize_t region_len = layout->entries[i].end - layout->entries[i].start + 1; + const chipoff_t region_start = layout->entry.start; + const chipsize_t region_len = layout->entry.end - layout->entry.start + 1; memcpy(newcontents + region_start, oldcontents + region_start, region_len); + layout = layout->next; } } diff --git a/ich_descriptors.c b/ich_descriptors.c index 56e846f..4df34a7 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -17,6 +17,8 @@ #include "ich_descriptors.h" +#include <stdlib.h> + #ifdef ICH_DESCRIPTORS_FROM_DUMP_ONLY #include <stdio.h> #include <string.h> @@ -1179,8 +1181,21 @@ snprintf(layout->entries[j].name, sizeof(layout->entries[j].name), "%s", regions[i]); ++j; } - layout->base.entries = layout->entries; - layout->base.num_entries = j; + + struct flashrom_layout *layout_end = &layout->base; + + for (i = 0; i < j; i++) { + struct flashrom_layout *temp = malloc(sizeof(struct flashrom_layout)); + temp->entry = layout->entries[i]; + temp->next = NULL; + if (i == 0) { + layout->base = *temp; + continue; + } + layout_end->next = temp; + layout_end = temp; + } + return 0; } diff --git a/layout.c b/layout.c index fa66238..d486840 100644 --- a/layout.c +++ b/layout.c @@ -23,8 +23,9 @@ #include "programmer.h" #include "layout.h" -struct romentry entries[MAX_ROMLAYOUT]; -static struct flashrom_layout layout = { entries, 0 }; +static struct flashrom_layout *layout = NULL; +static struct flashrom_layout *layout_end = NULL; + /* include_args holds the arguments specified at the command line with -i. They must be processed at some point * so that desired regions are marked as "included" in the layout. */ @@ -33,23 +34,39 @@ struct flashrom_layout *get_global_layout(void) { - return &layout; + return layout; } const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx) { - if (flashctx->layout && flashctx->layout->num_entries) + if (flashctx->layout) return flashctx->layout; else return &flashctx->fallback_layout.base; } +void append_entry_to_layout(const struct romentry entry) +{ + struct flashrom_layout *temp = malloc(sizeof(struct flashrom_layout)); + temp->entry = entry; + temp->next = NULL; + + if (layout == NULL) { + layout = temp; + layout_end = temp; + return; + } + layout_end->next=temp; + layout_end = temp; +} + #ifndef __LIBPAYLOAD__ int read_romlayout(const char *name) { FILE *romlayout; char tempstr[256]; - int i; + + struct romentry entry; romlayout = fopen(name, "r"); @@ -62,13 +79,7 @@ while (!feof(romlayout)) { char *tstr1, *tstr2; - if (layout.num_entries >= MAX_ROMLAYOUT) { - msg_gerr("Maximum number of ROM images (%i) in layout " - "file reached.\n", MAX_ROMLAYOUT); - (void)fclose(romlayout); - return 1; - } - if (2 != fscanf(romlayout, "%255s %255s\n", tempstr, layout.entries[layout.num_entries].name)) + if (2 != fscanf(romlayout, "%255s %255s\n", tempstr, entry.name)) continue; #if 0 // fscanf does not like arbitrary comments like that :( later @@ -83,16 +94,19 @@ (void)fclose(romlayout); return 1; } - layout.entries[layout.num_entries].start = strtol(tstr1, (char **)NULL, 16); - layout.entries[layout.num_entries].end = strtol(tstr2, (char **)NULL, 16); - layout.entries[layout.num_entries].included = 0; - layout.num_entries++; + + entry.start = strtol(tstr1, (char **)NULL, 16); + entry.end = strtol(tstr2, (char **)NULL, 16); + entry.included = 0; + append_entry_to_layout(entry); } - for (i = 0; i < layout.num_entries; i++) { + const struct flashrom_layout *local_layout = get_global_layout(); + while (local_layout != NULL) { msg_gdbg("romlayout %08x - %08x named %s\n", - layout.entries[i].start, - layout.entries[i].end, layout.entries[i].name); + local_layout->entry.start, + local_layout->entry.end, local_layout->entry.name); + local_layout = local_layout->next; } (void)fclose(romlayout); @@ -135,21 +149,19 @@ return 0; } -/* returns the index of the entry (or a negative value if it is not found) */ +/* returns 0 if an entry is found, -1 if it is not found */ static int find_romentry(struct flashrom_layout *const l, char *name) { - int i; - - if (l->num_entries == 0) - return -1; + struct flashrom_layout *local_layout = l; msg_gspew("Looking for region \"%s\"... ", name); - for (i = 0; i < l->num_entries; i++) { - if (!strcmp(l->entries[i].name, name)) { - l->entries[i].included = 1; + while(local_layout != NULL) { + if (!strcmp(local_layout->entry.name, name)) { + local_layout->entry.included = 1; msg_gspew("found.\n"); - return i; + return 0; } + local_layout = local_layout->next; } msg_gspew("not found.\n"); return -1; @@ -167,7 +179,7 @@ return 0; /* User has specified an area, but no layout file is loaded. */ - if (l->num_entries == 0) { + if (l == NULL) { msg_gerr("Region requested (with -i \"%s\"), " "but no layout data is available.\n", include_args[0]); @@ -200,10 +212,13 @@ } num_include_args = 0; - for (i = 0; i < layout.num_entries; i++) { - layout.entries[i].included = 0; + struct flashrom_layout *tmp = layout; + + while(tmp != NULL) { + struct flashrom_layout *next = tmp->next; + free(tmp); + tmp = next; } - layout.num_entries = 0; } /* Validate and - if needed - normalize layout entries. */ @@ -212,19 +227,21 @@ chipsize_t total_size = flash->chip->total_size * 1024; int ret = 0; - int i; - for (i = 0; i < layout.num_entries; i++) { - if (layout.entries[i].start >= total_size || layout.entries[i].end >= total_size) { + + const struct flashrom_layout *local_layout = get_global_layout(); + while (local_layout != NULL) { + if (local_layout->entry.start >= total_size || local_layout->entry.end >= total_size) { msg_gwarn("Warning: Address range of region \"%s\" exceeds the current chip's " - "address space.\n", layout.entries[i].name); - if (layout.entries[i].included) + "address space.\n", local_layout->entry.name); + if (local_layout->entry.included) ret = 1; } - if (layout.entries[i].start > layout.entries[i].end) { + if (local_layout->entry.start > local_layout->entry.end) { msg_gerr("Error: Size of the address range of region \"%s\" is not positive.\n", - layout.entries[i].name); + local_layout->entry.name); ret = 1; } + local_layout = local_layout->next; } return ret; diff --git a/layout.h b/layout.h index eb54a4f..19c8ac3 100644 --- a/layout.h +++ b/layout.h @@ -43,9 +43,8 @@ struct flashrom_layout { /* entries store the entries specified in a layout file and associated run-time data */ - struct romentry *entries; - /* the number of successfully parsed entries */ - size_t num_entries; + struct romentry entry; + struct flashrom_layout *next; }; struct single_layout { @@ -56,6 +55,7 @@ struct flashrom_layout *get_global_layout(void); struct flashrom_flashctx; const struct flashrom_layout *get_layout(const struct flashrom_flashctx *const flashctx); +void append_entry_to_layout(const struct romentry entry); int process_include_args(struct flashrom_layout *); diff --git a/libflashrom.c b/libflashrom.c index 34e881a..6fc88d7 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -292,12 +292,14 @@ */ int flashrom_layout_include_region(struct flashrom_layout *const layout, const char *name) { - size_t i; - for (i = 0; i < layout->num_entries; ++i) { - if (!strcmp(layout->entries[i].name, name)) { - layout->entries[i].included = true; + struct flashrom_layout *tmp_layout = layout; + + while(tmp_layout != NULL) { + if (!strcmp(layout->entry.name, name)) { + layout->entry.included = true; return 0; } + tmp_layout = tmp_layout->next; } return 1; } @@ -361,15 +363,9 @@ ret = 4; goto _finalize_ret; } - - if (chip_layout->base.num_entries != dump_layout.base.num_entries || - memcmp(chip_layout->entries, dump_layout.entries, sizeof(dump_layout.entries))) { - ret = 5; - goto _finalize_ret; - } } - *layout = (struct flashrom_layout *)chip_layout; + *layout = &chip_layout->base; ret = 0; _finalize_ret: -- To view, visit https://review.coreboot.org/27102 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I049657805ce5676ea4c4b5e49f26d0e8a1f26a3e Gerrit-Change-Number: 27102 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz>

1 0

Change in flashrom[master]: bitbang_spi: Add half-duplex optimizations
by Daniel Thompson (Code Review) 14 Jun '18

14 Jun '18

Daniel Thompson has posted comments on this change. ( https://review.coreboot.org/26947 ) Change subject: bitbang_spi: Add half-duplex optimizations ...................................................................... Patch Set 2: Regarding the "Note sure if the full-duplex mode is used anywhere"... I had originally planned to retain bitbang_spi_rw_byte() based on pretty much the same thought process. However it turned out to only ever be called from a single function that was already explicitly half-duplex. Thus even if some devices did offer a full duplex mode there is currently no way for bitbang_spi to take advantage of it, either before or after my changes. I decided to remove it because the resulting dead code was static and therefore triggered -Werror build failures. -- To view, visit https://review.coreboot.org/26947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I33b9f363716f651146c09113bda5fffe53b16738 Gerrit-Change-Number: 26947 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 14 Jun 2018 11:21:12 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in flashrom[master]: bitbang_spi: Add half-duplex optimizations
by Patrick Georgi (Code Review) 14 Jun '18

14 Jun '18

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/26947 ) Change subject: bitbang_spi: Add half-duplex optimizations ...................................................................... Patch Set 2: Code-Review+1 Not sure if the full-duplex mode is used anywhere with SPI flash. If yes, that (slower) function should probably remain somewhere, but OTOH we don't need dead code either. Nico? -- To view, visit https://review.coreboot.org/26947 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I33b9f363716f651146c09113bda5fffe53b16738 Gerrit-Change-Number: 26947 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 14 Jun 2018 09:58:03 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: bitbang_spi: Add functions to optimize xfers
by Patrick Georgi (Code Review) 14 Jun '18

14 Jun '18

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/26946 ) Change subject: bitbang_spi: Add functions to optimize xfers ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/26946 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic3430a9df34844cdfa82e109456be788eaa1789a Gerrit-Change-Number: 26946 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 14 Jun 2018 09:55:27 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: flashchips: Add Macronix MX25U51245G
by build bot (Jenkins) (Code Review) 07 Jun '18

07 Jun '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/26949 ) Change subject: flashchips: Add Macronix MX25U51245G ...................................................................... Patch Set 2: Verified+1 Build Successful https://qa.coreboot.org/job/flashrom-customrules/1329/ : SUCCESS https://qa.coreboot.org/job/flashrom_gerrit/1098/ : SUCCESS -- To view, visit https://review.coreboot.org/26949 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2117fc205006088967f3d97644375d10db1791f1 Gerrit-Change-Number: 26949 Gerrit-PatchSet: 2 Gerrit-Owner: Daniel Thompson <daniel.thompson(a)linaro.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 07 Jun 2018 15:09:04 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit June 2018