Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47823 )
Change subject: cbfstool: Move alignment/baseaddress handling into cbfs_add_component() ......................................................................
cbfstool: Move alignment/baseaddress handling into cbfs_add_component()
The --alignment flag is currently only handled by cbfstool add, but there seems little reason to not handle it for all file-adding commands (the help text actually mentions it for add-stage as well but it doesn't currently work there). This patch moves the related code (and the related baseaddress handling) into cbfs_add_component(). As a nice side effect this allows us to rearrange cbfs_add_component() such that we can conclusively determine whether we need a hash attribute before trying to align the file, allowing that code to correctly infer the final header size even when a hash attribute was implicitly added (for an image built with CBFS verification enabled).
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: Idc6d68b2c7f30e5d136433adb3aec5a87053f992 --- M util/cbfstool/cbfstool.c 1 file changed, 37 insertions(+), 47 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/23/47823/1
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index 878db22..68c2c22 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -301,7 +301,7 @@ return convert_to_from_absolute_top_aligned(region, offset); }
-static int do_cbfs_locate(int32_t *cbfs_addr, size_t metadata_size, +static int do_cbfs_locate(uint32_t *cbfs_addr, size_t metadata_size, size_t data_size) { if (!param.filename) { @@ -363,7 +363,7 @@ int32_t address = cbfs_locate_entry(&image, data_size, param.pagesize, param.alignment, metadata_size);
- if (address == -1) { + if (address < 0) { ERROR("'%s' can't fit in CBFS for page-size %#x, align %#x.\n", param.name, param.pagesize, param.alignment); return 1; @@ -616,11 +616,16 @@ static int cbfs_add_component(const char *filename, const char *name, uint32_t type, - uint32_t offset, uint32_t headeroffset, convert_buffer_t convert) { size_t len_align = 0; + uint32_t offset = param.baseaddress_assigned ? param.baseaddress : 0; + + if (param.alignment && param.baseaddress_assigned) { + ERROR("Cannot specify both alignment and base address\n"); + return 1; + }
if (!filename) { ERROR("You need to specify -f/--filename.\n"); @@ -652,22 +657,9 @@ return 1; }
- /* - * Check if Intel CPU topswap is specified this will require a - * second bootblock to be added. - */ - if (type == CBFS_TYPE_BOOTBLOCK && param.topswap_size) - if (add_topswap_bootblock(&buffer, &offset)) - return 1; - struct cbfs_file *header = cbfs_create_file_header(type, buffer.size, name);
- if (convert && convert(&buffer, &offset, header) != 0) { - ERROR("Failed to parse file '%s'.\n", filename); - goto error; - } - /* Bootblock and CBFS header should never have file hashes. When adding the bootblock it is important that we *don't* look up the metadata hash yet (before it is added) or we'll cache an outdated result. */ @@ -683,12 +675,31 @@ goto error; } } + }
- if (param.hash != VB2_HASH_INVALID && - cbfs_add_file_hash(header, &buffer, param.hash) == -1) { - ERROR("couldn't add hash for '%s'\n", name); + /* This needs to run after potentially updating param.hash above. */ + if (param.alignment) + if (do_cbfs_locate(&offset, 0, 0)) goto error; - } + + /* + * Check if Intel CPU topswap is specified this will require a + * second bootblock to be added. + */ + if (type == CBFS_TYPE_BOOTBLOCK && param.topswap_size) + if (add_topswap_bootblock(&buffer, &offset)) + goto error; + + if (convert && convert(&buffer, &offset, header) != 0) { + ERROR("Failed to parse file '%s'.\n", filename); + goto error; + } + + /* This needs to run after convert(). */ + if (param.hash != VB2_HASH_INVALID && + cbfs_add_file_hash(header, &buffer, param.hash) == -1) { + ERROR("couldn't add hash for '%s'\n", name); + goto error; }
if (param.autogen_attr) { @@ -700,7 +711,7 @@ CBFS_FILE_ATTR_TAG_POSITION, sizeof(struct cbfs_file_attr_position)); if (attrs == NULL) - return -1; + goto error; attrs->position = htonl(offset); } /* Add alignment attribute if used */ @@ -711,7 +722,7 @@ CBFS_FILE_ATTR_TAG_ALIGNMENT, sizeof(struct cbfs_file_attr_align)); if (attrs == NULL) - return -1; + goto error; attrs->alignment = htonl(param.alignment); } } @@ -722,7 +733,7 @@ CBFS_FILE_ATTR_TAG_IBB, sizeof(struct cbfs_file_attribute)); if (attrs == NULL) - return -1; + goto error; /* For Intel TXT minimum align is 16 */ len_align = 16; } @@ -736,7 +747,7 @@ header, CBFS_FILE_ATTR_TAG_PADDING, size); if (attr == NULL) - return -1; + goto error; }
if (IS_TOP_ALIGNED_ADDRESS(offset)) @@ -892,7 +903,7 @@ return -1;
if (param.stage_xip) { - int32_t address; + uint32_t address; size_t data_size;
if (elf_program_file_size(buffer, &data_size) < 0) { @@ -1027,16 +1038,7 @@
static int cbfs_add(void) { - int32_t address; - convert_buffer_t convert; - uint32_t local_baseaddress = param.baseaddress; - - if (param.alignment && param.baseaddress) { - ERROR("Cannot specify both alignment and base address\n"); - return 1; - } - - convert = cbfstool_convert_raw; + convert_buffer_t convert = cbfstool_convert_raw;
/* Set the alignment to 4KiB minimum for FSP blobs when no base address * is provided so that relocation can occur. */ @@ -1049,18 +1051,9 @@ return 1; }
- if (param.alignment) { - /* CBFS compression file attribute is unconditionally added. */ - size_t metadata_sz = sizeof(struct cbfs_file_attr_compression); - if (do_cbfs_locate(&address, metadata_sz, 0)) - return 1; - local_baseaddress = address; - } - return cbfs_add_component(param.filename, param.name, param.type, - local_baseaddress, param.headeroffset, convert); } @@ -1082,7 +1075,6 @@ return cbfs_add_component(param.filename, param.name, CBFS_TYPE_STAGE, - param.baseaddress, param.headeroffset, cbfstool_convert_mkstage); } @@ -1092,7 +1084,6 @@ return cbfs_add_component(param.filename, param.name, CBFS_TYPE_SELF, - param.baseaddress, param.headeroffset, cbfstool_convert_mkpayload); } @@ -1112,7 +1103,6 @@ return cbfs_add_component(param.filename, param.name, CBFS_TYPE_SELF, - param.baseaddress, param.headeroffset, cbfstool_convert_mkflatpayload); }