Mathew King has uploaded this change for review.

View Change

ifdtool: Add validate option to ifdtool

Add an option to ifdtool which validates that the flash regions defined
in the descriptor match the coresponding areas in the FMAP.

BUG=chromium:992215
TEST=Ran 'ifdtool -t' with a good bios image and verify no issues
run 'ifdtool -t' with a bad bios image and verify expected issues

Change-Id: Idebf105dee1b8f829d54bd65c82867af7aa4aded
---
M util/ifdtool/Makefile
M util/ifdtool/ifdtool.c
M util/ifdtool/ifdtool.h
3 files changed, 79 insertions(+), 14 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/34802/1
diff --git a/util/ifdtool/Makefile b/util/ifdtool/Makefile
index cc52b1e..a4f0af6 100644
--- a/util/ifdtool/Makefile
+++ b/util/ifdtool/Makefile
@@ -18,10 +18,16 @@
CC = gcc
INSTALL = /usr/bin/env install
PREFIX = /usr/local
-CFLAGS = -O2 -g -Wall -Wextra -Wmissing-prototypes -Werror -I../../src/commonlib/include
+CFLAGS = -O2 -g -Wall -Wextra -Wmissing-prototypes -Werror
+CFLAGS += -I../../src/commonlib/include
+CFLAGS += -I../cbfstool/flashmap
+CFLAGS += -include ../../src/commonlib/include/commonlib/compiler.h
LDFLAGS =

OBJS = ifdtool.o
+OBJS += fmap.o
+OBJS += kv_pair.o
+OBJS += valstr.o

all: dep $(PROGRAM)

@@ -38,6 +44,9 @@
%.o: %.c
$(CC) $(CFLAGS) -c -o $@ $<

+%.o: ../cbfstool/flashmap/%.c
+ $(CC) $(CFLAGS) -c -o $@ $<
+
install: $(PROGRAM)
mkdir -p $(DESTDIR)$(PREFIX)/bin
$(INSTALL) $(PROGRAM) $(DESTDIR)$(PREFIX)/bin
diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c
index 83caa69..eb9b420 100644
--- a/util/ifdtool/ifdtool.c
+++ b/util/ifdtool/ifdtool.c
@@ -22,6 +22,7 @@
#include <sys/types.h>
#include <sys/stat.h>
#include <commonlib/helpers.h>
+#include <fmap.h>
#include "ifdtool.h"

#ifndef O_BINARY
@@ -46,15 +47,15 @@
static int platform = -1;

static const struct region_name region_names[MAX_REGIONS] = {
- { "Flash Descriptor", "fd", "flashregion_0_flashdescriptor.bin" },
- { "BIOS", "bios", "flashregion_1_bios.bin" },
- { "Intel ME", "me", "flashregion_2_intel_me.bin" },
- { "GbE", "gbe", "flashregion_3_gbe.bin" },
- { "Platform Data", "pd", "flashregion_4_platform_data.bin" },
- { "Reserved", "res1", "flashregion_5_reserved.bin" },
- { "Reserved", "res2", "flashregion_6_reserved.bin" },
- { "Reserved", "res3", "flashregion_7_reserved.bin" },
- { "EC", "ec", "flashregion_8_ec.bin" },
+ { "Flash Descriptor", "fd", "flashregion_0_flashdescriptor.bin", "SI_DESC" },
+ { "BIOS", "bios", "flashregion_1_bios.bin", "SI_BIOS" },
+ { "Intel ME", "me", "flashregion_2_intel_me.bin", "SI_ME" },
+ { "GbE", "gbe", "flashregion_3_gbe.bin", "SI_GBE" },
+ { "Platform Data", "pd", "flashregion_4_platform_data.bin", "SI_PDR" },
+ { "Reserved", "res1", "flashregion_5_reserved.bin", NULL },
+ { "Reserved", "res2", "flashregion_6_reserved.bin", NULL },
+ { "Reserved", "res3", "flashregion_7_reserved.bin", NULL },
+ { "EC", "ec", "flashregion_8_ec.bin", "SI_EC" },
};

/* port from flashrom */
@@ -804,6 +805,51 @@
}
}

+static void validate_layout(char *image, int size)
+{
+ uint i, errors = 0;
+ struct fmap* fmap;
+ long int fmap_loc = fmap_find((uint8_t *)image, size);
+ const frba_t *frba = find_frba(image, size);
+
+ if (fmap_loc < 0 || !frba)
+ exit(EXIT_FAILURE);
+
+ fmap = (struct fmap *)(image + fmap_loc);
+
+ for (i = 0; i < max_regions; i++) {
+ if (region_names[i].fmapname == NULL)
+ continue;
+
+ region_t region = get_region(frba, i);
+
+ if (region.size == 0)
+ continue;
+
+ const struct fmap_area *area =
+ fmap_find_area(fmap, region_names[i].fmapname);
+
+ if (!area)
+ continue;
+
+ if ((uint)region.base != area->offset ||
+ (uint)region.size != area->size) {
+ printf("Region mismatch between %s and %s\n",
+ region_names[i].terse, area->name);
+ printf(" Descriptor region %s:\n", region_names[i].terse);
+ printf(" offset: 0x%08x\n", region.base);
+ printf(" length: 0x%08x\n", region.size);
+ printf(" FMAP area %s:\n", area->name);
+ printf(" offset: 0x%08x\n", area->offset);
+ printf(" length: 0x%08x\n", area->size);
+ errors++;
+ }
+ }
+
+ if (errors > 0)
+ exit(EXIT_FAILURE);
+}
+
static void write_image(const char *filename, char *image, int size)
{
char new_filename[FILENAME_MAX]; // allow long file names
@@ -1359,6 +1405,7 @@
printf("\n"
" -d | --dump: dump intel firmware descriptor\n"
" -f | --layout <filename> dump regions into a flashrom layout file\n"
+ " -t | --validate Validate that the firmware descriptor layout matches the fmap layout\n"
" -x | --extract: extract intel fd modules\n"
" -i | --inject <region>:<module> inject file <module> into region <region>\n"
" -n | --newlayout <filename> update regions using a flashrom layout file\n"
@@ -1388,7 +1435,7 @@
{
int opt, option_index = 0;
int mode_dump = 0, mode_extract = 0, mode_inject = 0, mode_spifreq = 0;
- int mode_em100 = 0, mode_locked = 0, mode_unlocked = 0;
+ int mode_em100 = 0, mode_locked = 0, mode_unlocked = 0, mode_validate = 0;
int mode_layout = 0, mode_newlayout = 0, mode_density = 0;
int mode_altmedisable = 0, altmedisable = 0;
char *region_type_string = NULL, *region_fname = NULL;
@@ -1413,10 +1460,11 @@
{"version", 0, NULL, 'v'},
{"help", 0, NULL, 'h'},
{"platform", 0, NULL, 'p'},
+ {"validate", 0, NULL, 't'},
{0, 0, 0, 0}
};

- while ((opt = getopt_long(argc, argv, "df:D:C:M:xi:n:s:p:eluvh?",
+ while ((opt = getopt_long(argc, argv, "df:D:C:M:xi:n:s:p:eluvth?",
long_options, &option_index)) != EOF) {
switch (opt) {
case 'd':
@@ -1593,6 +1641,9 @@
}
fprintf(stderr, "Platform is: %s\n", optarg);
break;
+ case 't':
+ mode_validate = 1;
+ break;
case 'v':
print_version();
exit(EXIT_SUCCESS);
@@ -1608,7 +1659,7 @@

if ((mode_dump + mode_layout + mode_extract + mode_inject +
mode_newlayout + (mode_spifreq | mode_em100 | mode_unlocked |
- mode_locked) + mode_altmedisable) > 1) {
+ mode_locked) + mode_altmedisable + mode_layout) > 1) {
fprintf(stderr, "You may not specify more than one mode.\n\n");
print_usage(argv[0]);
exit(EXIT_FAILURE);
@@ -1616,7 +1667,8 @@

if ((mode_dump + mode_layout + mode_extract + mode_inject +
mode_newlayout + mode_spifreq + mode_em100 + mode_locked +
- mode_unlocked + mode_density + mode_altmedisable) == 0) {
+ mode_unlocked + mode_density + mode_altmedisable +
+ mode_validate) == 0) {
fprintf(stderr, "You need to specify a mode.\n\n");
print_usage(argv[0]);
exit(EXIT_FAILURE);
@@ -1667,6 +1719,9 @@
if (mode_extract)
write_regions(image, size);

+ if (mode_validate)
+ validate_layout(image, size);
+
if (mode_inject)
inject_region(filename, image, size, region_type,
region_fname);
diff --git a/util/ifdtool/ifdtool.h b/util/ifdtool/ifdtool.h
index f3b9a53..195a09c 100644
--- a/util/ifdtool/ifdtool.h
+++ b/util/ifdtool/ifdtool.h
@@ -165,4 +165,5 @@
const char *pretty;
const char *terse;
const char *filename;
+ const char *fmapname;
};

To view, visit change 34802. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Idebf105dee1b8f829d54bd65c82867af7aa4aded
Gerrit-Change-Number: 34802
Gerrit-PatchSet: 1
Gerrit-Owner: Mathew King <mathewk@chromium.org>
Gerrit-MessageType: newchange