cbfstool: Adapt "cbfstool copy" to only use fmap regions.

These need to go together, so the commit became a bit larger than
typial.

- Add an option -R for the copy source fmap region.
  Use: cbfstool copy -r target-region -R source-region.
- Don't generate a CBFS master header because for fmap regions, we
  assume that the region starts with a file header.
  Use cbfstool add-master-header to add it afterwards, if necessary.
- Don't copy files of type "cbfs master header" (which are what cbfstool
  add-master-header creates)
- Leave room for the master header pointer
- Remove -D command line option as it's no longer used.

BUG=chromium:445938
BRANCH=none
TEST=Manual test on image and integration test w/ bundle_firmware
     changes.
CQ-DEPEND=CL:313770,CL:313771

Change-Id: I2a11cda42caee96aa763f162b5f3bc11bb7992f9
Signed-off-by: Patrick Georgi <pgeorgi@google.com>
Signed-off-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-on: https://review.coreboot.org/12788
Tested-by: build bot (Jenkins)

2 files changed, 29 insertions, 52 deletions

diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index 25aafe941d..3820c403de 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -390,31 +390,16 @@ int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in,
 int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst)
 {
 	assert(image);
-	if (!cbfs_is_legacy_cbfs(image))
-		return -1;
 
 	struct cbfs_file *src_entry, *dst_entry;
-	struct cbfs_header *copy_header;
-	size_t align, entry_offset;
+	size_t align;
 	ssize_t last_entry_size;
 
 	size_t copy_end = buffer_size(dst);
 
-	align = image->header.align;
-
-	/* This will work, let's create a copy. */
-	copy_header = (struct cbfs_header *)buffer_get(dst);
-	cbfs_put_header(copy_header, &image->header);
+	align = CBFS_ENTRY_ALIGNMENT;
 
-	copy_header->bootblocksize = 0;
-	/* FIXME: romsize and offset have a full-flash interpretation even
-	 * though they don't need to and should be region-relative (where
-	 * region is sufficiently specified by the master header pointer.
-	 * But that's for a later change. */
-	copy_header->romsize = htonl(dst->offset + buffer_size(dst));
-	entry_offset = align_up(sizeof(*copy_header), align);
-	copy_header->offset = htonl(dst->offset + entry_offset);
-	dst_entry = (struct cbfs_file *)(buffer_get(dst) + entry_offset);
+	dst_entry = (struct cbfs_file *)buffer_get(dst);
 
 	/* Copy non-empty files */
 	for (src_entry = cbfs_find_first_entry(image);
@@ -423,6 +408,7 @@ int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst)
 		size_t entry_size;
 
 		if ((src_entry->type == htonl(CBFS_COMPONENT_NULL)) ||
+		    (src_entry->type == htonl(CBFS_COMPONENT_CBFSHEADER)) ||
 		    (src_entry->type == htonl(CBFS_COMPONENT_DELETED)))
 			continue;
 
@@ -438,9 +424,12 @@ int cbfs_copy_instance(struct cbfs_image *image, struct buffer *dst)
 		}
 	}
 
-	/* Last entry size is all the room above it. */
+	/* Last entry size is all the room above it, except for top 4 bytes
+	 * which may be used by the master header pointer. This messes with
+	 * the ability to stash something "top-aligned" into the region, but
+	 * keeps things simpler. */
 	last_entry_size = copy_end - ((void *)dst_entry - buffer_get(dst))
-		- cbfs_calculate_file_header_size("");
+		- cbfs_calculate_file_header_size("") - sizeof(int32_t);
 
 	if (last_entry_size < 0)
 		WARN("No room to create the last entry!\n")
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index c9a9564e94..f00aa30183 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -49,6 +49,7 @@ static struct param {
 	const char *filename;
 	const char *fmap;
 	const char *region_name;
+	const char *source_region;
 	const char *bootblock;
 	const char *ignore_section;
 	uint64_t u64val;
@@ -56,8 +57,6 @@ static struct param {
 	uint32_t baseaddress;
 	uint32_t baseaddress_assigned;
 	uint32_t loadaddress;
-	uint32_t copyoffset;
-	uint32_t copyoffset_assigned;
 	uint32_t headeroffset;
 	uint32_t headeroffset_assigned;
 	uint32_t entrypoint;
@@ -917,34 +916,25 @@ static int cbfs_update_fit(void)
 
 static int cbfs_copy(void)
 {
-	/* always a valid source region */
 	struct cbfs_image src_image;
+	struct buffer src_buf;
 
-	if (cbfs_image_from_buffer(&src_image, param.image_region,
-							param.headeroffset))
+	if (!param.source_region) {
+		ERROR("You need to specify -R/--source-region.\n");
 		return 1;
+	}
 
-	struct buffer dst_buffer;
-
-	if (cbfs_is_legacy_cbfs(&src_image)) {
-		if (!param.copyoffset_assigned) {
-			ERROR("You need to specify -D/--copy-offset.\n");
-			return 1;
-		}
-
-		if (!param.size) {
-			ERROR("You need to specify -s/--size.\n");
-			return 1;
-		}
-
-		buffer_splice(&dst_buffer, param.image_region,
-			param.copyoffset, param.size);
-	} else {
-		ERROR("This operation is only valid on legacy images having CBFS master headers\n");
+	/* Obtain the source region and convert it to a cbfs_image. */
+	if (!partitioned_file_read_region(&src_buf, param.image_file,
+						param.source_region)) {
+		ERROR("Region not found in image: %s\n", param.source_region);
 		return 1;
 	}
 
-	return cbfs_copy_instance(&src_image, &dst_buffer);
+	if (cbfs_image_from_buffer(&src_image, &src_buf, param.headeroffset))
+		return 1;
+
+	return cbfs_copy_instance(&src_image, param.image_region);
 }
 
 static const struct command commands[] = {
@@ -955,7 +945,7 @@ static const struct command commands[] = {
 	{"add-stage", "a:H:r:f:n:t:c:b:P:S:yvA:h?", cbfs_add_stage, true, true},
 	{"add-int", "H:r:i:n:b:vh?", cbfs_add_integer, true, true},
 	{"add-master-header", "H:r:vh?", cbfs_add_master_header, true, true},
-	{"copy", "H:D:s:h?", cbfs_copy, true, true},
+	{"copy", "r:R:h?", cbfs_copy, true, true},
 	{"create", "M:r:s:B:b:H:o:m:vh?", cbfs_create, true, true},
 	{"extract", "H:r:m:n:f:vh?", cbfs_extract, true, false},
 	{"layout", "wvh?", cbfs_layout, false, false},
@@ -972,7 +962,6 @@ static struct option long_options[] = {
 	{"bootblock",     required_argument, 0, 'B' },
 	{"cmdline",       required_argument, 0, 'C' },
 	{"compression",   required_argument, 0, 'c' },
-	{"copy-offset",   required_argument, 0, 'D' },
 	{"empty-fits",    required_argument, 0, 'x' },
 	{"entry-point",   required_argument, 0, 'e' },
 	{"file",          required_argument, 0, 'f' },
@@ -980,6 +969,7 @@ static struct option long_options[] = {
 	{"fill-upward",   no_argument,       0, 'u' },
 	{"flashmap",      required_argument, 0, 'M' },
 	{"fmap-regions",  required_argument, 0, 'r' },
+	{"source-region", required_argument, 0, 'R' },
 	{"hash-algorithm",required_argument, 0, 'A' },
 	{"header-offset", required_argument, 0, 'H' },
 	{"help",          no_argument,       0, 'h' },
@@ -1085,9 +1075,8 @@ static void usage(char *name)
 			"Add a legacy CBFS master header\n"
 	     " remove [-r image,regions] -n NAME                           "
 			"Remove a component\n"
-	     " copy -D new_header_offset -s region size \\\n"
-	     "        [-H source header offset]                            "
-			"Create a copy (duplicate) cbfs instance*\n"
+	     " copy -r image,regions -R source-region                      "
+			"Create a copy (duplicate) cbfs instance in fmap\n"
 	     " create -m ARCH -s size [-b bootblock offset] \\\n"
 	     "        [-o CBFS offset] [-H header offset] [-B bootblock]   "
 			"Create a legacy ROM file with CBFS master header*\n"
@@ -1216,6 +1205,9 @@ int main(int argc, char **argv)
 			case 'r':
 				param.region_name = optarg;
 				break;
+			case 'R':
+				param.source_region = optarg;
+				break;
 			case 'b':
 				param.baseaddress = strtoul(optarg, NULL, 0);
 				// baseaddress may be zero on non-x86, so we
@@ -1246,10 +1238,6 @@ int main(int argc, char **argv)
 						optarg, NULL, 0);
 				param.headeroffset_assigned = 1;
 				break;
-			case 'D':
-				param.copyoffset = strtoul(optarg, NULL, 0);
-				param.copyoffset_assigned = 1;
-				break;
 			case 'a':
 				param.alignment = strtoul(optarg, NULL, 0);
 				break;