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@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: