<p>Bill XIE has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/21511">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">ifdtool: refacter some code<br><br>add find_fcba(), find_frba(), find_fmba(), find_fpsba()<br>and find_fmsba() to replace those copy-pasted addressings.<br><br>This commit is one separated from the original I6d05418c.<br><br>Change-Id: I98965711e4cb9792e5cc86cc4c1035559e0274f5<br>Signed-off-by: Bill XIE <persmule@gmail.com><br>---<br>M util/ifdtool/ifdtool.c<br>1 file changed, 112 insertions(+), 54 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/21511/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 f6524c9..a0245c7 100644<br>--- a/util/ifdtool/ifdtool.c<br>+++ b/util/ifdtool/ifdtool.c<br>@@ -28,6 +28,17 @@<br> #define O_BINARY 0<br> #endif<br> <br>+/**<br>+ * PTR_IN_RANGE - examine whether a pointer falls in [base, base + limit)<br>+ * @param ptr: the non-void* pointer to a single arbitrary-sized object.<br>+ * @param base: base address represented with char* type.<br>+ * @param limit: upper limit of the legal address.<br>+ *<br>+ */<br>+#define PTR_IN_RANGE(ptr, base, limit) \<br>+ ((const char *)(ptr) >= (base) && \<br>+ (const char *)&(ptr)[1] <= (base) + (limit))<br>+<br> static int ifd_version;<br> static unsigned int max_regions = 0;<br> static int selected_chip = 0;<br>@@ -62,7 +73,56 @@<br> return NULL;<br> }<br> <br>- return (fdbar_t *) (image + i);<br>+ fdbar_t *fdb = (fdbar_t *) (image + i);<br>+ return PTR_IN_RANGE(fdb, image, size) ? fdb : NULL;<br>+}<br>+<br>+static fcba_t *find_fcba(char *image, int size)<br>+{<br>+ fdbar_t *fdb = find_fd(image, size);<br>+ if (!fdb)<br>+ return NULL;<br>+ fcba_t *fcba = (fcba_t *) (image + ((fdb->flmap0 & 0xff) << 4));<br>+ return PTR_IN_RANGE(fcba, image, size) ? fcba : NULL;<br>+<br>+}<br>+<br>+static fmba_t *find_fmba(char *image, int size)<br>+{<br>+ fdbar_t *fdb = find_fd(image, size);<br>+ if (!fdb)<br>+ return NULL;<br>+ fmba_t *fmba = (fmba_t *) (image + ((fdb->flmap1 & 0xff) << 4));<br>+ return PTR_IN_RANGE(fmba, image, size) ? fmba : NULL;<br>+}<br>+<br>+static frba_t *find_frba(char *image, int size)<br>+{<br>+ fdbar_t *fdb = find_fd(image, size);<br>+ if (!fdb)<br>+ return NULL;<br>+ frba_t *frba =<br>+ (frba_t *) (image + (((fdb->flmap0 >> 16) & 0xff) << 4));<br>+ return PTR_IN_RANGE(frba, image, size) ? frba : NULL;<br>+}<br>+<br>+static fpsba_t *find_fpsba(char *image, int size)<br>+{<br>+ fdbar_t *fdb = find_fd(image, size);<br>+ if (!fdb)<br>+ return NULL;<br>+ fpsba_t *fpsba =<br>+ (fpsba_t *) (image + (((fdb->flmap1 >> 16) & 0xff) << 4));<br>+ return PTR_IN_RANGE(fpsba, image, size) ? fpsba : NULL;<br>+}<br>+<br>+static fmsba_t *find_fmsba(char *image, int size)<br>+{<br>+ fdbar_t *fdb = find_fd(image, size);<br>+ if (!fdb)<br>+ return NULL;<br>+ fmsba_t *fmsba = (fmsba_t *) (image + ((fdb->flmap2 & 0xff) << 4));<br>+ return PTR_IN_RANGE(fmsba, image, size) ? fmsba : NULL;<br> }<br> <br> /*<br>@@ -72,14 +132,9 @@<br> */<br> static void check_ifd_version(char *image, int size)<br> {<br>- fdbar_t *fdb = find_fd(image, size);<br>- fcba_t *fcba;<br> int read_freq;<br> <br>- if (!fdb)<br>- exit(EXIT_FAILURE);<br>-<br>- fcba = (fcba_t *) (image + (((fdb->flmap0) & 0xff) << 4));<br>+ const fcba_t *fcba = find_fcba(image, size);<br> if (!fcba)<br> exit(EXIT_FAILURE);<br> <br>@@ -430,11 +485,10 @@<br> <br> static void dump_fmsba(const fmsba_t *fmsba)<br> {<br>+ unsigned int i;<br> printf("Found Processor Strap Section\n");<br>- printf("????: 0x%08x\n", fmsba->data[0]);<br>- printf("????: 0x%08x\n", fmsba->data[1]);<br>- printf("????: 0x%08x\n", fmsba->data[2]);<br>- printf("????: 0x%08x\n", fmsba->data[3]);<br>+ for (i = 0; i < ARRAY_SIZE(fmsba->data); i++)<br>+ printf("????: 0x%08x\n", fmsba->data[i]);<br> }<br> <br> static void dump_jid(uint32_t jid)<br>@@ -528,7 +582,7 @@<br> <br> static void dump_fd(char *image, int size)<br> {<br>- fdbar_t *fdb = find_fd(image, size);<br>+ const fdbar_t *fdb = find_fd(image, size);<br> if (!fdb)<br> exit(EXIT_FAILURE);<br> <br>@@ -557,36 +611,40 @@<br> (image + ((fdb->flumap1 & 0xff) << 4)),<br> (fdb->flumap1 >> 8) & 0xff);<br> dump_oem((const uint8_t *)image + 0xf00);<br>- dump_frba((const frba_t *)<br>- (image + (((fdb->flmap0 >> 16) & 0xff) << 4)));<br>- dump_fcba((const fcba_t *) (image + ((fdb->flmap0 & 0xff) << 4)));<br>- dump_fpsba((const fpsba_t *)<br>- (image + (((fdb->flmap1 >> 16) & 0xff) << 4)));<br>- dump_fmba((const fmba_t *) (image + ((fdb->flmap1 & 0xff) << 4)));<br>- dump_fmsba((const fmsba_t *) (image + ((fdb->flmap2 & 0xff) << 4)));<br>+<br>+ const frba_t *frba = find_frba(image, size);<br>+ const fcba_t *fcba = find_fcba(image, size);<br>+ const fpsba_t *fpsba = find_fpsba(image, size);<br>+ const fmba_t *fmba = find_fmba(image, size);<br>+ const fmsba_t *fmsba = find_fmsba(image, size);<br>+<br>+ if (frba && fcba && fpsba && fmba && fmsba) {<br>+ dump_frba(frba);<br>+ dump_fcba(fcba);<br>+ dump_fpsba(fpsba);<br>+ dump_fmba(fmba);<br>+ dump_fmsba(fmsba);<br>+ } else {<br>+ printf("FD is corrupted!\n");<br>+ }<br> }<br> <br> static void dump_layout(char *image, int size, const char *layout_fname)<br> {<br>- fdbar_t *fdb = find_fd(image, size);<br>- if (!fdb)<br>+ const frba_t *frba = find_frba(image, size);<br>+ if (!frba)<br> exit(EXIT_FAILURE);<br> <br>- dump_frba_layout((frba_t *)<br>- (image + (((fdb->flmap0 >> 16) & 0xff) << 4)),<br>- layout_fname);<br>+ dump_frba_layout(frba, layout_fname);<br> }<br> <br> static void write_regions(char *image, int size)<br> {<br> unsigned int i;<br>+ const frba_t *frba = find_frba(image, size);<br> <br>- fdbar_t *fdb = find_fd(image, size);<br>- if (!fdb)<br>+ if (!frba)<br> exit(EXIT_FAILURE);<br>-<br>- frba_t *frba =<br>- (frba_t *) (image + (((fdb->flmap0 >> 16) & 0xff) << 4));<br> <br> for (i = 0; i < max_regions; i++) {<br> region_t region = get_region(frba, i);<br>@@ -634,8 +692,9 @@<br> static void set_spi_frequency(const char *filename, char *image, int size,<br> enum spi_frequency freq)<br> {<br>- fdbar_t *fdb = find_fd(image, size);<br>- fcba_t *fcba = (fcba_t *) (image + (((fdb->flmap0) & 0xff) << 4));<br>+ fcba_t *fcba = find_fcba(image, size);<br>+ if (!fcba)<br>+ exit(EXIT_FAILURE);<br> <br> /* clear bits 21-29 */<br> fcba->flcomp &= ~0x3fe00000;<br>@@ -651,8 +710,10 @@<br> <br> static void set_em100_mode(const char *filename, char *image, int size)<br> {<br>- fdbar_t *fdb = find_fd(image, size);<br>- fcba_t *fcba = (fcba_t *) (image + (((fdb->flmap0) & 0xff) << 4));<br>+ fcba_t *fcba = find_fcba(image, size);<br>+ if (!fcba)<br>+ exit(EXIT_FAILURE);<br>+<br> int freq;<br> <br> switch (ifd_version) {<br>@@ -674,8 +735,9 @@<br> static void set_chipdensity(const char *filename, char *image, int size,<br> unsigned int density)<br> {<br>- fdbar_t *fdb = find_fd(image, size);<br>- fcba_t *fcba = (fcba_t *) (image + (((fdb->flmap0) & 0xff) << 4));<br>+ fcba_t *fcba = find_fcba(image, size);<br>+ if (!fcba)<br>+ exit(EXIT_FAILURE);<br> <br> printf("Setting chip density to ");<br> decode_component_density(density);<br>@@ -727,8 +789,9 @@<br> static void lock_descriptor(const char *filename, char *image, int size)<br> {<br> int wr_shift, rd_shift;<br>- fdbar_t *fdb = find_fd(image, size);<br>- fmba_t *fmba = (fmba_t *) (image + (((fdb->flmap1) & 0xff) << 4));<br>+ fmba_t *fmba = find_fmba(image, size);<br>+ if (!fmba)<br>+ exit(EXIT_FAILURE);<br> /* TODO: Dynamically take Platform Data Region and GbE Region<br> * into regard.<br> */<br>@@ -783,8 +846,9 @@<br> <br> static void unlock_descriptor(const char *filename, char *image, int size)<br> {<br>- fdbar_t *fdb = find_fd(image, size);<br>- fmba_t *fmba = (fmba_t *) (image + (((fdb->flmap1) & 0xff) << 4));<br>+ fmba_t *fmba = find_fmba(image, size);<br>+ if (!fmba)<br>+ exit(EXIT_FAILURE);<br> <br> if (ifd_version >= IFD_VERSION_2) {<br> /* Access bits for each region are read: 19:8 write: 31:20 */<br>@@ -804,11 +868,9 @@<br> void inject_region(const char *filename, char *image, int size,<br> unsigned int region_type, const char *region_fname)<br> {<br>- fdbar_t *fdb = find_fd(image, size);<br>- if (!fdb)<br>+ frba_t *frba = find_frba(image, size);<br>+ if (!frba)<br> exit(EXIT_FAILURE);<br>- frba_t *frba =<br>- (frba_t *) (image + (((fdb->flmap0 >> 16) & 0xff) << 4));<br> <br> region_t region = get_region(frba, region_type);<br> if (region.size <= 0xfff) {<br>@@ -913,12 +975,9 @@<br> char *new_image;<br> <br> /* load current descriptor map and regions */<br>- fdbar_t *fdb = find_fd(image, size);<br>- if (!fdb)<br>+ frba_t *frba = find_frba(image, size);<br>+ if (!frba)<br> exit(EXIT_FAILURE);<br>-<br>- frba_t *frba =<br>- (frba_t *) (image + (((fdb->flmap0 >> 16) & 0xff) << 4));<br> <br> for (i = 0; i < max_regions; i++) {<br> current_regions[i] = get_region(frba, i);<br>@@ -1030,14 +1089,12 @@<br> }<br> <br> /* update new descriptor regions */<br>- fdb = find_fd(new_image, new_extent);<br>- if (!fdb)<br>+ frba = find_frba(new_image, new_extent);<br>+ if (!frba)<br> exit(EXIT_FAILURE);<br> <br>- frba = (frba_t *) (new_image + (((fdb->flmap0 >> 16) & 0xff) << 4));<br>- for (i = 1; i < max_regions; i++) {<br>+ for (i = 1; i < max_regions; i++)<br> set_region(frba, i, &new_regions[i]);<br>- }<br> <br> write_image(filename, new_image, new_extent);<br> free(new_image);<br>@@ -1089,7 +1146,8 @@<br> int mode_dump = 0, mode_extract = 0, mode_inject = 0, mode_spifreq = 0;<br> int mode_em100 = 0, mode_locked = 0, mode_unlocked = 0;<br> int mode_layout = 0, mode_newlayout = 0, mode_density = 0;<br>- char *region_type_string = NULL, *region_fname = NULL, *layout_fname = NULL;<br>+ char *region_type_string = NULL, *region_fname = NULL;<br>+ const char *layout_fname = NULL;<br> int region_type = -1, inputfreq = 0;<br> unsigned int new_density = 0;<br> enum spi_frequency spifreq = SPI_FREQUENCY_20MHZ;<br></pre><p>To view, visit <a href="https://review.coreboot.org/21511">change 21511</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/21511"/><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: I98965711e4cb9792e5cc86cc4c1035559e0274f5 </div>
<div style="display:none"> Gerrit-Change-Number: 21511 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Bill XIE <persmule@gmail.com> </div>