Bill XIE has uploaded this change for review. ( https://review.coreboot.org/21315
Change subject: ifdtool: refactor region_name(s) and new_layout ......................................................................
ifdtool: refactor region_name(s) and new_layout
The feature "--newlayout" is designed to use scheme "right-align" to copy every regions since its birstday (commit 4eabe1e), but such scheme may be only valid for the bios region. For example, it will cut off the starting chunk of the me region when attempting to shrink it, according to https://github.com/corna/me_cleaner/issues/58 .
In this commit, I add a member "dir" to "struct region_info" (former "struct region_name" to record the alignment direction of the region, and for now, only bios region uses "right-align". (ME region should use "left-align", according to me_cleaner's practice)
Change-Id: Ia2b92c3f835f9849a243a48c37588c2dbe7449bf Signed-off-by: Bill XIE persmule@gmail.com --- M util/ifdtool/ifdtool.c M util/ifdtool/ifdtool.h 2 files changed, 102 insertions(+), 42 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/21315/1
diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c index d2e530d..a12611e 100644 --- a/util/ifdtool/ifdtool.c +++ b/util/ifdtool/ifdtool.c @@ -32,16 +32,16 @@ static int selected_chip = 0; static int platform = -1;
-static const struct region_name region_names[MAX_REGIONS] = { - { "Flash Descriptor", "fd" }, - { "BIOS", "bios" }, - { "Intel ME", "me" }, - { "GbE", "gbe" }, - { "Platform Data", "pd" }, - { "Reserved", "res1" }, - { "Reserved", "res2" }, - { "Reserved", "res3" }, - { "EC", "ec" }, +static const struct region_info region_infos[MAX_REGIONS] = { + { "Flash Descriptor", "fd", ALD_left }, + { "BIOS", "bios", ALD_right }, + { "Intel ME", "me", ALD_left }, + { "GbE", "gbe", ALD_left }, + { "Platform Data", "pd", ALD_left }, + { "Reserved", "res1", ALD_left }, + { "Reserved", "res2", ALD_left }, + { "Reserved", "res3", ALD_left }, + { "EC", "ec", ALD_left }, };
static fdbar_t *find_fd(char *image, int size) @@ -150,7 +150,7 @@ exit (EXIT_FAILURE); }
- return region_names[region_type].pretty; + return region_infos[region_type].pretty; }
static const char *region_name_short(int region_type) @@ -160,7 +160,17 @@ exit (EXIT_FAILURE); }
- return region_names[region_type].terse; + return region_infos[region_type].terse; +} + +static int region_align_dir(int region_type) +{ + if (region_type < 0 || region_type >= max_regions) { + fprintf(stderr, "Invalid region type.\n"); + exit (EXIT_FAILURE); + } + + return region_infos[region_type].dir; }
static int region_num(const char *name) @@ -168,9 +178,9 @@ int i;
for (i = 0; i < max_regions; i++) { - if (strcasecmp(name, region_names[i].pretty) == 0) + if (strcasecmp(name, region_infos[i].pretty) == 0) return i; - if (strcasecmp(name, region_names[i].terse) == 0) + if (strcasecmp(name, region_infos[i].terse) == 0) return i; }
@@ -911,6 +921,62 @@ return 0; }
+/** + * Copy a region into new image's memory. + * + * @param image, size current image with size. + * @param new_image, new_size new image with size. + * @param current_regions, new_regions pointer to array of region_t. + * @param region_type type of a region, also act as index. + */ +static void copy_region(const char *image, int size, + char *new_image, int new_size, + const region_t (*current_regions)[MAX_REGIONS], + const region_t (*new_regions)[MAX_REGIONS], + int region_type) +{ + int dir = region_align_dir(region_type); + const region_t *current = &(*current_regions)[region_type]; + const region_t *new = &(*new_regions)[region_type]; + int copy_size = new->size; + int offset_current = 0, offset_new = 0; + if (dir == ALD_right) { + if (new->size > current->size) { + /* copy from the end of the current region */ + copy_size = current->size; + offset_new = new->size - current->size; + } + + if (new->size < current->size) { + /* copy to the end of the new region */ + offset_current = current->size - new->size; + } + } else { + if (new->size > current->size) { + /* copy from the start of the current region */ + copy_size = current->size; + } + } + + printf("Copy Descriptor %d (%s) (%d bytes)\n", region_type, + region_name(region_type), copy_size); + printf(" from %08x+%08x:%08x (%10d)\n", current->base, + offset_current, current->limit, current->size); + printf(" to %08x+%08x:%08x (%10d)\n", new->base, + offset_new, new->limit, new->size); + + if (((new->base + offset_new) > new_size) && + (current->base + offset_current > size)) { + fprintf(stderr, "Region %d (%s) exceeds the end of the image.\n", + region_type, region_name(region_type)); + exit(EXIT_FAILURE); + } + + memcpy(new_image + new->base + offset_new, + image + current->base + offset_current, + copy_size); +} + void new_layout(const char *filename, char *image, int size, const char *layout_fname) { @@ -1010,35 +1076,11 @@ new_image = malloc(new_extent); memset(new_image, 0xff, new_extent); for (i = 0; i < max_regions; i++) { - int copy_size = new_regions[i].size; - int offset_current = 0, offset_new = 0; - region_t current = current_regions[i]; - region_t new = new_regions[i]; - - if (new.size == 0) + if (new_regions[i].size == 0) continue;
- if (new.size > current.size) { - /* copy from the end of the current region */ - copy_size = current.size; - offset_new = new.size - current.size; - } - - if (new.size < current.size) { - /* copy to the end of the new region */ - offset_current = current.size - new.size; - } - - printf("Copy Descriptor %d (%s) (%d bytes)\n", i, - region_name(i), copy_size); - printf(" from %08x+%08x:%08x (%10d)\n", current.base, - offset_current, current.limit, current.size); - printf(" to %08x+%08x:%08x (%10d)\n", new.base, - offset_new, new.limit, new.size); - - memcpy(new_image + new.base + offset_new, - image + current.base + offset_current, - copy_size); + copy_region(image, size, new_image, new_extent, + ¤t_regions, &new_regions, i); }
/* update new descriptor regions */ diff --git a/util/ifdtool/ifdtool.h b/util/ifdtool/ifdtool.h index b4a6cd4..a7a8398 100644 --- a/util/ifdtool/ifdtool.h +++ b/util/ifdtool/ifdtool.h @@ -64,6 +64,23 @@ #define MAX_REGIONS 9 #define MAX_REGIONS_OLD 5
+enum region_types { + RT_fd, + RT_bios, + RT_me, + RT_gbe, + RT_pd, + RT_res1, + RT_res2, + RT_res3, + RT_ec +}; + +enum alignment_direction { + ALD_left, + ALD_right +}; + typedef struct { uint32_t flreg[MAX_REGIONS]; } __attribute__((packed)) frba_t; @@ -118,7 +135,8 @@ int base, limit, size; } region_t;
-struct region_name { +struct region_info { const char *pretty; const char *terse; + enum alignment_direction dir; };
-
Bill XIE (Code Review)