Attention is currently required from: Zheng Bao. Hello Zheng Bao,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/59354
to review the following change.
Change subject: amdfwtool: Upgrade "relative address" to four address modes ......................................................................
amdfwtool: Upgrade "relative address" to four address modes
Address Mode 0: Physical Address, bit 63~56: 0x00 Address Mode 1: Relative Address to entire BIOS image, bit 63~56: 0x40 Address Mode 2: Relative Address to PSP/BIOS directory, bit 63~56: 0x80 Address Mode 3: Relative Address to slot N, bit 63~56: 0xC0
It is the expanding mode for simple relative address mode, for which address_mode equals 1.
Only mode 2 is added. We need to record current table base address and calculate the offset. The ctx.current_table is zero outside the table. When it goes into the function to integrate the table, it should backup the old value and get current table base. Before it goes out the function, it should restore the value.
The old mode 0 should be back compatible.
Change-Id: I29a03f4381cd0507e2b2e3b359111e3375a73de1 Signed-off-by: Zheng Bao fishbaozi@gmail.com
address mode
Change-Id: Ie221736c8eec2d9b535909273402ac388521aa8a Signed-off-by: Zheng Bao fishbaozi@gmail.com --- M util/amdfwtool/amdfwtool.c M util/amdfwtool/amdfwtool.h 2 files changed, 80 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/59354/1
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 35b28ab..e0c99c3 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -343,17 +343,34 @@ typedef struct _context { char *rom; /* target buffer, size of flash device */ uint32_t rom_size; /* size of flash device */ - uint32_t abs_address; /* produce absolute or relative address */ + uint32_t address_mode; /* 0:abs address; 1:relative to flash; 2: relative to table */ uint32_t current; /* pointer within flash & proxy buffer */ + uint32_t current_table; } context;
#define RUN_BASE(ctx) (0xFFFFFFFF - (ctx).rom_size + 1) -#define RUN_OFFSET(ctx, offset) ((ctx).abs_address ? RUN_BASE(ctx) + (offset) : (offset)) +#define RUN_OFFSET_MODE(ctx, offset, mode) \ + ((mode) == 0 ? RUN_BASE(ctx) + (offset) : \ + ((mode) == 1 ? (offset) : \ + ((mode) == 2 ? (offset) - ctx.current_table : (offset)) )) +#define RUN_OFFSET(ctx, offset) RUN_OFFSET_MODE((ctx), (offset), (ctx).address_mode) +#define RUN_TO_OFFSET(ctx, run) ((ctx).address_mode == 0 ? (run) - RUN_BASE(ctx) : (run)) /* TODO: */ #define RUN_CURRENT(ctx) RUN_OFFSET((ctx), (ctx).current) +/* The mode in entry can not be higher than the header's. + For example, if table mode is 0, all the entry mode will be 0. */ +#define RUN_CURRENT_MODE(ctx, mode) RUN_OFFSET_MODE((ctx), (ctx).current, \ + (ctx).address_mode < (mode) ? (ctx).address_mode : (mode)) #define BUFF_OFFSET(ctx, offset) ((void *)((ctx).rom + (offset))) #define BUFF_CURRENT(ctx) BUFF_OFFSET((ctx), (ctx).current) #define BUFF_TO_RUN(ctx, ptr) RUN_OFFSET((ctx), ((char *)(ptr) - (ctx).rom)) +#define BUFF_TO_RUN_MODE(ctx, ptr, mode) RUN_OFFSET_MODE((ctx), ((char *)(ptr) - (ctx).rom), \ + (ctx).address_mode < (mode) ? (ctx).address_mode : (mode)) #define BUFF_ROOM(ctx) ((ctx).rom_size - (ctx).current) +/* Only set the address mode in entry if the table is mode 2. */ +#define SET_ADDR_MODE(mode, table) \ + ((table)->header.additional_info_fields.address_mode == 2 ? (mode) : 0) +#define SET_ADDR_MODE_BY_TABLE(table) \ + SET_ADDR_MODE((table)->header.additional_info_fields.address_mode, table)
void assert_fw_entry(uint32_t count, uint32_t max, context *ctx) { @@ -380,7 +397,8 @@ ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT);
ptr = BUFF_CURRENT(*ctx); - ((psp_directory_header *)ptr)->additional_info = ctx->current; + ((psp_directory_header *)ptr)->additional_info = 0; + ((psp_directory_header *)ptr)->additional_info_fields.address_mode = ctx->address_mode; ctx->current += sizeof(psp_directory_header) + MAX_PSP_ENTRIES * sizeof(psp_directory_entry); return ptr; @@ -432,14 +450,16 @@ break; case PSP_COOKIE: case PSPL2_COOKIE: - table_size = ctx->current - dir->header.additional_info; + table_size = ctx->current - ctx->current_table; if ((table_size % TABLE_ALIGNMENT) != 0) { fprintf(stderr, "The PSP table size should be 4K aligned\n"); exit(1); } dir->header.cookie = cookie; dir->header.num_entries = count; - dir->header.additional_info = (table_size / 0x1000) | (1 << 10); + dir->header.additional_info_fields.dir_size = table_size / 0x1000; + dir->header.additional_info_fields.spi_block_size = 1; + dir->header.additional_info_fields.base_addr = 0; /* checksum everything that comes after the Checksum field */ dir->header.checksum = fletcher32(&dir->header.num_entries, count * sizeof(psp_directory_entry) @@ -448,14 +468,16 @@ break; case BDT1_COOKIE: case BDT2_COOKIE: - table_size = ctx->current - bdir->header.additional_info; + table_size = ctx->current - ctx->current_table; if ((table_size % TABLE_ALIGNMENT) != 0) { fprintf(stderr, "The BIOS table size should be 4K aligned\n"); exit(1); } bdir->header.cookie = cookie; bdir->header.num_entries = count; - bdir->header.additional_info = (table_size / 0x1000) | (1 << 10); + bdir->header.additional_info_fields.dir_size = table_size / 0x1000;; + bdir->header.additional_info_fields.spi_block_size = 1; + bdir->header.additional_info_fields.base_addr = 0; /* checksum everything that comes after the Checksum field */ bdir->header.checksum = fletcher32(&bdir->header.num_entries, count * sizeof(bios_directory_entry) @@ -641,6 +663,7 @@ ssize_t bytes; unsigned int i, count; int level; + uint32_t current_table_save;
/* This function can create a primary table, a secondary table, or a * flattened table which contains all applicable types. These if-else @@ -658,6 +681,8 @@ else level = PSP_BOTH;
+ current_table_save = ctx->current_table; + ctx->current_table = (char *)pspdir - ctx->rom; ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT);
for (i = 0, count = 0; fw_table[i].type != AMD_FW_INVALID; i++) { @@ -673,6 +698,7 @@ pspdir->entries[count].type = fw_table[i].type; pspdir->entries[count].size = 4096; /* TODO: doc? */ pspdir->entries[count].addr = RUN_CURRENT(*ctx); + pspdir->entries[count].address_mode = SET_ADDR_MODE_BY_TABLE(pspdir); pspdir->entries[count].subprog = fw_table[i].subprog; pspdir->entries[count].rsvd = 0; ctx->current = ALIGN(ctx->current + 4096, 0x100U); @@ -683,6 +709,7 @@ pspdir->entries[count].rsvd = 0; pspdir->entries[count].size = 0xFFFFFFFF; pspdir->entries[count].addr = fw_table[i].other; + pspdir->entries[count].address_mode = 0; count++; } else if (fw_table[i].type == AMD_FW_PSP_NVRAM) { if (fw_table[i].filename == NULL) @@ -704,7 +731,8 @@ pspdir->entries[count].rsvd = 0; pspdir->entries[count].size = ALIGN(bytes, ERASE_ALIGNMENT); - pspdir->entries[count].addr = RUN_CURRENT(*ctx); + pspdir->entries[count].addr = RUN_CURRENT_MODE(*ctx, 1); + pspdir->entries[count].address_mode = SET_ADDR_MODE(1, pspdir);
ctx->current = ALIGN(ctx->current + bytes, BLOB_ERASE_ALIGNMENT); @@ -722,6 +750,7 @@ pspdir->entries[count].rsvd = 0; pspdir->entries[count].size = (uint32_t)bytes; pspdir->entries[count].addr = RUN_CURRENT(*ctx); + pspdir->entries[count].address_mode = SET_ADDR_MODE_BY_TABLE(pspdir);
ctx->current = ALIGN(ctx->current + bytes, BLOB_ALIGNMENT); @@ -740,11 +769,13 @@ + pspdir2->header.num_entries * sizeof(psp_directory_entry);
- pspdir->entries[count].addr = BUFF_TO_RUN(*ctx, pspdir2); + pspdir->entries[count].addr = BUFF_TO_RUN_MODE(*ctx, pspdir2, 1); + pspdir->entries[count].address_mode = SET_ADDR_MODE(1, pspdir); count++; }
fill_dir_header(pspdir, count, cookie, ctx); + ctx->current_table = current_table_save; }
static void *new_bios_dir(context *ctx, bool multi) @@ -761,7 +792,9 @@ else ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT); ptr = BUFF_CURRENT(*ctx); - ((bios_directory_hdr *) ptr)->additional_info = ctx->current; + ((bios_directory_hdr *) ptr)->additional_info = 0; + ((bios_directory_hdr *) ptr)->additional_info_fields.address_mode = ctx->address_mode; + ctx->current_table = ctx->current; ctx->current += sizeof(bios_directory_hdr) + MAX_BIOS_ENTRIES * sizeof(bios_directory_entry); return ptr; @@ -913,16 +946,19 @@ case AMD_BIOS_APOB: biosdir->entries[count].size = fw_table[i].size; biosdir->entries[count].source = fw_table[i].src; + biosdir->entries[count].address_mode = SET_ADDR_MODE_BY_TABLE(biosdir); break; case AMD_BIOS_APOB_NV: if (fw_table[i].src) { /* If source is given, use that and its size */ biosdir->entries[count].source = fw_table[i].src; + biosdir->entries[count].address_mode = SET_ADDR_MODE(1, biosdir); biosdir->entries[count].size = fw_table[i].size; } else { /* Else reserve size bytes within amdfw.rom */ ctx->current = ALIGN(ctx->current, ERASE_ALIGNMENT); biosdir->entries[count].source = RUN_CURRENT(*ctx); + biosdir->entries[count].address_mode = SET_ADDR_MODE(1, biosdir); biosdir->entries[count].size = ALIGN( fw_table[i].size, ERASE_ALIGNMENT); memset(BUFF_CURRENT(*ctx), 0xff, @@ -935,12 +971,14 @@ /* Don't make a 2nd copy, point to the same one */ if (level == BDT_LVL1 && locate_bdt2_bios(biosdir2, &source, &size)) { biosdir->entries[count].source = source; + biosdir->entries[count].address_mode = SET_ADDR_MODE(1, biosdir); biosdir->entries[count].size = size; break; }
/* level 2, or level 1 and no copy found in level 2 */ biosdir->entries[count].source = fw_table[i].src; + biosdir->entries[count].address_mode = SET_ADDR_MODE(1, biosdir); biosdir->entries[count].dest = fw_table[i].dest; biosdir->entries[count].size = fw_table[i].size;
@@ -954,7 +992,8 @@ exit(1); }
- biosdir->entries[count].source = RUN_CURRENT(*ctx); + biosdir->entries[count].source = RUN_CURRENT_MODE(*ctx, 1); + biosdir->entries[count].address_mode = SET_ADDR_MODE(1, biosdir);
ctx->current = ALIGN(ctx->current + bytes, 0x100U); break; @@ -977,6 +1016,7 @@
biosdir->entries[count].size = (uint32_t)bytes; biosdir->entries[count].source = RUN_CURRENT(*ctx); + biosdir->entries[count].address_mode = SET_ADDR_MODE_BY_TABLE(biosdir);
ctx->current = ALIGN(ctx->current + bytes, 0x100U); break; @@ -994,6 +1034,7 @@ * sizeof(bios_directory_entry); biosdir->entries[count].source = BUFF_TO_RUN(*ctx, biosdir2); + biosdir->entries[count].address_mode = SET_ADDR_MODE(1, biosdir); biosdir->entries[count].subprog = 0; biosdir->entries[count].inst = 0; biosdir->entries[count].copy = 0; @@ -1598,15 +1639,16 @@ }
if (amd_romsig->efs_gen.gen == EFS_SECOND_GEN) - ctx.abs_address = 0; + ctx.address_mode = 1; else - ctx.abs_address = 1; + ctx.address_mode = 0; printf(" AMDFWTOOL Using firmware directory location of %s address: 0x%08x\n", - ctx.abs_address == 1 ? "absolute" : "relative", RUN_CURRENT(ctx)); + ctx.address_mode == 0 ? "absolute" : "relative", RUN_CURRENT(ctx));
integrate_firmwares(&ctx, amd_romsig, amd_fw_table);
ctx.current = ALIGN(ctx.current, 0x10000U); /* TODO: is it necessary? */ + ctx.current_table = 0;
if (cb_config.multi_level) { /* Do 2nd PSP directory followed by 1st */ diff --git a/util/amdfwtool/amdfwtool.h b/util/amdfwtool/amdfwtool.h index a7ac7c1..ad5056f 100644 --- a/util/amdfwtool/amdfwtool.h +++ b/util/amdfwtool/amdfwtool.h @@ -118,7 +118,16 @@ uint32_t cookie; uint32_t checksum; uint32_t num_entries; - uint32_t additional_info; + union { + uint32_t additional_info; + struct { + uint32_t dir_size:10; + uint32_t spi_block_size:4; + uint32_t base_addr:15; + uint32_t address_mode:2; + uint32_t not_used:1; + } __attribute__((packed)) additional_info_fields; + }; } __attribute__((packed, aligned(16))) psp_directory_header;
typedef struct _psp_directory_entry { @@ -126,7 +135,8 @@ uint8_t subprog; uint16_t rsvd; uint32_t size; - uint64_t addr; /* or a value in some cases */ + uint64_t addr:62; /* or a value in some cases */ + uint64_t address_mode:2; } __attribute__((packed)) psp_directory_entry;
typedef struct _psp_directory_table { @@ -161,7 +171,16 @@ uint32_t cookie; uint32_t checksum; uint32_t num_entries; - uint32_t additional_info; + union { + uint32_t additional_info; + struct { + uint32_t dir_size:10; + uint32_t spi_block_size:4; + uint32_t base_addr:15; + uint32_t address_mode:2; + uint32_t not_used:1; + } __attribute__((packed)) additional_info_fields; + }; } __attribute__((packed, aligned(16))) bios_directory_hdr;
typedef struct _bios_directory_entry { @@ -174,7 +193,8 @@ int inst:4; uint8_t subprog; /* b[7:3] reserved */ uint32_t size; - uint64_t source; + uint64_t source:62; + uint64_t address_mode:2; uint64_t dest; } __attribute__((packed)) bios_directory_entry;