CBFS: Automate ROM image layout and remove hardcoded offsets

Non-x86 boards currently need to hardcode the position of their CBFS
master header in a Kconfig. This is very brittle because it is usually
put in between the bootblock and the first CBFS entry, without any
checks to guarantee that it won't overlap either of those. It is not fun
to debug random failures that move and disappear with tiny alignment
changes because someone decided to write "ORBC1112" over some part of
your data section (in a way that is not visible in the symbolized .elf
binaries, only in the final image). This patch seeks to prevent those
issues and reduce the need for manual configuration by making the image
layout a completely automated part of cbfstool.

Since automated placement of the CBFS header means we can no longer
hardcode its position into coreboot, this patch takes the existing x86
solution of placing a pointer to the header at the very end of the
CBFS-managed section of the ROM and generalizes it to all architectures.
This is now even possible with the read-only/read-write split in
ChromeOS, since coreboot knows how large that section is from the
CBFS_SIZE Kconfig (which is by default equal to ROM_SIZE, but can be
changed on systems that place other data next to coreboot/CBFS in ROM).

Also adds a feature to cbfstool that makes the -B (bootblock file name)
argument on image creation optional, since we have recently found valid
use cases for CBFS images that are not the first boot medium of the
device (instead opened by an earlier bootloader that can already
interpret CBFS) and therefore don't really need a bootblock.

BRANCH=None
BUG=None
TEST=Built and booted on Veyron_Pinky, Nyan_Blaze and Falco.

Change-Id: Ib715bb8db258e602991b34f994750a2d3e2d5adf
Signed-off-by: Patrick Georgi <pgeorgi@chromium.org>
Original-Commit-Id: e9879c0fbd57f105254c54bacb3e592acdcad35c
Original-Change-Id: Ifcc755326832755cfbccd6f0a12104cba28a20af
Original-Signed-off-by: Julius Werner <jwerner@chromium.org>
Original-Reviewed-on: https://chromium-review.googlesource.com/229975
Reviewed-on: http://review.coreboot.org/9620
Tested-by: build bot (Jenkins)
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>

3 files changed, 107 insertions, 74 deletions

diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index 1f60d68154..0230d8032e 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -66,13 +66,6 @@ static const struct typedesc_t types_cbfs_compression[] = {
 	{0, NULL},
 };
 
-static uint32_t align_up(uint32_t value, uint32_t align)
-{
-	if (value % align)
-		value += align - (value % align);
-	return value;
-}
-
 static const char *lookup_name_by_type(const struct typedesc_t *desc, uint32_t type,
 				const char *default_value)
 {
@@ -180,6 +173,7 @@ int cbfs_image_create(struct cbfs_image *image,
 {
 	struct cbfs_header header;
 	struct cbfs_file *entry;
+	int32_t *rel_offset;
 	uint32_t cbfs_len;
 	size_t entry_header_len;
 	void *header_loc;
@@ -207,9 +201,6 @@ int cbfs_image_create(struct cbfs_image *image,
 	      "header=0x%x, entries_offset=0x%x\n",
 	      bootblock_offset, header_offset, entries_offset);
 
-	if (align == 0)
-		align = 64;  // default align size.
-
 	// Prepare bootblock
 	if (bootblock_offset + bootblock->size > size) {
 		ERROR("Bootblock (0x%x+0x%zx) exceed ROM size (0x%zx)\n",
@@ -226,7 +217,7 @@ int cbfs_image_create(struct cbfs_image *image,
 	       bootblock->size);
 
 	// Prepare header
-	if (header_offset + sizeof(header) > size) {
+	if (header_offset + sizeof(header) > size - sizeof(int32_t)) {
 		ERROR("Header (0x%x+0x%zx) exceed ROM size (0x%zx)\n",
 		      header_offset, sizeof(header), size);
 		return -1;
@@ -242,6 +233,14 @@ int cbfs_image_create(struct cbfs_image *image,
 	header_loc = (image->buffer.data + header_offset);
 	cbfs_put_header(header_loc, image->header);
 
+	// The last 4 byte of the image contain the relative offset from the end
+	// of the image to the master header as a 32-bit signed integer. x86
+	// relies on this also being its (memory-mapped, top-aligned) absolute
+	// 32-bit address by virtue of how two's complement numbers work.
+	assert(size % sizeof(int32_t) == 0);
+	rel_offset = (int32_t *)(image->buffer.data + size - sizeof(int32_t));
+	*rel_offset = header_offset - size;
+
 	// Prepare entries
 	if (align_up(entries_offset, align) != entries_offset) {
 		ERROR("Offset (0x%x) must be aligned to 0x%x.\n",
@@ -256,8 +255,8 @@ int cbfs_image_create(struct cbfs_image *image,
 	}
 	entry = (struct cbfs_file *)(image->buffer.data + entries_offset);
 	// To calculate available length, find
-	//   e = min(bootblock, header, size) where e > entries_offset.
-	cbfs_len = size;
+	//   e = min(bootblock, header, rel_offset) where e > entries_offset.
+	cbfs_len = size - sizeof(int32_t);
 	if (bootblock_offset > entries_offset && bootblock_offset < cbfs_len)
 		cbfs_len = bootblock_offset;
 	if (header_offset > entries_offset && header_offset < cbfs_len)
@@ -772,17 +771,21 @@ struct cbfs_header *cbfs_find_header(char *data, size_t size)
 {
 	size_t offset;
 	int found = 0;
-	uint32_t x86sig;
+	int32_t rel_offset;
 	struct cbfs_header *header, *result = NULL;
 
-	// Try x86 style (check signature in bottom) header first.
-	x86sig = *(uint32_t *)(data + size - sizeof(uint32_t));
-	offset = (x86sig + (uint32_t)size);
-	DEBUG("x86sig: 0x%x, offset: 0x%zx\n", x86sig, offset);
+	// Try finding relative offset of master header at end of file first.
+	rel_offset = *(int32_t *)(data + size - sizeof(int32_t));
+	offset = size + rel_offset;
+	DEBUG("relative offset: %#zx(-%#zx), offset: %#zx\n",
+	      (size_t)rel_offset, (size_t)-rel_offset, offset);
 	if (offset >= size - sizeof(*header) ||
 	    ntohl(((struct cbfs_header *)(data + offset))->magic) !=
-	    CBFS_HEADER_MAGIC)
+	    CBFS_HEADER_MAGIC) {
+		// Some use cases append non-CBFS data to the end of the ROM.
+		DEBUG("relative offset seems wrong, scanning whole image...\n");
 		offset = 0;
+	}
 
 	for (; offset + sizeof(*header) < size; offset++) {
 		header = (struct cbfs_header *)(data + offset);
@@ -793,14 +796,15 @@ struct cbfs_header *cbfs_find_header(char *data, size_t size)
 			// Probably not a real CBFS header?
 			continue;
 		}
-		found++;
-		result = header;
+		if (!found++)
+			result = header;
 	}
-	if (found > 1) {
-		ERROR("multiple (%d) CBFS headers found!\n",
+	if (found > 1)
+		// Top-aligned images usually have a working relative offset
+		// field, so this is more likely to happen on bottom-aligned
+		// ones (where the first header is the "outermost" one)
+		WARN("Multiple (%d) CBFS headers found, using the first one.\n",
 		       found);
-		result = NULL;
-	}
 	return result;
 }
 
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index f57f288416..9c611197cf 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -53,7 +53,8 @@ static struct param {
 	uint32_t size;
 	uint32_t alignment;
 	uint32_t pagesize;
-	uint32_t offset;
+	uint32_t cbfsoffset;
+	uint32_t cbfsoffset_assigned;
 	uint32_t top_aligned;
 	uint32_t arch;
 	int fit_empty_entries;
@@ -339,40 +340,58 @@ static int cbfs_create(void)
 		return 1;
 	}
 
-	if (!param.bootblock) {
-		ERROR("You need to specify -B/--bootblock.\n");
-		return 1;
-	}
-
 	if (param.arch == CBFS_ARCHITECTURE_UNKNOWN) {
 		ERROR("You need to specify -m/--machine arch.\n");
 		return 1;
 	}
 
-	if (buffer_from_file(&bootblock, param.bootblock) != 0) {
+	if (!param.bootblock) {
+		DEBUG("-B not given, creating image without bootblock.\n");
+		buffer_create(&bootblock, 0, "(dummy)");
+	} else if (buffer_from_file(&bootblock, param.bootblock)) {
 		return 1;
 	}
 
-	// Setup default boot offset and header offset.
+	if (!param.alignment)
+		param.alignment = 64;	// default CBFS entry alignment
+
+	// Set default offsets. x86, as usual, needs to be a special snowflake.
 	if (!param.baseaddress_assigned) {
-		// put boot block before end of ROM.
-		param.baseaddress = param.size - bootblock.size;
-		DEBUG("bootblock in end of ROM.\n");
+		if (param.arch == CBFS_ARCHITECTURE_X86) {
+			// Make sure there's at least enough room for rel_offset
+			param.baseaddress = param.size - (
+				bootblock.size > sizeof(int32_t) ?
+				bootblock.size : sizeof(int32_t));
+			DEBUG("x86 -> bootblock lies at end of ROM (%#x).\n",
+			      param.baseaddress);
+		} else {
+			param.baseaddress = 0;
+			DEBUG("bootblock starts at address 0x0.\n");
+		}
 	}
 	if (!param.headeroffset_assigned) {
-		// Put header before bootblock, and make a reference in end of
-		// bootblock.
-		param.headeroffset = (
-				param.baseaddress -
-				sizeof(struct cbfs_header));
-		if (bootblock.size >= sizeof(uint32_t)) {
-			// TODO this only works for 32b top-aligned system now...
-			uint32_t ptr = param.headeroffset - param.size;
-			uint32_t *sig = (uint32_t *)(bootblock.data +
-						     bootblock.size -
-						     sizeof(ptr));
-			*sig = ptr;
-			DEBUG("CBFS header reference in end of bootblock.\n");
+		if (param.arch == CBFS_ARCHITECTURE_X86) {
+			param.headeroffset = param.baseaddress -
+					     sizeof(struct cbfs_header);
+			DEBUG("x86 -> CBFS header before bootblock (%#x).\n",
+				param.headeroffset);
+		} else {
+			param.headeroffset = align_up(param.baseaddress +
+				bootblock.size, sizeof(uint32_t));
+			DEBUG("CBFS header placed behind bootblock (%#x).\n",
+				param.headeroffset);
+		}
+	}
+	if (!param.cbfsoffset_assigned) {
+		if (param.arch == CBFS_ARCHITECTURE_X86) {
+			param.cbfsoffset = 0;
+			DEBUG("x86 -> CBFS entries start at address 0x0.\n");
+		} else {
+			param.cbfsoffset = align_up(param.headeroffset +
+						    sizeof(struct cbfs_header),
+						    param.alignment);
+			DEBUG("CBFS entries start beind master header (%#x).\n",
+			      param.cbfsoffset);
 		}
 	}
 
@@ -383,7 +402,7 @@ static int cbfs_create(void)
 			      &bootblock,
 			      param.baseaddress,
 			      param.headeroffset,
-			      param.offset) != 0) {
+			      param.cbfsoffset) != 0) {
 		ERROR("Failed to create %s.\n", param.cbfs_name);
 		return 1;
 	}
@@ -533,28 +552,29 @@ static const struct command commands[] = {
 };
 
 static struct option long_options[] = {
-	{"name",         required_argument, 0, 'n' },
-	{"type",         required_argument, 0, 't' },
-	{"compression",  required_argument, 0, 'c' },
-	{"base-address", required_argument, 0, 'b' },
-	{"load-address", required_argument, 0, 'l' },
-	{"top-aligned",  required_argument, 0, 'T' },
-	{"entry-point",  required_argument, 0, 'e' },
-	{"size",         required_argument, 0, 's' },
-	{"bootblock",    required_argument, 0, 'B' },
-	{"alignment",    required_argument, 0, 'a' },
-	{"page-size",    required_argument, 0, 'P' },
-	{"offset",       required_argument, 0, 'o' },
-	{"file",         required_argument, 0, 'f' },
-	{"int",          required_argument, 0, 'i' },
-	{"machine",      required_argument, 0, 'm' },
-	{"empty-fits",   required_argument, 0, 'x' },
-	{"initrd",       required_argument, 0, 'I' },
-	{"cmdline",      required_argument, 0, 'C' },
-	{"ignore-sec",   required_argument, 0, 'S' },
-	{"verbose",      no_argument,       0, 'v' },
-	{"help",         no_argument,       0, 'h' },
-	{NULL,           0,                 0,  0  }
+	{"name",          required_argument, 0, 'n' },
+	{"type",          required_argument, 0, 't' },
+	{"compression",   required_argument, 0, 'c' },
+	{"base-address",  required_argument, 0, 'b' },
+	{"load-address",  required_argument, 0, 'l' },
+	{"top-aligned",   required_argument, 0, 'T' },
+	{"entry-point",   required_argument, 0, 'e' },
+	{"size",          required_argument, 0, 's' },
+	{"bootblock",     required_argument, 0, 'B' },
+	{"header-offset", required_argument, 0, 'H' },
+	{"alignment",     required_argument, 0, 'a' },
+	{"page-size",     required_argument, 0, 'P' },
+	{"offset",        required_argument, 0, 'o' },
+	{"file",          required_argument, 0, 'f' },
+	{"int",           required_argument, 0, 'i' },
+	{"machine",       required_argument, 0, 'm' },
+	{"empty-fits",    required_argument, 0, 'x' },
+	{"initrd",        required_argument, 0, 'I' },
+	{"cmdline",       required_argument, 0, 'C' },
+	{"ignore-sec",    required_argument, 0, 'S' },
+	{"verbose",       no_argument,       0, 'v' },
+	{"help",          no_argument,       0, 'h' },
+	{NULL,            0,                 0,  0  }
 };
 
 static void usage(char *name)
@@ -582,7 +602,8 @@ static void usage(char *name)
 			"Add a raw 64-bit integer value\n"
 	     " remove -n NAME                                              "
 			"Remove a component\n"
-	     " create -s size -B bootblock -m ARCH [-a align] [-o offset]  "
+	     " create -s size -m ARCH [-B bootblock] [-b bootblock offset] \\\n"
+	     "        [-o CBFS offset] [-H header offset] [-a align]       "
 			"Create a ROM file\n"
 	     " locate -f FILE -n NAME [-P page-size] [-a align] [-T]       "
 			"Find a place for a file of that size\n"
@@ -694,7 +715,8 @@ int main(int argc, char **argv)
 				param.pagesize = strtoul(optarg, NULL, 0);
 				break;
 			case 'o':
-				param.offset = strtoul(optarg, NULL, 0);
+				param.cbfsoffset = strtoul(optarg, NULL, 0);
+				param.cbfsoffset_assigned = 1;
 				break;
 			case 'f':
 				param.filename = optarg;
diff --git a/util/cbfstool/common.h b/util/cbfstool/common.h
index 02a1076488..0284742cb4 100644
--- a/util/cbfstool/common.h
+++ b/util/cbfstool/common.h
@@ -47,6 +47,13 @@ extern int verbose;
 
 #define unused __attribute__((unused))
 
+static inline uint32_t align_up(uint32_t value, uint32_t align)
+{
+	if (value % align)
+		value += align - (value % align);
+	return value;
+}
+
 /* Buffer and file I/O */
 struct buffer {
 	char *name;