<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>+                          &current_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>