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

Signed-off-by: Mathew King <mathewk@chromium.org>
Change-Id: Idebf105dee1b8f829d54bd65c82867af7aa4aded
Reviewed-on: https://review.coreboot.org/c/coreboot/+/34802
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>

Documentation/ifdtool/index.md

@@ -0,0 +1,5 @@
+
+Contents:
+
+* [Intel IFD Binary Extraction](binary_extraction.md)
+* [IFD Layout](layout.md)

Documentation/ifdtool/layout.md

@@ -0,0 +1,66 @@
+# IFD Layout
+
+A coreboot image for an Intel SoC contains two separate definitions of the
+layout of the flash. The Intel Flash Descriptor (IFD) which defines offsets and
+sizes of various regions of flash and the [coreboot FMAP](../lib/flashmap.md).
+
+The FMAP should define all of the of the regions defined by the IFD to ensure
+that those regions are accounted for by coreboot and will not be accidentally
+modified.
+
+## IFD mapping
+
+The names of the IFD regions in the FMAP should follow the convention of
+starting with the prefix `SI_` which stands for `silicon initialization` as a
+way to categorize anything required by the SoC but not provided by coreboot.
+
+|IFD Region index|IFD Region name|FMAP Name|Notes|
+|---|---|---|---|
+|0|Flash Descriptor|SI_DESC|Always the top 4KB of flash|
+|1|BIOS|SI_BIOS|This is the region that contains coreboot|
+|2|Intel ME|SI_ME||
+|3|Gigabit Ethernet|SI_GBE||
+|4|Platform Data|SI_PDR||
+|8|EC Firmware|SI_EC|Most Chrome OS devices do not use this region; EC firmware is stored BIOS region of flash|
+
+## Validation
+
+The ifdtool can be used to manipulate a firmware image with a IFD. This tool
+will not take into account the FMAP while modifying the image which can lead to
+unexpected and hard to debug issues with the firmware image. For example if the
+ME region is defined at 6 MB in the IFD but the FMAP only allocates 4 MB for the
+ME, then when the ME is added by the ifdtool 6 MB will be written which could
+overwrite 2 MB of the BIOS.
+
+In order to validate that the FMAP and the IFD are compatible the ifdtool
+provides --validate (-t) option. `ifdtool -t` will read both the IFD and the
+FMAP in the image and for every non empty region in the IFD if that region is
+defined in the FMAP but the offset or size is different then the tool will
+return an error.
+
+Example:
+
+```console
+foo@bar:~$ ifdtool -t bad_image.bin
+Region mismatch between bios and SI_BIOS
+ Descriptor region bios:
+  offset: 0x00400000
+  length: 0x01c00000
+ FMAP area SI_BIOS:
+  offset: 0x00800000
+  length: 0x01800000
+Region mismatch between me and SI_ME
+ Descriptor region me:
+  offset: 0x00103000
+  length: 0x002f9000
+ FMAP area SI_ME:
+  offset: 0x00103000
+  length: 0x006f9000
+Region mismatch between pd and SI_PDR
+ Descriptor region pd:
+  offset: 0x003fc000
+  length: 0x00004000
+ FMAP area SI_PDR:
+  offset: 0x007fc000
+  length: 0x00004000
+```

Documentation/index.md

@@ -169,7 +169,6 @@ Contents:
 * [coreboot at conferences](community/conferences.md)
 * [Payloads](payloads.md)
 * [Distributions](distributions.md)
-* [Intel IFD Binary Extraction](Binary_Extraction.md)
 * [Dealing with Untrusted Input in SMM](technotes/2017-02-dealing-with-untrusted-input-in-smm.md)
 * [GPIO toggling in ACPI AML](acpi/gpio.md)
 * [Adding devices to a device tree](acpi/devicetree.md)

Documentation/util.md

@@ -47,7 +47,7 @@ Controller (EC). `C`
 * __genprof__ - Format function tracing logs `Bash` `C`
 * __gitconfig__ - Initialize git repository submodules install git
 hooks `Bash`
-* __ifdtool__ - Extract and dump Intel Firmware Descriptor information
+* [__ifdtool__](ifdtool/index.md) - Extract and dump Intel Firmware Descriptor information
 `C`
 * __intelmetool__ - Dump interesting things about Management Engine
 even if hidden `C`

util/ifdtool/Makefile

@@ -18,10 +18,16 @@ PROGRAM = ifdtool
 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 @@ dep:
 %.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

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 selected_chip = 0;
 static int platform = -1;
 
 static const struct region_name region_names[MAX_REGIONS] = {
-	{ "Flash Descriptor", "fd", "flashregion_0_flashdescriptor.bin" },
+	{ "Flash Descriptor", "fd", "flashregion_0_flashdescriptor.bin", "SI_DESC" },
-	{ "BIOS", "bios", "flashregion_1_bios.bin" },
+	{ "BIOS", "bios", "flashregion_1_bios.bin", "SI_BIOS" },
-	{ "Intel ME", "me", "flashregion_2_intel_me.bin" },
+	{ "Intel ME", "me", "flashregion_2_intel_me.bin", "SI_ME" },
-	{ "GbE", "gbe", "flashregion_3_gbe.bin" },
+	{ "GbE", "gbe", "flashregion_3_gbe.bin", "SI_GBE" },
-	{ "Platform Data", "pd", "flashregion_4_platform_data.bin" },
+	{ "Platform Data", "pd", "flashregion_4_platform_data.bin", "SI_PDR" },
-	{ "Reserved", "res1", "flashregion_5_reserved.bin" },
+	{ "Reserved", "res1", "flashregion_5_reserved.bin", NULL },
-	{ "Reserved", "res2", "flashregion_6_reserved.bin" },
+	{ "Reserved", "res2", "flashregion_6_reserved.bin", NULL },
-	{ "Reserved", "res3", "flashregion_7_reserved.bin" },
+	{ "Reserved", "res3", "flashregion_7_reserved.bin", NULL },
-	{ "EC", "ec", "flashregion_8_ec.bin" },
+	{ "EC", "ec", "flashregion_8_ec.bin", "SI_EC" },
 };
 
 /* port from flashrom */
@@ -804,6 +805,51 @@ static void write_regions(char *image, int size)
 	}
 }
 
+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 @@ static void print_usage(const char *name)
 	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 main(int argc, char *argv[])
 {
 	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 @@ int main(int argc, char *argv[])
 		{"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 @@ int main(int argc, char *argv[])
 			}
 			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 @@ int main(int argc, char *argv[])
 
 	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,7 @@ int main(int argc, char *argv[])
 
 	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 +1718,9 @@ int main(int argc, char *argv[])
 	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);

util/ifdtool/ifdtool.h

@@ -165,4 +165,5 @@ struct region_name {
 	const char *pretty;
 	const char *terse;
 	const char *filename;
+	const char *fmapname;
 };