[coreboot-gerrit] Patch set updated for coreboot: fc9b5bb cbfstool: Support top-aligned addresses for new-format images

Sol Boucher (solb@chromium.org) gerrit at coreboot.org
Fri May 8 04:43:29 CEST 2015


Sol Boucher (solb at chromium.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/10136

-gerrit

commit fc9b5bb82c493fe7116ec416df141bacfd67adce
Author: Sol Boucher <solb at chromium.org>
Date:   Thu May 7 02:39:22 2015 -0700

    cbfstool: Support top-aligned addresses for new-format images
    
    The cbfstool handling of new-style FMAP-driven "partitioned" images
    originally disallowed the use of x86-style top-aligned addresses with
    the add.* and layout actions because it wasn't obvious how they should
    work, especially since the normal addressing is done relative to each
    individual region for these types of images. Not surprisingly,
    however, the x86 portions of the build system make copious use of
    top-aligned addresses, so this allows their use with new images and
    specifies their behavior as being relative to the *image* end---not
    the region end---just as it is for legacy images.
    
    Change-Id: Icecc843f4f8b6bb52aa0ea16df771faa278228d2
    Signed-off-by: Sol Boucher <solb at chromium.org>
---
 util/cbfstool/cbfs_image.c       | 19 ++++++------------
 util/cbfstool/cbfs_image.h       |  4 +---
 util/cbfstool/cbfstool.c         | 43 ++++++++++++++++++++++++++++++----------
 util/cbfstool/common.h           |  2 ++
 util/cbfstool/partitioned_file.c |  7 +++++++
 util/cbfstool/partitioned_file.h |  3 +++
 6 files changed, 52 insertions(+), 26 deletions(-)

diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index 4ecb461..8fb2a60 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -526,6 +526,12 @@ static int cbfs_add_entry_at(struct cbfs_image *image,
 int cbfs_add_entry(struct cbfs_image *image, struct buffer *buffer,
 		   const char *name, uint32_t type, uint32_t content_offset)
 {
+	assert(image);
+	assert(buffer);
+	assert(buffer->data);
+	assert(name);
+	assert(!IS_TOP_ALIGNED_ADDRESS(content_offset));
+
 	uint32_t entry_type;
 	uint32_t addr, addr_next;
 	struct cbfs_file *entry, *next;
@@ -537,19 +543,6 @@ int cbfs_add_entry(struct cbfs_image *image, struct buffer *buffer,
 	DEBUG("cbfs_add_entry('%s'@0x%x) => need_size = %u+%zu=%u\n",
 	      name, content_offset, header_size, buffer->size, need_size);
 
-	if (IS_TOP_ALIGNED_ADDRESS(content_offset)) {
-		if (!cbfs_is_legacy_cbfs(image)) {
-			ERROR("Top-aligned offsets are only supported for legacy CBFSes (with master headers)\n");
-			return -1;
-		}
-
-		// legacy cbfstool takes top-aligned address.
-		uint32_t theromsize = image->header.romsize;
-		INFO("Converting top-aligned address 0x%x to offset: 0x%x\n",
-		     content_offset, content_offset + theromsize);
-		content_offset = theromsize + (int32_t)content_offset;
-	}
-
 	// Merge empty entries.
 	DEBUG("(trying to merge empty entries...)\n");
 	cbfs_walk(image, cbfs_merge_empty_entry, NULL);
diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h
index 024edc5..5ff8a9a 100644
--- a/util/cbfstool/cbfs_image.h
+++ b/util/cbfstool/cbfs_image.h
@@ -22,8 +22,6 @@
 #include "common.h"
 #include "cbfs.h"
 
-#define IS_TOP_ALIGNED_ADDRESS(x)	((uint32_t)(x) > 0x80000000)
-
 /* CBFS image processing */
 
 struct cbfs_image {
@@ -88,7 +86,7 @@ int cbfs_export_entry(struct cbfs_image *image, const char *entry_name,
 
 /* Adds an entry to CBFS image by given name and type. If content_offset is
  * non-zero, try to align "content" (CBFS_SUBHEADER(p)) at content_offset.
- * Note that top-aligned addresses are only supported for legacy CBFSes.
+ * Never pass this function a top-aligned address: convert it to an offset.
  * Returns 0 on success, otherwise non-zero. */
 int cbfs_add_entry(struct cbfs_image *image, struct buffer *buffer,
 		   const char *name, uint32_t type, uint32_t content_offset);
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index 29ac6b6..ea3c339 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -98,6 +98,21 @@ static bool region_is_modern_cbfs(const char *region)
 				CBFS_FILE_MAGIC, strlen(CBFS_FILE_MAGIC));
 }
 
+/*
+ * Converts between offsets from the start of the specified image region and
+ * "top-aligned" offsets from the top of the entire flash image. Works in either
+ * direction: pass in one type of offset and receive the other type.
+ * N.B. A top-aligned offset is always a positive number, and should not be
+ * confused with a top-aliged *address*, which is its arithmetic inverse. */
+static unsigned convert_to_from_top_aligned(const struct buffer *region,
+								unsigned offset)
+{
+	assert(region);
+
+	size_t image_size = partitioned_file_total_size(param.image_file);
+	return image_size - region->offset - offset;
+}
+
 typedef int (*convert_buffer_t)(struct buffer *buffer, uint32_t *offset);
 
 static int cbfs_add_integer_component(const char *name,
@@ -129,6 +144,10 @@ static int cbfs_add_integer_component(const char *name,
 		goto done;
 	}
 
+	if (IS_TOP_ALIGNED_ADDRESS(offset))
+		offset = convert_to_from_top_aligned(param.image_region,
+								-offset);
+
 	if (cbfs_add_entry(&image, &buffer, name, CBFS_COMPONENT_RAW, offset) !=
 									0) {
 		ERROR("Failed to add %llu into ROM image as '%s'.\n",
@@ -187,6 +206,10 @@ static int cbfs_add_component(const char *filename,
 		return 1;
 	}
 
+	if (IS_TOP_ALIGNED_ADDRESS(offset))
+		offset = convert_to_from_top_aligned(param.image_region,
+								-offset);
+
 	if (cbfs_add_entry(&image, &buffer, name, type, offset) != 0) {
 		ERROR("Failed to add '%s' into ROM image.\n", filename);
 		buffer_delete(&buffer);
@@ -439,11 +462,6 @@ static int cbfs_locate(void)
 							param.headeroffset))
 		return 1;
 
-	if (!cbfs_is_legacy_cbfs(&image) && param.top_aligned) {
-		ERROR("The -T switch is only valid on legacy images having CBFS master headers\n");
-		return 1;
-	}
-
 	if (cbfs_get_entry(&image, param.name))
 		WARN("'%s' already in CBFS.\n", param.name);
 
@@ -464,7 +482,8 @@ static int cbfs_locate(void)
 	}
 
 	if (param.top_aligned)
-		address = address - image.header.romsize;
+		address = -convert_to_from_top_aligned(param.image_region,
+								address);
 
 	printf("0x%x\n", address);
 	return 0;
@@ -806,7 +825,7 @@ static void usage(char *name)
 	     "USAGE:\n" " %s [-h]\n"
 	     " %s FILE COMMAND [-v] [PARAMETERS]...\n\n" "OPTIONs:\n"
 	     "  -H header_offset Do not search for header; use this offset*\n"
-	     "  -T               Output top-aligned memory address*\n"
+	     "  -T               Output top-aligned memory address\n"
 	     "  -u               Accept short data; fill upward/from bottom\n"
 	     "  -d               Accept short data; fill downward/from top\n"
 	     "  -v               Provide verbose output\n"
@@ -857,8 +876,8 @@ static void usage(char *name)
 			"Updates the FIT table with microcode entries\n"
 	     "\n"
 	     "OFFSETs:\n"
-	     "  Numbers accompanying -b, -H, and -o switches may be provided\n"
-	     "  in two possible formats*: if their value is greater than\n"
+	     "  Numbers accompanying -b, -H, and -o switches* may be provided\n"
+	     "  in two possible formats: if their value is greater than\n"
 	     "  0x80000000, they are interpreted as a top-aligned x86 memory\n"
 	     "  address; otherwise, they are treated as an offset into flash.\n"
 	     "ARCHes:\n"
@@ -878,7 +897,11 @@ static void usage(char *name)
 	     "  that, when working with such images, the -F and -r switches\n"
 	     "  default to '%s' for convenience, and both the -b switch to\n"
 	     "  CBFS operations and the output of the locate action become\n"
-	     "  relative to the selected CBFS region's lowest address.\n",
+	     "  relative to the selected CBFS region's lowest address.\n"
+	     "  The one exception to this rule is the top-aligned address,\n"
+	     "  which is always relative to the end of the entire image\n"
+	     "  rather than relative to the local region; this is true for\n"
+	     "  for both input (sufficiently large) and output (-T) data.\n",
 	     SECTION_NAME_FMAP, SECTION_NAME_PRIMARY_CBFS,
 	     SECTION_NAME_PRIMARY_CBFS
 	     );
diff --git a/util/cbfstool/common.h b/util/cbfstool/common.h
index 831d3eb..392ec80 100644
--- a/util/cbfstool/common.h
+++ b/util/cbfstool/common.h
@@ -50,6 +50,8 @@ extern int verbose;
 #define MAX(x, y) ((x) > (y) ? (x) : (y))
 #define MIN(x, y) ((x) < (y) ? (x) : (y))
 
+#define IS_TOP_ALIGNED_ADDRESS(x)	((uint32_t)(x) > 0x80000000)
+
 #define unused __attribute__((unused))
 
 static inline uint32_t align_up(uint32_t value, uint32_t align)
diff --git a/util/cbfstool/partitioned_file.c b/util/cbfstool/partitioned_file.c
index 6473963..0b25272 100644
--- a/util/cbfstool/partitioned_file.c
+++ b/util/cbfstool/partitioned_file.c
@@ -290,6 +290,13 @@ bool partitioned_file_is_partitioned(const partitioned_file_t *file)
 	return partitioned_file_get_fmap(file) != NULL;
 }
 
+size_t partitioned_file_total_size(const partitioned_file_t *file)
+{
+	assert(file);
+
+	return file->buffer.size;
+}
+
 bool partitioned_file_region_check_magic(const partitioned_file_t *file,
 			const char *region, const char *magic, size_t magic_len)
 {
diff --git a/util/cbfstool/partitioned_file.h b/util/cbfstool/partitioned_file.h
index 97d1b57..92f228e 100644
--- a/util/cbfstool/partitioned_file.h
+++ b/util/cbfstool/partitioned_file.h
@@ -133,6 +133,9 @@ void partitioned_file_close(partitioned_file_t *file);
 /** @return Whether the file is partitioned (i.e. not flat). */
 bool partitioned_file_is_partitioned(const partitioned_file_t *file);
 
+/** @return The image's overall filesize, regardless of whether it's flat. */
+size_t partitioned_file_total_size(const partitioned_file_t *file);
+
 /** @return Whether the specified region begins with the magic bytes. */
 bool partitioned_file_region_check_magic(const partitioned_file_t *file,
 		const char *region, const char *magic, size_t magic_len);



More information about the coreboot-gerrit mailing list