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/+/59308
to review the following change.
Change subject: amdfwtool: Use address mode ......................................................................
amdfwtool: Use address mode
It is the expanding mode for simple relative address mode, for which address_mode equals 1.
Change-Id: I29a03f4381cd0507e2b2e3b359111e3375a73de1 Signed-off-by: Zheng Bao fishbaozi@gmail.com --- M util/amdfwtool/amdfwtool.c M util/amdfwtool/amdfwtool.h 2 files changed, 62 insertions(+), 15 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/59308/1
diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 35b28ab..8f6728c 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -343,12 +343,18 @@ 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)) #define RUN_CURRENT(ctx) RUN_OFFSET((ctx), (ctx).current) #define BUFF_OFFSET(ctx, offset) ((void *)((ctx).rom + (offset))) #define BUFF_CURRENT(ctx) BUFF_OFFSET((ctx), (ctx).current) @@ -380,7 +386,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 +439,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 +457,15 @@ 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.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 +651,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 +669,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 +686,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 = pspdir->header.additional_info_fields.address_mode; pspdir->entries[count].subprog = fw_table[i].subprog; pspdir->entries[count].rsvd = 0; ctx->current = ALIGN(ctx->current + 4096, 0x100U); @@ -683,6 +697,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) @@ -705,6 +720,7 @@ pspdir->entries[count].size = ALIGN(bytes, ERASE_ALIGNMENT); pspdir->entries[count].addr = RUN_CURRENT(*ctx); + pspdir->entries[count].address_mode = 1;
ctx->current = ALIGN(ctx->current + bytes, BLOB_ERASE_ALIGNMENT); @@ -722,6 +738,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 = pspdir->header.additional_info_fields.address_mode;
ctx->current = ALIGN(ctx->current + bytes, BLOB_ALIGNMENT); @@ -741,10 +758,12 @@ * sizeof(psp_directory_entry);
pspdir->entries[count].addr = BUFF_TO_RUN(*ctx, pspdir2); + pspdir->entries[count].address_mode = 1; 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 +780,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; @@ -923,6 +944,7 @@ /* 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 = biosdir->header.additional_info_fields.address_mode; biosdir->entries[count].size = ALIGN( fw_table[i].size, ERASE_ALIGNMENT); memset(BUFF_CURRENT(*ctx), 0xff, @@ -941,6 +963,8 @@
/* 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 = 1; + biosdir->entries[count].dest = fw_table[i].dest; biosdir->entries[count].size = fw_table[i].size;
@@ -977,6 +1001,7 @@
biosdir->entries[count].size = (uint32_t)bytes; biosdir->entries[count].source = RUN_CURRENT(*ctx); + biosdir->entries[count].address_mode = biosdir->header.additional_info_fields.address_mode;
ctx->current = ALIGN(ctx->current + bytes, 0x100U); break; @@ -1598,15 +1623,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 */ @@ -1658,6 +1684,7 @@ integrate_bios_firmwares(&ctx, biosdir, NULL, amd_bios_table, BDT1_COOKIE, &cb_config); } + switch (soc_id) { case PLATFORM_RENOIR: case PLATFORM_LUCIENNE: 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;
-
Bao Zheng (Code Review)