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