[coreboot-gerrit] New patch to review for coreboot: cbfstool: provide metadata size to cbfs_locate_entry()
Aaron Durbin (adurbin@chromium.org)
gerrit at coreboot.org
Wed Sep 16 01:21:38 CET 2015
Aaron Durbin (adurbin at chromium.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/11668
-gerrit
commit a2ef767a6438f683884873401bc3c17d1be69def
Author: Aaron Durbin <adurbin at chromium.org>
Date: Tue Sep 15 12:50:14 2015 -0500
cbfstool: provide metadata size to cbfs_locate_entry()
The cbfs_locate_entry() function had a hack in there which
assumed a struct cbfs_stage data was being added in addition
to the struct cbfs_file and name. Move that logic out to the
callers while still maintaining the logic for consistency.
The only impacted commands cbfs_add and cbfs_locate, but
those are using the default 'always adding struct cbfs_stage'
in addition to cbfs_file + name. Eventually those should be
removed when cbfs_locate is removed as cbfs_add has no smarts
related to the cbfs file type provided.
BUG=chrome-os-partner:44827
BRANCH=None
TEST=Built rambi.
Change-Id: I2771116ea1ff439ea53b8886e1f33e0e637a79d4
Signed-off-by: Aaron Durbin <adurbin at chromium.org>
---
util/cbfstool/cbfs_image.c | 41 +++++++++++++++++------------------------
util/cbfstool/cbfs_image.h | 4 ++--
util/cbfstool/cbfstool.c | 23 ++++++++++++++++++-----
3 files changed, 37 insertions(+), 31 deletions(-)
diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index c2f0b37..55f8084 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -1136,20 +1136,20 @@ static int is_in_same_page(uint32_t start, uint32_t size, uint32_t page)
}
/* Tests if data can fit in a range by given offset:
- * start ->| header_len | offset (+ size) |<- end
+ * start ->| metadata_size | offset (+ size) |<- end
*/
-static int is_in_range(uint32_t start, uint32_t end, uint32_t header_len,
- uint32_t offset, uint32_t size)
+static int is_in_range(size_t start, size_t end, size_t metadata_size,
+ size_t offset, size_t size)
{
- return (offset >= start + header_len && offset + size <= end);
+ return (offset >= start + metadata_size && offset + size <= end);
}
-int32_t cbfs_locate_entry(struct cbfs_image *image, const char *name,
- uint32_t size, uint32_t page_size, uint32_t align)
+int32_t cbfs_locate_entry(struct cbfs_image *image, size_t size,
+ size_t page_size, size_t align, size_t metadata_size)
{
struct cbfs_file *entry;
size_t need_len;
- uint32_t addr, addr_next, addr2, addr3, offset, header_len;
+ size_t addr, addr_next, addr2, addr3, offset;
/* Default values: allow fitting anywhere in ROM. */
if (!page_size)
@@ -1159,23 +1159,16 @@ int32_t cbfs_locate_entry(struct cbfs_image *image, const char *name,
align = 1;
if (size > page_size)
- ERROR("Input file size (%d) greater than page size (%d).\n",
+ ERROR("Input file size (%zd) greater than page size (%zd).\n",
size, page_size);
- uint32_t image_align = image->has_header ? image->header.align :
+ size_t image_align = image->has_header ? image->header.align :
CBFS_ENTRY_ALIGNMENT;
if (page_size % image_align)
- WARN("%s: Page size (%#x) not aligned with CBFS image (%#x).\n",
+ WARN("%s: Page size (%#zx) not aligned with CBFS image (%#zx).\n",
__func__, page_size, image_align);
- /* TODO Old cbfstool always assume input is a stage file (and adding
- * sizeof(cbfs_stage) for header. We should fix that by adding "-t"
- * (type) param in future. For right now, we assume cbfs_stage is the
- * largest structure and add it into header size. */
- assert(sizeof(struct cbfs_stage) >= sizeof(struct cbfs_payload));
- header_len = (cbfs_calculate_file_header_size(name) +
- sizeof(struct cbfs_stage));
- need_len = header_len + size;
+ need_len = metadata_size + size;
// Merge empty entries to build get max available space.
cbfs_walk(image, cbfs_merge_empty_entry, NULL);
@@ -1215,26 +1208,26 @@ int32_t cbfs_locate_entry(struct cbfs_image *image, const char *name,
if (addr_next - addr < need_len)
continue;
- offset = align_up(addr + header_len, align);
+ offset = align_up(addr + metadata_size, align);
if (is_in_same_page(offset, size, page_size) &&
- is_in_range(addr, addr_next, header_len, offset, size)) {
+ is_in_range(addr, addr_next, metadata_size, offset, size)) {
DEBUG("cbfs_locate_entry: FIT (PAGE1).");
return offset;
}
addr2 = align_up(addr, page_size);
offset = align_up(addr2, align);
- if (is_in_range(addr, addr_next, header_len, offset, size)) {
+ if (is_in_range(addr, addr_next, metadata_size, offset, size)) {
DEBUG("cbfs_locate_entry: OVERLAP (PAGE2).");
return offset;
}
- /* Assume page_size >= header_len so adding one page will
+ /* Assume page_size >= metadata_size so adding one page will
* definitely provide the space for header. */
- assert(page_size >= header_len);
+ assert(page_size >= metadata_size);
addr3 = addr2 + page_size;
offset = align_up(addr3, align);
- if (is_in_range(addr, addr_next, header_len, offset, size)) {
+ if (is_in_range(addr, addr_next, metadata_size, offset, size)) {
DEBUG("cbfs_locate_entry: OVERLAP+ (PAGE3).");
return offset;
}
diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h
index aea8260..baceb51 100644
--- a/util/cbfstool/cbfs_image.h
+++ b/util/cbfstool/cbfs_image.h
@@ -112,8 +112,8 @@ int cbfs_create_empty_entry(struct cbfs_file *entry, int type,
* "page_size" limits the content to fit on same memory page, and
* "align" specifies starting address alignment.
* Returns a valid offset, or -1 on failure. */
-int32_t cbfs_locate_entry(struct cbfs_image *image, const char *name,
- uint32_t size, uint32_t page_size, uint32_t align);
+int32_t cbfs_locate_entry(struct cbfs_image *image, size_t size,
+ size_t page_size, size_t align, size_t metadata_size);
/* Callback function used by cbfs_walk.
* Returns 0 on success, or non-zero to stop further iteration. */
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index d6c116a..182fc8e 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -328,7 +328,7 @@ static int cbfstool_convert_mkflatpayload(struct buffer *buffer,
return 0;
}
-static int do_cbfs_locate(int32_t *cbfs_addr)
+static int do_cbfs_locate(int32_t *cbfs_addr, size_t metadata_size)
{
if (!param.filename) {
ERROR("You need to specify -f/--filename.\n");
@@ -354,8 +354,11 @@ static int do_cbfs_locate(int32_t *cbfs_addr)
return 1;
}
- int32_t address = cbfs_locate_entry(&image, param.name, buffer.size,
- param.pagesize, param.alignment);
+ /* Include cbfs_file size along with space for with name. */
+ metadata_size += cbfs_calculate_file_header_size(param.name);
+
+ int32_t address = cbfs_locate_entry(&image, buffer.size, param.pagesize,
+ param.alignment, metadata_size);
buffer_delete(&buffer);
if (address == -1) {
@@ -372,6 +375,16 @@ static int do_cbfs_locate(int32_t *cbfs_addr)
return 0;
}
+static size_t cbfs_default_metadata_size(void)
+{
+ /* TODO Old cbfstool always assume input is a stage file (and adding
+ * sizeof(cbfs_stage) for header. We should fix that by adding "-t"
+ * (type) param in future. For right now, we assume cbfs_stage is the
+ * largest structure and add it into header size. */
+ assert(sizeof(struct cbfs_stage) >= sizeof(struct cbfs_payload));
+ return sizeof(struct cbfs_stage);
+}
+
static int cbfs_add(void)
{
int32_t address;
@@ -382,7 +395,7 @@ static int cbfs_add(void)
}
if (param.alignment) {
- if (do_cbfs_locate(&address))
+ if (do_cbfs_locate(&address, cbfs_default_metadata_size()))
return 1;
param.baseaddress = address;
}
@@ -553,7 +566,7 @@ static int cbfs_locate(void)
{
int32_t address;
- if (do_cbfs_locate(&address) != 0)
+ if (do_cbfs_locate(&address, cbfs_default_metadata_size()) != 0)
return 1;
printf("0x%x\n", address);
More information about the coreboot-gerrit
mailing list