Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56001 )
Change subject: util/cbfstool: Remove legacy cbfs support ......................................................................
util/cbfstool: Remove legacy cbfs support
FMAP has been there for quite some time and the coreboot code itself cannot handle legacy images anymore so it makes sense to drop support in cbfstool itself.
Change-Id: I2f34a25dae1ec1924cabe392dcd1480d7b4fbf49 Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M util/cbfstool/cbfs_image.c M util/cbfstool/cbfs_image.h M util/cbfstool/cbfstool.c 3 files changed, 21 insertions(+), 261 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/01/56001/1
diff --git a/util/cbfstool/cbfs_image.c b/util/cbfstool/cbfs_image.c index 1fb19ba..598f697 100644 --- a/util/cbfstool/cbfs_image.c +++ b/util/cbfstool/cbfs_image.c @@ -241,94 +241,7 @@ capacity, ""); }
-int cbfs_legacy_image_create(struct cbfs_image *image, - uint32_t architecture, - uint32_t align, - struct buffer *bootblock, - uint32_t bootblock_offset, - uint32_t header_offset, - uint32_t entries_offset) -{ - assert(image); - assert(image->buffer.data); - assert(bootblock); - - int32_t *rel_offset; - uint32_t cbfs_len; - void *header_loc; - size_t size = image->buffer.size; - - DEBUG("cbfs_image_create: bootblock=0x%x+0x%zx, " - "header=0x%x+0x%zx, entries_offset=0x%x\n", - bootblock_offset, bootblock->size, header_offset, - sizeof(image->header), entries_offset); - - DEBUG("cbfs_create_image: (real offset) bootblock=0x%x, " - "header=0x%x, entries_offset=0x%x\n", - bootblock_offset, header_offset, entries_offset); - - // Prepare bootblock - if (bootblock_offset + bootblock->size > size) { - ERROR("Bootblock (0x%x+0x%zx) exceed ROM size (0x%zx)\n", - bootblock_offset, bootblock->size, size); - return -1; - } - if (entries_offset > bootblock_offset && - entries_offset < bootblock->size) { - ERROR("Bootblock (0x%x+0x%zx) overlap CBFS data (0x%x)\n", - bootblock_offset, bootblock->size, entries_offset); - return -1; - } - memcpy(image->buffer.data + bootblock_offset, bootblock->data, - bootblock->size); - - // Prepare header - if (header_offset + sizeof(image->header) > size - sizeof(int32_t)) { - ERROR("Header (0x%x+0x%zx) exceed ROM size (0x%zx)\n", - header_offset, sizeof(image->header), size); - return -1; - } - image->header.magic = CBFS_HEADER_MAGIC; - image->header.version = CBFS_HEADER_VERSION; - image->header.romsize = size; - image->header.bootblocksize = bootblock->size; - image->header.align = align; - image->header.offset = entries_offset; - image->header.architecture = architecture; - - header_loc = (image->buffer.data + header_offset); - cbfs_put_header(header_loc, &image->header); - image->has_header = true; - - // The last 4 byte of the image contain the relative offset from the end - // of the image to the master header as a 32-bit signed integer. x86 - // relies on this also being its (memory-mapped, top-aligned) absolute - // 32-bit address by virtue of how two's complement numbers work. - assert(size % sizeof(int32_t) == 0); - rel_offset = (int32_t *)(image->buffer.data + size - sizeof(int32_t)); - *rel_offset = header_offset - size; - - // Prepare entries - if (align_up(entries_offset, align) != entries_offset) { - ERROR("Offset (0x%x) must be aligned to 0x%x.\n", - entries_offset, align); - return -1; - } - // To calculate available length, find - // e = min(bootblock, header, rel_offset) where e > entries_offset. - cbfs_len = size - sizeof(int32_t); - if (bootblock_offset > entries_offset && bootblock_offset < cbfs_len) - cbfs_len = bootblock_offset; - if (header_offset > entries_offset && header_offset < cbfs_len) - cbfs_len = header_offset; - - if (cbfs_image_create(image, cbfs_len - entries_offset)) - return -1; - return 0; -} - -int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in, - uint32_t offset) +int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in) { assert(out); assert(in); @@ -341,16 +254,14 @@ return 0; }
- void *header_loc = cbfs_find_header(in->data, in->size, offset); + void *header_loc = cbfs_find_header(in->data, in->size, ~0); if (header_loc) { cbfs_get_header(&out->header, header_loc); out->has_header = true; cbfs_fix_legacy_size(out, header_loc); return 0; - } else if (offset != ~0u) { - ERROR("The -H switch is only valid on legacy images having CBFS master headers.\n"); - return 1; } + ERROR("Selected image region is not a valid CBFS.\n"); return 1; } @@ -416,7 +327,7 @@
struct cbfs_image image; memset(&image, 0, sizeof(image)); - if (cbfs_image_from_buffer(&image, region, 0)) { + if (cbfs_image_from_buffer(&image, region)) { ERROR("reading CBFS failed!\n"); return 1; } @@ -455,7 +366,7 @@
struct cbfs_image image; memset(&image, 0, sizeof(image)); - if (cbfs_image_from_buffer(&image, region, 0)) { + if (cbfs_image_from_buffer(&image, region)) { ERROR("reading CBFS failed!\n"); return 1; } diff --git a/util/cbfstool/cbfs_image.h b/util/cbfstool/cbfs_image.h index 664d146..36eaf34 100644 --- a/util/cbfstool/cbfs_image.h +++ b/util/cbfstool/cbfs_image.h @@ -39,26 +39,12 @@ * Returns 0 on success, otherwise nonzero. */ int cbfs_image_create(struct cbfs_image *image, size_t entries_size);
-/* Creates an empty CBFS image by given size, and description to its content - * (bootblock, align, header location, starting offset of CBFS entries). - * The output image will contain a valid cbfs_header, with one cbfs_file - * entry with type CBFS_TYPE_NULL, with max available size. - * Only call this if you want a legacy CBFS with a master header. - * Returns 0 on success, otherwise nonzero. */ -int cbfs_legacy_image_create(struct cbfs_image *image, - uint32_t arch, - uint32_t align, - struct buffer *bootblock, - uint32_t bootblock_offset, - uint32_t header_offset, - uint32_t entries_offset);
/* Constructs a cbfs_image from a buffer. The resulting image contains a shallow * copy of the buffer; releasing either one is the legal way to clean up after * both of them at once. Always produces a cbfs_image, but... * Returns 0 if it contains a valid CBFS, non-zero if it's unrecognized data. */ -int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in, - uint32_t offset); +int cbfs_image_from_buffer(struct cbfs_image *out, struct buffer *in);
/* Create a duplicate CBFS image. Returns 0 on success, otherwise non-zero. * Will not succeed on new-style images without a master header. */ diff --git a/util/cbfstool/cbfstool.c b/util/cbfstool/cbfstool.c index dc6a432..9828c79 100644 --- a/util/cbfstool/cbfstool.c +++ b/util/cbfstool/cbfstool.c @@ -45,7 +45,6 @@ const char *fmap; const char *region_name; const char *source_region; - const char *bootblock; const char *ignore_section; const char *ucode_region; uint64_t u64val; @@ -58,24 +57,10 @@ long long int baseaddress_input; uint32_t baseaddress_assigned; uint32_t loadaddress; - uint32_t headeroffset; - /* - * Input can be negative. It will be transformed to offset from start of region (if - * negative) and stored in baseaddress. - */ - long long int headeroffset_input; - uint32_t headeroffset_assigned; uint32_t entrypoint; uint32_t size; uint32_t alignment; uint32_t pagesize; - uint32_t cbfsoffset; - /* - * Input can be negative. It will be transformed to corresponding region offset (if - * negative) and stored in baseaddress. - */ - long long int cbfsoffset_input; - uint32_t cbfsoffset_assigned; uint32_t arch; uint32_t padding; uint32_t topswap_size; @@ -113,7 +98,6 @@ .arch = CBFS_ARCHITECTURE_UNKNOWN, .compression = CBFS_COMPRESS_NONE, .hash = VB2_HASH_INVALID, - .headeroffset = ~0, .region_name = SECTION_NAME_PRIMARY_CBFS, .u64val = -1, }; @@ -166,7 +150,7 @@ SECTION_NAME_PRIMARY_CBFS)) goto no_metadata_hash; mhc.region = SECTION_NAME_PRIMARY_CBFS; - if (cbfs_image_from_buffer(&cbfs, &buffer, param.headeroffset)) + if (cbfs_image_from_buffer(&cbfs, &buffer)) goto no_metadata_hash; bootblock = cbfs_get_entry(&cbfs, "bootblock"); if (!bootblock || ntohl(bootblock->type) != CBFS_TYPE_BOOTBLOCK) @@ -521,8 +505,7 @@ }
struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1;
if (cbfs_get_entry(&image, param.name)) @@ -582,8 +565,7 @@
static int cbfs_add_integer_component(const char *name, uint64_t u64val, - uint32_t offset, - uint32_t headeroffset) { + uint32_t offset) { struct cbfs_image image; struct cbfs_file *header = NULL; struct buffer buffer; @@ -600,7 +582,7 @@ for (i = 0; i < 8; i++) buffer.data[i] = (u64val >> i*8) & 0xff;
- if (cbfs_image_from_buffer(&image, param.image_region, headeroffset)) { + if (cbfs_image_from_buffer(&image, param.image_region)) { ERROR("Selected image region is not a CBFS.\n"); goto done; } @@ -695,8 +677,7 @@ size_t size; void *h_loc;
- if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) { + if (cbfs_image_from_buffer(&image, param.image_region)) { ERROR("Selected image region is not a CBFS.\n"); return 1; } @@ -813,7 +794,6 @@ static int cbfs_add_component(const char *filename, const char *name, uint32_t type, - uint32_t headeroffset, convert_buffer_t convert) { size_t len_align = 0; @@ -840,7 +820,7 @@ }
struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1;
if (cbfs_get_entry(&image, name)) { @@ -1254,7 +1234,6 @@ return cbfs_add_component(param.filename, param.name, param.type, - param.headeroffset, convert); }
@@ -1275,7 +1254,6 @@ return cbfs_add_component(param.filename, param.name, CBFS_TYPE_STAGE, - param.headeroffset, cbfstool_convert_mkstage); }
@@ -1284,7 +1262,6 @@ return cbfs_add_component(param.filename, param.name, CBFS_TYPE_SELF, - param.headeroffset, cbfstool_convert_mkpayload); }
@@ -1303,7 +1280,6 @@ return cbfs_add_component(param.filename, param.name, CBFS_TYPE_SELF, - param.headeroffset, cbfstool_convert_mkflatpayload); }
@@ -1314,9 +1290,8 @@ return 1; } return cbfs_add_integer_component(param.name, - param.u64val, - param.baseaddress, - param.headeroffset); + param.u64val, + param.baseaddress); }
static int cbfs_remove(void) @@ -1327,8 +1302,7 @@ }
struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1;
if (cbfs_remove_entry(&image, param.name) != 0) { @@ -1346,84 +1320,7 @@ memset(&image, 0, sizeof(image)); buffer_clone(&image.buffer, param.image_region);
- if (param.fmap) { - if (param.arch != CBFS_ARCHITECTURE_UNKNOWN || param.size || - param.baseaddress_assigned || - param.headeroffset_assigned || - param.cbfsoffset_assigned || - param.bootblock) { - ERROR("Since -M was provided, -m, -s, -b, -o, -H, and -B should be omitted\n"); - return 1; - } - - return cbfs_image_create(&image, image.buffer.size); - } - - if (param.arch == CBFS_ARCHITECTURE_UNKNOWN) { - ERROR("You need to specify -m/--machine arch.\n"); - return 1; - } - - struct buffer bootblock; - if (!param.bootblock) { - DEBUG("-B not given, creating image without bootblock.\n"); - if (buffer_create(&bootblock, 0, "(dummy)") != 0) - return 1; - } else if (buffer_from_file(&bootblock, param.bootblock)) { - return 1; - } - - if (!param.alignment) - param.alignment = CBFS_ALIGNMENT; - - // Set default offsets. x86, as usual, needs to be a special snowflake. - if (!param.baseaddress_assigned) { - if (param.arch == CBFS_ARCHITECTURE_X86) { - // Make sure there's at least enough room for rel_offset - param.baseaddress = param.size - - MAX(bootblock.size, sizeof(int32_t)); - DEBUG("x86 -> bootblock lies at end of ROM (%#x).\n", - param.baseaddress); - } else { - param.baseaddress = 0; - DEBUG("bootblock starts at address 0x0.\n"); - } - } - if (!param.headeroffset_assigned) { - if (param.arch == CBFS_ARCHITECTURE_X86) { - param.headeroffset = param.baseaddress - - sizeof(struct cbfs_header); - DEBUG("x86 -> CBFS header before bootblock (%#x).\n", - param.headeroffset); - } else { - param.headeroffset = align_up(param.baseaddress + - bootblock.size, sizeof(uint32_t)); - DEBUG("CBFS header placed behind bootblock (%#x).\n", - param.headeroffset); - } - } - if (!param.cbfsoffset_assigned) { - if (param.arch == CBFS_ARCHITECTURE_X86) { - param.cbfsoffset = 0; - DEBUG("x86 -> CBFS entries start at address 0x0.\n"); - } else { - param.cbfsoffset = align_up(param.headeroffset + - sizeof(struct cbfs_header), - CBFS_ALIGNMENT); - DEBUG("CBFS entries start beind master header (%#x).\n", - param.cbfsoffset); - } - } - - int ret = cbfs_legacy_image_create(&image, - param.arch, - CBFS_ALIGNMENT, - &bootblock, - param.baseaddress, - param.headeroffset, - param.cbfsoffset); - buffer_delete(&bootblock); - return ret; + return cbfs_image_create(&image, image.buffer.size); }
static int cbfs_layout(void) @@ -1496,8 +1393,7 @@ static int cbfs_print(void) { struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1; if (param.machine_parseable) { if (verbose) @@ -1551,8 +1447,7 @@ }
struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1;
return cbfs_export_entry(&image, param.name, param.filename, @@ -1668,7 +1563,7 @@ return 1; }
- if (cbfs_image_from_buffer(&src_image, &src_buf, param.headeroffset)) + if (cbfs_image_from_buffer(&src_image, &src_buf)) return 1;
return cbfs_copy_instance(&src_image, param.image_region); @@ -1677,8 +1572,7 @@ static int cbfs_compact(void) { struct cbfs_image image; - if (cbfs_image_from_buffer(&image, param.image_region, - param.headeroffset)) + if (cbfs_image_from_buffer(&image, param.image_region)) return 1; WARN("Compacting a CBFS doesn't honor alignment or fixed addresses!\n"); return cbfs_compact_instance(&image); @@ -1750,7 +1644,6 @@ static struct option long_options[] = { {"alignment", required_argument, 0, 'a' }, {"base-address", required_argument, 0, 'b' }, - {"bootblock", required_argument, 0, 'B' }, {"cmdline", required_argument, 0, 'C' }, {"compression", required_argument, 0, 'c' }, {"topswap-size", required_argument, 0, 'j' }, @@ -1764,7 +1657,6 @@ {"force", no_argument, 0, 'F' }, {"source-region", required_argument, 0, 'R' }, {"hash-algorithm",required_argument, 0, 'A' }, - {"header-offset", required_argument, 0, 'H' }, {"help", no_argument, 0, 'h' }, {"ignore-sec", required_argument, 0, 'S' }, {"initrd", required_argument, 0, 'I' }, @@ -1772,7 +1664,6 @@ {"load-address", required_argument, 0, 'l' }, {"machine", required_argument, 0, 'm' }, {"name", required_argument, 0, 'n' }, - {"offset", required_argument, 0, 'o' }, {"padding", required_argument, 0, 'p' }, {"pow2page", no_argument, 0, 'Q' }, {"ucode-region", required_argument, 0, 'q' }, @@ -1809,10 +1700,6 @@
if (param.baseaddress_assigned) ret |= get_region_offset(param.baseaddress_input, ¶m.baseaddress); - if (param.headeroffset_assigned) - ret |= get_region_offset(param.headeroffset_input, ¶m.headeroffset); - if (param.cbfsoffset_assigned) - ret |= get_region_offset(param.cbfsoffset_input, ¶m.cbfsoffset);
return ret; } @@ -1928,9 +1815,6 @@ "Defragment CBFS image.\n" " copy -r image,regions -R source-region " "Create a copy (duplicate) cbfs instance in fmap\n" - " create -m ARCH -s size [-b bootblock offset] \\n" - " [-o CBFS offset] [-H header offset] [-B bootblock] " - "Create a legacy ROM file with CBFS master header*\n" " create -M flashmap [-r list,of,regions,containing,cbfses] " "Create a new-style partitioned firmware image\n" " locate [-r image,regions] -f FILE -n NAME [-P page-size] \\n" @@ -1951,7 +1835,7 @@ " expand [-r fmap-region] " "Expand CBFS to span entire region\n" "OFFSETs:\n" - " Numbers accompanying -b, -H, and -o switches* may be provided\n" + " Numbers accompanying the -b switche may be provided\n" " in two possible formats: if their value is greater than\n" " 0x80000000, they are interpreted as a top-aligned x86 memory\n" " address; otherwise, they are treated as an offset into flash.\n" @@ -2130,18 +2014,6 @@ return 1; } break; - case 'B': - param.bootblock = optarg; - break; - case 'H': - param.headeroffset_input = strtoll(optarg, &suffix, 0); - if (!*optarg || (suffix && *suffix)) { - ERROR("Invalid header offset '%s'.\n", - optarg); - return 1; - } - param.headeroffset_assigned = 1; - break; case 'a': param.alignment = strtoul(optarg, &suffix, 0); if (!*optarg || (suffix && *suffix)) { @@ -2161,15 +2033,6 @@ case 'Q': param.force_pow2_pagesize = 1; break; - case 'o': - param.cbfsoffset_input = strtoll(optarg, &suffix, 0); - if (!*optarg || (suffix && *suffix)) { - ERROR("Invalid cbfs offset '%s'.\n", - optarg); - return 1; - } - param.cbfsoffset_assigned = 1; - break; case 'f': param.filename = optarg; break;
-
Arthur Heymans (Code Review)