summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMathew King <mathewk@chromium.org>2019-08-08 14:59:25 -0600
committerPatrick Georgi <pgeorgi@google.com>2019-10-03 15:28:15 +0000
commitc7ddc999fc076bf6871e3b5e641c07f17b0b1746 (patch)
treea7dfb7aec7bade6ddf74f23d1f1f6bdb26f62ccd
parent006eb9d8c8550ce0e8d01d37cceb88e7c1e12a5e (diff)
downloadcoreboot-c7ddc999fc076bf6871e3b5e641c07f17b0b1746.tar.xz
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>
-rw-r--r--Documentation/ifdtool/binary_extraction.md (renamed from Documentation/Binary_Extraction.md)0
-rw-r--r--Documentation/ifdtool/index.md5
-rw-r--r--Documentation/ifdtool/layout.md66
-rw-r--r--Documentation/index.md1
-rw-r--r--Documentation/util.md2
-rw-r--r--util/ifdtool/Makefile11
-rw-r--r--util/ifdtool/ifdtool.c80
-rw-r--r--util/ifdtool/ifdtool.h1
8 files changed, 150 insertions, 16 deletions
diff --git a/Documentation/Binary_Extraction.md b/Documentation/ifdtool/binary_extraction.md
index fe8c25ca4e..fe8c25ca4e 100644
--- a/Documentation/Binary_Extraction.md
+++ b/Documentation/ifdtool/binary_extraction.md
diff --git a/Documentation/ifdtool/index.md b/Documentation/ifdtool/index.md
new file mode 100644
index 0000000000..e6e6057a74
--- /dev/null
+++ b/Documentation/ifdtool/index.md
@@ -0,0 +1,5 @@
+
+Contents:
+
+* [Intel IFD Binary Extraction](binary_extraction.md)
+* [IFD Layout](layout.md) \ No newline at end of file
diff --git a/Documentation/ifdtool/layout.md b/Documentation/ifdtool/layout.md
new file mode 100644
index 0000000000..950db6f7ff
--- /dev/null
+++ b/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
+``` \ No newline at end of file
diff --git a/Documentation/index.md b/Documentation/index.md
index b880c1c4d5..39c8d11f9a 100644
--- a/Documentation/index.md
+++ b/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)
diff --git a/Documentation/util.md b/Documentation/util.md
index f8fabc1ec3..3a09b00101 100644
--- a/Documentation/util.md
+++ b/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`
diff --git a/util/ifdtool/Makefile b/util/ifdtool/Makefile
index cc52b1ed4a..a4f0af6217 100644
--- a/util/ifdtool/Makefile
+++ b/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
diff --git a/util/ifdtool/ifdtool.c b/util/ifdtool/ifdtool.c
index 83caa693ab..0e83c760d8 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 selected_chip = 0;
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 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);
diff --git a/util/ifdtool/ifdtool.h b/util/ifdtool/ifdtool.h
index f3b9a53e06..195a09cce4 100644
--- a/util/ifdtool/ifdtool.h
+++ b/util/ifdtool/ifdtool.h
@@ -165,4 +165,5 @@ struct region_name {
const char *pretty;
const char *terse;
const char *filename;
+ const char *fmapname;
};