Attention is currently required from: Chen, Gang C, Jincheng Li.
Hello Chen, Gang C, Jincheng Li,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/86569?usp=email
to review the following change.
Change subject: util/cbfstool: Optionally place new components as high as possible ......................................................................
util/cbfstool: Optionally place new components as high as possible
cbfstool by default places new components as low as possible. Add option -z to place them as high as possible. This useful to put files into cached region when only the highest portion close to 4G boundary is cached.
Change-Id: Ida3131694f3e8883424351a68bba6a8da612ae09 Signed-off-by: Shuo Liu shuo.liu@intel.com Signed-off-by: Gang Chen gang.c.chen@intel.com Signed-off-by: Jincheng Li jincheng.li@intel.com --- M util/cbfstool/cbfs_image.c M util/cbfstool/cbfs_image.h M util/cbfstool/cbfstool.c M util/cbfstool/common.h 4 files changed, 45 insertions(+), 11 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/86569/1
diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 3d768b0..cdcf8fa 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -1948,21 +1948,24 @@ }
static size_t absolute_align(const struct cbfs_image *image, size_t val, - size_t align) + size_t align, bool up) { const size_t region_offset = buffer_offset(&image->buffer); /* To perform alignment on absolute address, take the region offset */ /* of the image into account. */ - return align_up(val + region_offset, align) - region_offset; + size_t aligned = up ? align_up(val + region_offset, align) : + align_down(val + region_offset, align); + return aligned - region_offset;
}
int32_t cbfs_locate_entry(struct cbfs_image *image, size_t size, - size_t page_size, size_t align, size_t metadata_size) + size_t page_size, size_t align, size_t metadata_size, + bool close_to_top) { struct cbfs_file *entry; size_t need_len; - size_t addr, addr_next, addr2, addr3, offset; + size_t addr, addr_next, addr2, addr3, offset, offset2;
/* Default values: allow fitting anywhere in ROM. */ if (!page_size) @@ -2007,6 +2010,7 @@ * For stage targets, the address is also used to re-link stage before * being added into CBFS. */ + offset = (uint32_t)-1; for (entry = cbfs_find_first_entry(image); entry && cbfs_is_valid_entry(image, entry); entry = cbfs_find_next_entry(image, entry)) { @@ -2021,7 +2025,20 @@ if (addr_next - addr < need_len) continue;
- offset = absolute_align(image, addr + metadata_size, align); + if (close_to_top) { + offset2 = absolute_align(image, addr_next - size, align, false); + if (is_in_same_page(offset2, size, page_size) && + is_in_range(addr, addr_next, metadata_size, offset2, size)) { + offset = offset2; + } + /* + * By now, extra page alignment is not needed. + * TODO: add page alignment based on needs. + */ + continue; + } + + offset = absolute_align(image, addr + metadata_size, align, true); if (is_in_same_page(offset, size, page_size) && is_in_range(addr, addr_next, metadata_size, offset, size)) { DEBUG("cbfs_locate_entry: FIT (PAGE1).\n"); @@ -2029,7 +2046,7 @@ }
addr2 = align_up(addr, page_size); - offset = absolute_align(image, addr2, align); + offset = absolute_align(image, addr2, align, true); if (is_in_range(addr, addr_next, metadata_size, offset, size)) { DEBUG("cbfs_locate_entry: OVERLAP (PAGE2).\n"); return offset; @@ -2039,11 +2056,15 @@ * definitely provide the space for header. */ assert(page_size >= metadata_size); addr3 = addr2 + page_size; - offset = absolute_align(image, addr3, align); + offset = absolute_align(image, addr3, align, true); if (is_in_range(addr, addr_next, metadata_size, offset, size)) { DEBUG("cbfs_locate_entry: OVERLAP+ (PAGE3).\n"); return offset; } } + + if (close_to_top && (offset != (uint32_t)-1)) + return offset; + return -1; } diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h index 85ba023..8f00d8a 100644 --- a/util/cbfstool/cbfs_image.h +++ b/util/cbfstool/cbfs_image.h @@ -118,7 +118,8 @@ * "align" specifies starting address alignment. * Returns a valid offset, or -1 on failure. */ int32_t cbfs_locate_entry(struct cbfs_image *image, size_t size, - size_t page_size, size_t align, size_t metadata_size); + size_t page_size, size_t align, size_t metadata_size, + bool close_to_top);
/* Callback function used by cbfs_legacy_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 9acfd26..bde61b5 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -108,6 +108,7 @@ */ uint32_t ext_win_base; uint32_t ext_win_size; + bool close_to_top; } param = { /* All variables not listed are initialized as zero. */ .arch = CBFS_ARCHITECTURE_UNKNOWN, @@ -116,6 +117,7 @@ .headeroffset = HEADER_OFFSET_UNKNOWN, .region_name = SECTION_NAME_PRIMARY_CBFS, .u64val = -1, + .close_to_top = false, };
/* @@ -612,7 +614,7 @@ metadata_size += cbfs_file_attr_hash_size(param.hash);
int32_t address = cbfs_locate_entry(&image, data_size, param.pagesize, - param.alignment, metadata_size); + param.alignment, metadata_size, param.close_to_top);
if (address < 0) { ERROR("'%s'(%u + %zu) can't fit in CBFS for page-size %#x, align %#x.\n", @@ -1801,12 +1803,12 @@ }
static const struct command commands[] = { - {"add", "H:r:f:n:t:c:b:a:p:yvA:j:gh?", cbfs_add, true, true}, + {"add", "H:r:f:n:t:c:b:a:p:yvA:j:gzh?", cbfs_add, true, true}, {"add-flat-binary", "H:r:f:n:l:e:c:b:p:vA:gh?", cbfs_add_flat_binary, true, true}, {"add-payload", "H:r:f:n:c:b:a:C:I:p:vA:gh?", cbfs_add_payload, true, true}, - {"add-stage", "a:H:r:f:n:t:c:b:P:QS:p:yvA:gh?", cbfs_add_stage, + {"add-stage", "a:H:r:f:n:t:c:b:P:QS:p:yvA:gzh?", cbfs_add_stage, true, true}, {"add-int", "H:r:i:n:b:vgh?", cbfs_add_integer, true, true}, {"add-master-header", "H:r:vh?j:", cbfs_add_master_header, true, true}, @@ -2304,6 +2306,9 @@ case 'U': param.unprocessed = true; break; + case 'z': + param.close_to_top = true; + break; case LONGOPT_IBB: param.ibb = true; break; diff --git a/util/cbfstool/common.h b/util/cbfstool/common.h index 88d5238..50e4ceb 100644 --- a/util/cbfstool/common.h +++ b/util/cbfstool/common.h @@ -30,6 +30,13 @@ return value; }
+static inline uint32_t align_down(uint32_t value, uint32_t align) +{ + if (value % align) + value -= (value % align); + return value; +} + /* Buffer and file I/O */ struct buffer { char *name;