From ad06bae7b195497227fbb54e5d073740db7616c4 Mon Sep 17 00:00:00 2001 From: Karthikeyan Ramasubramanian Date: Fri, 8 Apr 2022 14:19:55 -0600 Subject: [PATCH] util/amdfwtool: Maintain one copy of PSP Level2 entries AMDFWtool maintains 2 copies of PSP Level2 entries - one in primary slot A (Type 0x48) and another in backup slot B (Type 0x4A). On boards which use VBOOT with 2 RW firmware slots, maintaining 2 copies of PSP Level2 entries in each FW slot is redundant and space-consuming. Introduce option to maintain only one copy of PSP Level2 entries and point to it from both slots A & B. BUG=None TEST=Build and boot to OS in Skyrim. Ensure that only one copy is added to each FW slot. This achieved a space saving of 1.5 MB in each FW slot. Before: apu/amdfw 0x415fc0 raw 3043328 none After: apu/amdfw 0x415fc0 raw 1556480 none Signed-off-by: Karthikeyan Ramasubramanian Change-Id: I06eef8e14b9c14db1d02b621c2f7207188d86326 Reviewed-on: https://review.coreboot.org/c/coreboot/+/63509 Tested-by: build bot (Jenkins) Reviewed-by: Felix Held --- util/amdfwtool/amdfwtool.c | 23 +++++++++++++++++++---- util/amdfwtool/amdfwtool.h | 1 + 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 646a744bc0..af89cadb20 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -857,6 +857,10 @@ static void integrate_psp_firmwares(context *ctx, if (pspdir2_b != NULL) integrate_psp_ab(ctx, pspdir, pspdir2_b, ish_b_dir, AMD_FW_RECOVERYAB_B, soc_id); + else + integrate_psp_ab(ctx, pspdir, pspdir2, 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); @@ -1202,6 +1206,7 @@ enum { AMDFW_OPT_GEC, AMDFW_OPT_COMBO, AMDFW_OPT_RECOVERY_AB, + AMDFW_OPT_RECOVERY_AB_SINGLE_COPY, AMDFW_OPT_MULTILEVEL, AMDFW_OPT_NVRAM, @@ -1250,6 +1255,7 @@ static struct option long_options[] = { /* PSP Directory Table items */ {"combo-capable", no_argument, 0, AMDFW_OPT_COMBO }, {"recovery-ab", no_argument, 0, AMDFW_OPT_RECOVERY_AB }, + {"recovery-ab-single-copy", no_argument, 0, AMDFW_OPT_RECOVERY_AB_SINGLE_COPY }, {"multilevel", no_argument, 0, AMDFW_OPT_MULTILEVEL }, {"nvram", required_argument, 0, AMDFW_OPT_NVRAM }, {"soft-fuse", required_argument, 0, AMDFW_OPT_FUSE }, @@ -1532,6 +1538,7 @@ int main(int argc, char **argv) cb_config.multi_level = false; cb_config.recovery_ab = false; cb_config.need_ish = false; + cb_config.recovery_ab_single_copy = false; while (1) { int optindex = 0; @@ -1560,6 +1567,10 @@ int main(int argc, char **argv) case AMDFW_OPT_RECOVERY_AB: cb_config.recovery_ab = true; break; + case AMDFW_OPT_RECOVERY_AB_SINGLE_COPY: + cb_config.recovery_ab = true; + cb_config.recovery_ab_single_copy = true; + break; case AMDFW_OPT_MULTILEVEL: cb_config.multi_level = true; break; @@ -1864,14 +1875,18 @@ int main(int argc, char **argv) pspdir2 = new_psp_dir(&ctx, cb_config.multi_level); integrate_psp_firmwares(&ctx, pspdir2, NULL, NULL, 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. */ + if (cb_config.recovery_ab && !cb_config.recovery_ab_single_copy) { + /* Create a copy of PSP Directory 2 in the backup slot B. + Related biosdir2_b copy will be created later. */ pspdir2_b = new_psp_dir(&ctx, cb_config.multi_level); integrate_psp_firmwares(&ctx, pspdir2_b, NULL, NULL, amd_psp_fw_table, PSPL2_COOKIE, soc_id, &cb_config); } else { + /* + * Either the platform is using only one slot or 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 = NULL; /* More explicitly */ } pspdir = new_psp_dir(&ctx, cb_config.multi_level); diff --git a/util/amdfwtool/amdfwtool.h b/util/amdfwtool/amdfwtool.h index d2571e58b3..22386386c9 100644 --- a/util/amdfwtool/amdfwtool.h +++ b/util/amdfwtool/amdfwtool.h @@ -279,6 +279,7 @@ typedef struct _amd_cb_config { bool second_gen; bool have_mb_spl; bool recovery_ab; + bool recovery_ab_single_copy; bool need_ish; } amd_cb_config;