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/+/59384
to review the following change.
Change subject: amdfwtool: Add ISH header support for A/B recovery layout ......................................................................
amdfwtool: Add ISH header support for A/B recovery layout
ISH is newly for Mendocino. The rom layout for A/B recovery with ISH: EFS -> PSP L1 0x48 -> ISH A -> PSP L2 A -> BIOS L2 A 0x4A -> ISH B -> PSP L2 B -> BIOS L2 B
Change-Id: Ib0690cde1dce949514c7aacebe13096b7814ceff Signed-off-by: Zheng Bao fishbaozi@gmail.com
amdfwtool: ish
Change-Id: I4710df4098c08007d9aa8f009595fdbf594a7b7b Signed-off-by: Zheng Bao fishbaozi@gmail.com --- M util/amdfwtool/Makefile M util/amdfwtool/Makefile.inc M util/amdfwtool/amdfwtool.c M util/amdfwtool/amdfwtool.h 4 files changed, 77 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/59384/1
diff --git a/util/amdfwtool/Makefile b/util/amdfwtool/Makefile index 1805459..7b77935 100644 --- a/util/amdfwtool/Makefile +++ b/util/amdfwtool/Makefile @@ -5,7 +5,7 @@ SRC = amdfwtool.c data_parse.c OBJ = $(SRC:%.c=%.o) TARGET = amdfwtool -WERROR=-Werror -Wno-unused-function +WERROR=-Werror CFLAGS=-O2 -Wall -Wextra -Wshadow ${WERROR}
all: $(TARGET) diff --git a/util/amdfwtool/Makefile.inc b/util/amdfwtool/Makefile.inc index f772299..4b7db76 100644 --- a/util/amdfwtool/Makefile.inc +++ b/util/amdfwtool/Makefile.inc @@ -2,7 +2,7 @@
amdfwtoolobj = amdfwtool.o data_parse.o
-AMDFWTOOLCFLAGS=-O2 -Wall -Wextra -Wshadow -Werror -Wno-unused-function +AMDFWTOOLCFLAGS=-O2 -Wall -Wextra -Wshadow -Werror
$(objutil)/amdfwtool/%.o: $(top)/util/amdfwtool/%.c # $(HEADER) $(HOSTCC) $(AMDFWTOOLCFLAGS) $(HOSTCFLAGS) -c -o $@ $< diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index dede243..490c613 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -407,6 +407,15 @@ return ptr; }
+static void *new_ish_dir(context *ctx) +{ + void *ptr; + ctx->current = ALIGN(ctx->current, TABLE_ALIGNMENT); + ptr = BUFF_CURRENT(*ctx); + ctx->current += TABLE_ALIGNMENT; + return ptr; +} + #if PSP_COMBO static void *new_combo_dir(context *ctx) { @@ -657,7 +666,8 @@ }
static void integrate_psp_ab(context *ctx, psp_directory_table *pspdir, - psp_directory_table *pspdir2, amd_fw_type ab) + psp_directory_table *pspdir2, ish_directory_table *ish, + amd_fw_type ab, enum platform soc_id) { uint32_t count; uint32_t current_table_save; @@ -666,14 +676,44 @@ ctx->current_table = 0; count = pspdir->header.num_entries; assert_fw_entry(count, MAX_PSP_ENTRIES, ctx); + if (ish != NULL) { + switch (ab) { + case AMD_FW_RECOVERYAB_A: + ish->pl2_location = BUFF_TO_RUN_MODE(*ctx, pspdir2, 1); + ish->boot_priority = 0xFFFFFFFF; + ish->update_retry_count = 0xf; + ish->glitch_retry_count = 0; + ish->psp_id = get_psp_id(soc_id); + ish->checksum = fletcher32(&ish->boot_priority, + sizeof(ish_directory_table) - sizeof(uint32_t)); + break; + case AMD_FW_RECOVERYAB_B: + ish->pl2_location = BUFF_TO_RUN_MODE(*ctx, pspdir2, 1); + ish->boot_priority = 0x2; + ish->update_retry_count = 0xf; + ish->glitch_retry_count = 0; + ish->psp_id = get_psp_id(soc_id); + ish->checksum = fletcher32(&ish->boot_priority, + sizeof(ish_directory_table) - sizeof(uint32_t)); + break; + default: + break; + } + } pspdir->entries[count].type = (uint8_t)ab; pspdir->entries[count].subprog = 0; pspdir->entries[count].rsvd = 0; - pspdir->entries[count].addr = BUFF_TO_RUN_MODE(*ctx, pspdir2, 1); - pspdir->entries[count].address_mode = SET_ADDR_MODE(1, pspdir); - pspdir->entries[count].size = pspdir2->header.num_entries * + if (ish != NULL) { + pspdir->entries[count].addr = BUFF_TO_RUN_MODE(*ctx, ish, 1); + pspdir->entries[count].address_mode = SET_ADDR_MODE(1, pspdir); + pspdir->entries[count].size = TABLE_ALIGNMENT; + } else { + pspdir->entries[count].addr = BUFF_TO_RUN_MODE(*ctx, pspdir2, 1); + pspdir->entries[count].address_mode = SET_ADDR_MODE(1, pspdir); + pspdir->entries[count].size = pspdir2->header.num_entries * sizeof(psp_directory_entry) + sizeof(psp_directory_header); + }
count++; pspdir->header.num_entries = count; @@ -686,6 +726,7 @@ psp_directory_table *pspdir2_b, amd_fw_entry *fw_table, uint32_t cookie, + enum platform soc_id, amd_cb_config *cb_config) { ssize_t bytes; @@ -693,6 +734,7 @@ int level; uint32_t current_table_save; bool recovery_ab = cb_config->recovery_ab; + ish_directory_table *ish_a_dir = NULL, *ish_b_dir = NULL;
/* This function can create a primary table, a secondary table, or a * flattened table which contains all applicable types. These if-else @@ -798,10 +840,17 @@ }
if (recovery_ab && (pspdir2 != NULL)) { + if (cb_config->need_ish) { /* Need ISH */ + ish_a_dir = new_ish_dir(ctx); + if (pspdir2_b != NULL) + ish_b_dir = new_ish_dir(ctx); + } pspdir->header.num_entries = count; - integrate_psp_ab(ctx, pspdir, pspdir2, AMD_FW_RECOVERYAB_A); + integrate_psp_ab(ctx, pspdir, pspdir2, ish_a_dir, + AMD_FW_RECOVERYAB_A, soc_id); if (pspdir2_b != NULL) - integrate_psp_ab(ctx, pspdir, pspdir2_b, AMD_FW_RECOVERYAB_B); + integrate_psp_ab(ctx, pspdir, pspdir2_b, ish_b_dir, + AMD_FW_RECOVERYAB_B, soc_id); count = pspdir->header.num_entries; } else if (pspdir2 != NULL) { assert_fw_entry(count, MAX_PSP_ENTRIES, ctx); @@ -1430,6 +1479,7 @@ cb_config.s0i3 = false; cb_config.multi_level = false; cb_config.recovery_ab = false; + cb_config.need_ish = false;
while (1) { int optindex = 0; @@ -1657,6 +1707,9 @@ retval = 1; }
+ if (soc_id == PLATFORM_MENDOCINO && cb_config.recovery_ab) + cb_config.need_ish = true; + if (retval) { usage(); return retval; @@ -1744,25 +1797,25 @@ /* Do 2nd PSP directory followed by 1st */ pspdir2 = new_psp_dir(&ctx, cb_config.multi_level); integrate_psp_firmwares(&ctx, pspdir2, NULL, NULL, - amd_psp_fw_table, PSPL2_COOKIE, &cb_config); + amd_psp_fw_table, PSPL2_COOKIE, soc_id, &cb_config); if (cb_config.recovery_ab) { /* B is same as above directories for A */ /* Skip creating pspdir2_b here to save flash space. Related * biosdir2_b will be skipped automatically. */ pspdir2_b = new_psp_dir(&ctx, cb_config.multi_level); integrate_psp_firmwares(&ctx, pspdir2_b, NULL, NULL, - amd_psp_fw_table, PSPL2_COOKIE, &cb_config); + amd_psp_fw_table, PSPL2_COOKIE, soc_id, &cb_config); } else { pspdir2_b = NULL; /* More explicitly */ } pspdir = new_psp_dir(&ctx, cb_config.multi_level); integrate_psp_firmwares(&ctx, pspdir, pspdir2, pspdir2_b, - amd_psp_fw_table, PSP_COOKIE, &cb_config); + amd_psp_fw_table, PSP_COOKIE, soc_id, &cb_config); } else { /* flat: PSP 1 cookie and no pointer to 2nd table */ pspdir = new_psp_dir(&ctx, cb_config.multi_level); integrate_psp_firmwares(&ctx, pspdir, NULL, NULL, - amd_psp_fw_table, PSP_COOKIE, &cb_config); + amd_psp_fw_table, PSP_COOKIE, soc_id, &cb_config); }
if (comboable) diff --git a/util/amdfwtool/amdfwtool.h b/util/amdfwtool/amdfwtool.h index cc270a4..face3bf 100644 --- a/util/amdfwtool/amdfwtool.h +++ b/util/amdfwtool/amdfwtool.h @@ -228,6 +228,17 @@ int level; } amd_bios_entry;
+typedef struct _ish_directory_table { + uint32_t checksum; + uint32_t boot_priority; + uint32_t update_retry_count; + uint8_t glitch_retry_count; + uint8_t glitch_higherbits_reserved[3]; + uint32_t pl2_location; + uint32_t psp_id; + uint32_t slot_max_size; + uint32_t reserved; +} __attribute__((packed)) ish_directory_table; #define EMBEDDED_FW_SIGNATURE 0x55aa55aa #define PSP_COOKIE 0x50535024 /* 'PSP$' */ #define PSPL2_COOKIE 0x324c5024 /* '2LP$' */ @@ -257,6 +268,7 @@ bool multi_level; bool s0i3; bool recovery_ab; + bool need_ish; } amd_cb_config;
void register_fw_fuse(char *str);
-
Bao Zheng (Code Review)