Furquan Shaikh has uploaded this change for review.
util/cbfstool: Treat region offsets differently than absolute addresses
cbfstool overloads baseaddress to represent multiple things:
1. Address in SPI flash space
2. Address in host space (for x86 platforms)
3. Offset from end of region (accepted as negative number)
This was done so that the different functions that use these
addresses/offsets don't need to be aware of what the value represents
and can use the helper functions convert_to_from* to get the required
values.
Thus, even if the user provides a negative value to represent offset
from end of region, it was stored as an unsigned integer. There are
special checks in convert_to_from_top_aligned which guesses if the
value provided is really an offset from the end of region and converts
it to an offset from start of region.
This has worked okay until now for x86 platforms because there is a
single fixed decode window mapping the SPI flash to host address
space. However, going forward new platforms might need to support more
decode windows that are not contiguous in the host space. Thus, it is
important to distinguish between offsets from end of region and
addresses in host/SPI flash space and treat them separately.
As a first step towards supporting this requirement for multiple
decode windows on new platforms, this change handles the negative
offset provided as input in dispatch_command before the requested cbfs
operation is performed.
This change adds baseaddress_input, headeroffset_input and
cbfsoffset_input to struct param and converts them to offsets from
start of region before storing into baseaddress, headeroffset and
cbfsoffset if the inputs are negative.
In follow up changes, cbfstool will be extended to add support
for multiple decode windows.
TEST=Verified using abuild with timeless option for all coreboot
boards that there is no change in the resultant coreboot.rom file.
Change-Id: Ib74a7e6ed9e88fbc5489640d73bedac14872953f
Signed-off-by: Furquan Shaikh <furquan@google.com>
---
M util/cbfstool/cbfs_image.c
M util/cbfstool/cbfstool.c
2 files changed, 54 insertions(+), 41 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/47829/1
diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c
index a042f0d..3f1bf43 100644
--- a/util/cbfstool/cbfs_image.c
+++ b/util/cbfstool/cbfs_image.c
@@ -277,14 +277,6 @@
bootblock_offset, bootblock->size, header_offset,
sizeof(image->header), entries_offset);
- // Adjust legacy top-aligned address to ROM offset.
- if (IS_TOP_ALIGNED_ADDRESS(entries_offset))
- entries_offset = size + (int32_t)entries_offset;
- if (IS_TOP_ALIGNED_ADDRESS(bootblock_offset))
- bootblock_offset = size + (int32_t)bootblock_offset;
- if (IS_TOP_ALIGNED_ADDRESS(header_offset))
- header_offset = size + (int32_t)header_offset;
-
DEBUG("cbfs_create_image: (real offset) bootblock=0x%x, "
"header=0x%x, entries_offset=0x%x\n",
bootblock_offset, header_offset, entries_offset);
diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c
index d2df1cc..c719b8f 100644
--- a/util/cbfstool/cbfstool.c
+++ b/util/cbfstool/cbfstool.c
@@ -47,15 +47,22 @@
uint64_t u64val;
uint32_t type;
uint32_t baseaddress;
+ /*
+ * Input can be negative. It will be transformed to corresponding region offset and
+ * stored in baseaddress.
+ */
+ long long int baseaddress_input;
uint32_t baseaddress_assigned;
uint32_t loadaddress;
uint32_t headeroffset;
+ long long int headeroffset_input;
uint32_t headeroffset_assigned;
uint32_t entrypoint;
uint32_t size;
uint32_t alignment;
uint32_t pagesize;
uint32_t cbfsoffset;
+ long long int cbfsoffset_input;
uint32_t cbfsoffset_assigned;
uint32_t arch;
uint32_t padding;
@@ -102,8 +109,7 @@
/*
* Converts between offsets from the start of the specified image region and
- * "top-aligned" offsets from the top of the entire boot media. See comment
- * below for convert_to_from_top_aligned() about forming addresses.
+ * "top-aligned" offsets from the top of the entire boot media.
*/
static unsigned convert_to_from_absolute_top_aligned(
const struct buffer *region, unsigned offset)
@@ -116,25 +122,26 @@
}
/*
- * Converts between offsets from the start of the specified image region and
- * "top-aligned" offsets from the top of the image region. 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-aligned *address*, which is its arithmetic inverse. */
-static unsigned convert_to_from_top_aligned(const struct buffer *region,
- unsigned offset)
+ * This function takes offset value which represents the offset from one end of the region and
+ * returns the offset from the other end of the region. offset is expected to be positive.
+ */
+static unsigned convert_region_offset(unsigned offset)
{
- assert(region);
+ size_t size;
- /* Cover the situation where a negative base address is given by the
- * user. Callers of this function negate it, so it'll be a positive
- * number smaller than the region.
- */
- if ((offset > 0) && (offset < region->size)) {
- return region->size - offset;
+ if (param.size) {
+ size = param.size;
+ } else {
+ assert(param.image_region);
+ size = param.image_region->size;
}
- return convert_to_from_absolute_top_aligned(region, offset);
+ if (size < offset) {
+ ERROR("Cannot convert region offset (size=0x%zx, offset=0x%x\n", size, offset);
+ return 0;
+ }
+
+ return size - offset;
}
static int do_cbfs_locate(int32_t *cbfs_addr, size_t metadata_size,
@@ -242,10 +249,6 @@
goto done;
}
- if (IS_TOP_ALIGNED_ADDRESS(offset))
- offset = convert_to_from_top_aligned(param.image_region,
- -offset);
-
header = cbfs_create_file_header(CBFS_COMPONENT_RAW,
buffer.size, name);
if (cbfs_add_entry(&image, &buffer, offset, header, 0) != 0) {
@@ -434,8 +437,7 @@
buffer_clone(buffer, &new_bootblock);
/* Update the location (offset) of bootblock in the region */
- *offset = convert_to_from_top_aligned(param.image_region,
- buffer_size(buffer));
+ *offset = convert_region_offset(buffer_size(buffer));
return 0;
}
@@ -519,11 +521,9 @@
/* to the cbfs file and therefore set the position */
/* the real beginning of the data. */
if (type == CBFS_COMPONENT_STAGE)
- attrs->position = htonl(offset +
- sizeof(struct cbfs_stage));
+ attrs->position = htonl(offset - sizeof(struct cbfs_stage));
else if (type == CBFS_COMPONENT_SELF)
- attrs->position = htonl(offset +
- sizeof(struct cbfs_payload));
+ attrs->position = htonl(offset - sizeof(struct cbfs_payload));
else
attrs->position = htonl(offset);
}
@@ -564,8 +564,8 @@
}
if (IS_TOP_ALIGNED_ADDRESS(offset))
- offset = convert_to_from_top_aligned(param.image_region,
- -offset);
+ offset = convert_to_from_absolute_top_aligned(param.image_region,
+ -offset);
if (cbfs_add_entry(&image, &buffer, offset, header, len_align) != 0) {
ERROR("Failed to add '%s' into ROM image.\n", filename);
free(header);
@@ -1336,6 +1336,16 @@
{NULL, 0, 0, 0 }
};
+static uint32_t get_region_offset(long long int offset)
+{
+ /* If offset is not negative, no transformation required. */
+ if (offset >= 0)
+ return offset;
+
+ /* Calculate offset from start of region. */
+ return convert_region_offset(-offset);
+}
+
static int dispatch_command(struct command command)
{
if (command.accesses_region) {
@@ -1373,6 +1383,17 @@
return 1;
}
}
+
+ /*
+ * Once image region is read, input offsets can be adjusted accordingly if the
+ * inputs are provided as negative integers i.e. offsets from end of region.
+ */
+ if (param.baseaddress_assigned)
+ param.baseaddress = get_region_offset(param.baseaddress_input);
+ if (param.headeroffset_assigned)
+ param.headeroffset = get_region_offset(param.headeroffset_input);
+ if (param.cbfsoffset_assigned)
+ param.cbfsoffset = get_region_offset(param.cbfsoffset_input);
}
if (command.function()) {
@@ -1592,7 +1613,7 @@
param.source_region = optarg;
break;
case 'b':
- param.baseaddress = strtoul(optarg, &suffix, 0);
+ param.baseaddress_input = strtoll(optarg, &suffix, 0);
if (!*optarg || (suffix && *suffix)) {
ERROR("Invalid base address '%s'.\n",
optarg);
@@ -1643,8 +1664,7 @@
param.bootblock = optarg;
break;
case 'H':
- param.headeroffset = strtoul(
- optarg, &suffix, 0);
+ param.headeroffset_input = strtoll(optarg, &suffix, 0);
if (!*optarg || (suffix && *suffix)) {
ERROR("Invalid header offset '%s'.\n",
optarg);
@@ -1680,7 +1700,7 @@
param.force_pow2_pagesize = 1;
break;
case 'o':
- param.cbfsoffset = strtoul(optarg, &suffix, 0);
+ param.cbfsoffset_input = strtoll(optarg, &suffix, 0);
if (!*optarg || (suffix && *suffix)) {
ERROR("Invalid cbfs offset '%s'.\n",
optarg);
@@ -1817,6 +1837,7 @@
seen_primary_cbfs = true;
param.image_region = image_regions + region;
+
if (dispatch_command(commands[i])) {
partitioned_file_close(param.image_file);
return 1;
To view, visit change 47829. To unsubscribe, or for help writing mail filters, visit settings.