From d7339411a983e562cddeba676384ad836bb56ec9 Mon Sep 17 00:00:00 2001 From: Aaron Durbin Date: Tue, 15 Sep 2015 12:50:14 -0500 Subject: 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 Reviewed-on: http://review.coreboot.org/11668 Tested-by: build bot (Jenkins) Reviewed-by: Patrick Georgi --- util/cbfstool/cbfs_image.c | 41 +++++++++++++++++------------------------ util/cbfstool/cbfs_image.h | 4 ++-- util/cbfstool/cbfstool.c | 23 ++++++++++++++++++----- 3 files changed, 37 insertions(+), 31 deletions(-) (limited to 'util') diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index c2f0b37c53..55f80844db 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 aea826053a..baceb51373 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 d6c116af15..182fc8ebe5 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); -- cgit v1.2.3