coreboot-gerrit March 2024

coreboot-gerrit@coreboot.org

1 participants
3668 discussions

[XS] Change in coreboot[main]: soc/qualcomm/sc7{1,2}80: Increase romstage/verstage section for clang
by Arthur Heymans (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Arthur Heymans. Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80639?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/qualcomm/sc7{1,2}80: Increase romstage/verstage section for clang ...................................................................... soc/qualcomm/sc7{1,2}80: Increase romstage/verstage section for clang Clang builds slightly larger binaries so increase the section. The qcsdi is used for an external blob that is currently not in use so reducing the size is fine for now. Change-Id: Ide01233f209613678c5408f1afab19415c1071be Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/soc/qualcomm/sc7180/memlayout.ld M src/soc/qualcomm/sc7280/memlayout.ld 2 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/80639/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/80639?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ide01233f209613678c5408f1afab19415c1071be Gerrit-Change-Number: 80639 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Shelley Chen <shchen(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

[XS] Change in coreboot[main]: vendorcode/cavium: Use unsigned integers in struct bitfields
by Arthur Heymans (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Arthur Heymans, Felix Singer, Paul Menzel. Hello Paul Menzel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80638?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by Paul Menzel, Verified+1 by build bot (Jenkins) Change subject: vendorcode/cavium: Use unsigned integers in struct bitfields ...................................................................... vendorcode/cavium: Use unsigned integers in struct bitfields Bitfields with signed integers are not valid C code. This fixes compilation with clang v16.0.6. Change-Id: I0b2add2f1078a88347fea7dc65d422d0e5a210a1 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/vendorcode/cavium/include/bdk/libbdk-hal/if/bdk-if.h 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/38/80638/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80638?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0b2add2f1078a88347fea7dc65d422d0e5a210a1 Gerrit-Change-Number: 80638 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: util/xcompile: Add target architecture to CPPFLAGS
by Arthur Heymans (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Arthur Heymans, Martin L Roth. Hello Felix Singer, Julius Werner, Martin L Roth, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/75031?usp=email to look at the new patch set (#12). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: util/xcompile: Add target architecture to CPPFLAGS ...................................................................... util/xcompile: Add target architecture to CPPFLAGS In order to preprocess linker scripts the target architecture needs to be specified. With clang this needs to be set via a cli argument. Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Change-Id: I4340681e30059d6f18a49a49937668cd3dd39ce1 --- M src/arch/arm/armv7/Makefile.mk M src/arch/arm64/armv8/Makefile.mk M util/xcompile/xcompile 3 files changed, 7 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/75031/12 -- To view, visit https://review.coreboot.org/c/coreboot/+/75031?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4340681e30059d6f18a49a49937668cd3dd39ce1 Gerrit-Change-Number: 75031 Gerrit-PatchSet: 12 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: amdfwtool: Set the cookie when the table header is created
by Felix Held (Code Review) 20 Mar '24

20 Mar '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81254?usp=email ) Change subject: amdfwtool: Set the cookie when the table header is created ...................................................................... amdfwtool: Set the cookie when the table header is created When the table is created, the cookie is known. When the packing going on, the cookie in header can be checked to see where we are. TEST=Identical test on all AMD SOC platform Change-Id: I300e30292c68a14b44c637b26a13b308dc9c0388 Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81254 Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M util/amdfwtool/amdfwtool.c 1 file changed, 20 insertions(+), 20 deletions(-) Approvals: Felix Held: Looks good to me, approved build bot (Jenkins): Verified diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index 0308b84..72f0c5f 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -473,7 +473,7 @@ set_current_pointer(ctx, ALIGN_UP(ctx->current + add, align)); } -static void *new_psp_dir(context *ctx, int multi) +static void *new_psp_dir(context *ctx, int multi, uint32_t cookie) { void *ptr; @@ -488,6 +488,7 @@ adjust_current_pointer(ctx, 0, TABLE_ALIGNMENT); ptr = BUFF_CURRENT(*ctx); + ((psp_directory_header *)ptr)->cookie = cookie; ((psp_directory_header *)ptr)->num_entries = 0; ((psp_directory_header *)ptr)->additional_info = 0; ((psp_directory_header *)ptr)->additional_info_fields.address_mode = ctx->address_mode; @@ -507,23 +508,26 @@ return ptr; } -static void *new_combo_dir(context *ctx) +static void *new_combo_dir(context *ctx, uint32_t cookie) { void *ptr; adjust_current_pointer(ctx, 0, TABLE_ALIGNMENT); ptr = BUFF_CURRENT(*ctx); + ((psp_combo_header *)ptr)->cookie = cookie; adjust_current_pointer(ctx, sizeof(psp_combo_header) + MAX_COMBO_ENTRIES * sizeof(psp_combo_entry), 1); return ptr; } -static void fill_dir_header(void *directory, uint32_t count, uint32_t cookie, context *ctx) +static void fill_dir_header(void *directory, uint32_t count, context *ctx) { psp_combo_directory *cdir = directory; psp_directory_table *dir = directory; bios_directory_table *bdir = directory; + /* The cookies have same offsets. */ + uint32_t cookie = ((psp_directory_table *)directory)->header.cookie; uint32_t table_size = 0; if (ctx == NULL || directory == NULL) { @@ -537,7 +541,6 @@ switch (cookie) { case PSP2_COOKIE: case BHD2_COOKIE: - cdir->header.cookie = cookie; /* lookup mode is hardcoded for now. */ cdir->header.lookup = 1; cdir->header.num_entries = count; @@ -565,7 +568,6 @@ dir->header.additional_info_fields.dir_size = table_size / TABLE_ALIGNMENT; } - dir->header.cookie = cookie; dir->header.num_entries = count; dir->header.additional_info_fields.spi_block_size = 1; dir->header.additional_info_fields.base_addr = 0; @@ -588,7 +590,6 @@ bdir->header.additional_info_fields.dir_size = table_size / TABLE_ALIGNMENT; } - bdir->header.cookie = cookie; bdir->header.num_entries = count; bdir->header.additional_info_fields.spi_block_size = 1; bdir->header.additional_info_fields.base_addr = 0; @@ -930,7 +931,7 @@ SET_ADDR_MODE(pspdir, AMD_ADDR_REL_BIOS); count++; } - fill_dir_header(pspdir, count, PSP_COOKIE, ctx); + fill_dir_header(pspdir, count, ctx); ctx->current_table = current_table_save; } @@ -954,7 +955,7 @@ * is passed, clearly a 2nd-level table is intended. However, a * 1st-level cookie may indicate level 1 or flattened. */ - pspdir = new_psp_dir(ctx, cb_config->multi_level); + pspdir = new_psp_dir(ctx, cb_config->multi_level, cookie); if (cookie == PSP_COOKIE) ctx->pspdir = pspdir; @@ -1080,7 +1081,7 @@ } } - fill_dir_header(pspdir, count, cookie, ctx); + fill_dir_header(pspdir, count, ctx); ctx->current_table = current_table_save; } @@ -1111,11 +1112,11 @@ if (index == count) count++; - fill_dir_header(pspdir, count, pspdir->header.cookie, ctx); + fill_dir_header(pspdir, count, ctx); ctx->current_table = current_table_save; } -static void *new_bios_dir(context *ctx, bool multi) +static void *new_bios_dir(context *ctx, bool multi, uint32_t cookie) { void *ptr; @@ -1129,6 +1130,7 @@ else adjust_current_pointer(ctx, 0, TABLE_ALIGNMENT); ptr = BUFF_CURRENT(*ctx); + ((bios_directory_hdr *) ptr)->cookie = cookie; ((bios_directory_hdr *) ptr)->additional_info = 0; ((bios_directory_hdr *) ptr)->additional_info_fields.address_mode = ctx->address_mode; adjust_current_pointer(ctx, @@ -1236,7 +1238,7 @@ ctx->biosdir->entries[count].ro = 0; count++; } - fill_dir_header(ctx->biosdir, count, BHD_COOKIE, ctx); + fill_dir_header(ctx->biosdir, count, ctx); ctx->current_table = current_table_save; } static void integrate_bios_firmwares(context *ctx, @@ -1253,7 +1255,7 @@ uint32_t current_table_save; bios_directory_table *biosdir; - biosdir = new_bios_dir(ctx, cb_config->multi_level); + biosdir = new_bios_dir(ctx, cb_config->multi_level, cookie); if (cookie == BHD_COOKIE) ctx->biosdir = biosdir; @@ -1459,7 +1461,7 @@ count++; } - fill_dir_header(biosdir, count, cookie, ctx); + fill_dir_header(biosdir, count, ctx); ctx->current_table = current_table_save; } @@ -1657,12 +1659,12 @@ cb_config.soc_id); if (cb_config.use_combo) { - ctx.psp_combo_dir = new_combo_dir(&ctx); + ctx.psp_combo_dir = new_combo_dir(&ctx, PSP2_COOKIE); adjust_current_pointer(&ctx, 0, 0x1000U); if (!cb_config.recovery_ab) - ctx.bhd_combo_dir = new_combo_dir(&ctx); + ctx.bhd_combo_dir = new_combo_dir(&ctx, BHD2_COOKIE); } combo_index = 0; @@ -1747,7 +1749,7 @@ ctx.psp_combo_dir->entries[combo_index].lvl2_addr = BUFF_TO_RUN_MODE(ctx, ctx.pspdir, AMD_ADDR_REL_BIOS); - fill_dir_header(ctx.psp_combo_dir, combo_index + 1, PSP2_COOKIE, &ctx); + fill_dir_header(ctx.psp_combo_dir, combo_index + 1, &ctx); } if (have_bios_tables(amd_bios_table)) { @@ -1793,9 +1795,7 @@ ctx.bhd_combo_dir->entries[combo_index].lvl2_addr = BUFF_TO_RUN_MODE(ctx, ctx.biosdir, AMD_ADDR_REL_BIOS); - fill_dir_header(ctx.bhd_combo_dir, combo_index + 1, - BHD2_COOKIE, &ctx); - + fill_dir_header(ctx.bhd_combo_dir, combo_index + 1, &ctx); } } if (cb_config.debug) -- To view, visit https://review.coreboot.org/c/coreboot/+/81254?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I300e30292c68a14b44c637b26a13b308dc9c0388 Gerrit-Change-Number: 81254 Gerrit-PatchSet: 3 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: amdfwtool: Move the header creation into integration function
by Felix Held (Code Review) 20 Mar '24

20 Mar '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81253?usp=email ) Change subject: amdfwtool: Move the header creation into integration function ...................................................................... amdfwtool: Move the header creation into integration function Before every integration there is a header creation. We can put them together. And the parameters for PSP/BIOS tables are useless. TEST=Identical test on all AMD SOC platform Change-Id: Ia9d78bb8145855203048208fcd67f8b9cd9d3199 Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81253 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- M util/amdfwtool/amdfwtool.c 1 file changed, 33 insertions(+), 26 deletions(-) Approvals: Felix Held: Looks good to me, approved build bot (Jenkins): Verified diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index f85c36d..0308b84 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -935,9 +935,6 @@ } static void integrate_psp_firmwares(context *ctx, - psp_directory_table *pspdir, - psp_directory_table *pspdir2, - psp_directory_table *pspdir2_b, amd_fw_entry *fw_table, uint32_t cookie, amd_cb_config *cb_config) @@ -946,6 +943,7 @@ unsigned int i, count; int level; uint32_t size; + psp_directory_table *pspdir; uint64_t addr; uint32_t current_table_save; bool recovery_ab = cb_config->recovery_ab; @@ -956,6 +954,17 @@ * is passed, clearly a 2nd-level table is intended. However, a * 1st-level cookie may indicate level 1 or flattened. */ + pspdir = new_psp_dir(ctx, cb_config->multi_level); + + if (cookie == PSP_COOKIE) + ctx->pspdir = pspdir; + else if (cookie == PSPL2_COOKIE) { + if (ctx->pspdir2 == NULL) + ctx->pspdir2 = pspdir; + else if (ctx->pspdir2_b == NULL) + ctx->pspdir2_b = pspdir; + } + if (!cb_config->multi_level) level = PSP_BOTH; else if (cookie == PSPL2_COOKIE) @@ -1071,8 +1080,6 @@ } } - (void) (pspdir2); /* TODO: To be removed in next CLs */ - (void) (pspdir2_b); fill_dir_header(pspdir, count, cookie, ctx); ctx->current_table = current_table_save; } @@ -1233,8 +1240,6 @@ ctx->current_table = current_table_save; } static void integrate_bios_firmwares(context *ctx, - bios_directory_table *biosdir, - bios_directory_table *biosdir2, amd_bios_entry *fw_table, uint32_t cookie, amd_cb_config *cb_config) @@ -1246,6 +1251,18 @@ uint32_t size; uint64_t source; uint32_t current_table_save; + bios_directory_table *biosdir; + + biosdir = new_bios_dir(ctx, cb_config->multi_level); + + if (cookie == BHD_COOKIE) + ctx->biosdir = biosdir; + else if (cookie == BHDL2_COOKIE) { + if (ctx->biosdir2 == NULL) + ctx->biosdir2 = biosdir; + else if (ctx->biosdir2_b == NULL) + ctx->biosdir2_b = biosdir; + } /* This function can create a primary table, a secondary table, or a * flattened table which contains all applicable types. These if-else @@ -1377,7 +1394,7 @@ break; case AMD_BIOS_BIN: /* Don't make a 2nd copy, point to the same one */ - if (level == BDT_LVL1 && locate_bdt2_bios(biosdir2, &source, &size)) { + if (level == BDT_LVL1 && locate_bdt2_bios(ctx->biosdir2, &source, &size)) { biosdir->entries[count].source = source; biosdir->entries[count].address_mode = SET_ADDR_MODE(biosdir, AMD_ADDR_REL_BIOS); @@ -1692,14 +1709,12 @@ if (cb_config.multi_level) { /* Do 2nd PSP directory followed by 1st */ - ctx.pspdir2 = new_psp_dir(&ctx, cb_config.multi_level); - integrate_psp_firmwares(&ctx, ctx.pspdir2, NULL, NULL, + integrate_psp_firmwares(&ctx, amd_psp_fw_table, PSPL2_COOKIE, &cb_config); 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. */ - ctx.pspdir2_b = new_psp_dir(&ctx, cb_config.multi_level); - integrate_psp_firmwares(&ctx, ctx.pspdir2_b, NULL, NULL, + integrate_psp_firmwares(&ctx, amd_psp_fw_table, PSPL2_COOKIE, &cb_config); } else { /* @@ -1712,14 +1727,12 @@ */ ctx.pspdir2_b = NULL; /* More explicitly */ } - ctx.pspdir = new_psp_dir(&ctx, cb_config.multi_level); - integrate_psp_firmwares(&ctx, ctx.pspdir, ctx.pspdir2, ctx.pspdir2_b, + integrate_psp_firmwares(&ctx, amd_psp_fw_table, PSP_COOKIE, &cb_config); integrate_psp_levels(&ctx, &cb_config); } else { /* flat: PSP 1 cookie and no pointer to 2nd table */ - ctx.pspdir = new_psp_dir(&ctx, cb_config.multi_level); - integrate_psp_firmwares(&ctx, ctx.pspdir, NULL, NULL, + integrate_psp_firmwares(&ctx, amd_psp_fw_table, PSP_COOKIE, &cb_config); } @@ -1740,15 +1753,11 @@ if (have_bios_tables(amd_bios_table)) { if (cb_config.multi_level) { /* Do 2nd level BIOS directory followed by 1st */ - ctx.biosdir2 = new_bios_dir(&ctx, cb_config.multi_level); - - integrate_bios_firmwares(&ctx, ctx.biosdir2, NULL, + integrate_bios_firmwares(&ctx, amd_bios_table, BHDL2_COOKIE, &cb_config); if (cb_config.recovery_ab) { if (ctx.pspdir2_b != NULL) { - ctx.biosdir2_b = new_bios_dir(&ctx, - cb_config.multi_level); - integrate_bios_firmwares(&ctx, ctx.biosdir2_b, NULL, + integrate_bios_firmwares(&ctx, amd_bios_table, BHDL2_COOKIE, &cb_config); } @@ -1759,15 +1768,13 @@ ctx.biosdir2_b, AMD_FW_BIOS_TABLE, TABLE_ALIGNMENT); } else { - ctx.biosdir = new_bios_dir(&ctx, cb_config.multi_level); - integrate_bios_firmwares(&ctx, ctx.biosdir, ctx.biosdir2, + integrate_bios_firmwares(&ctx, amd_bios_table, BHD_COOKIE, &cb_config); integrate_bios_levels(&ctx); } } else { /* flat: BHD1 cookie and no pointer to 2nd table */ - ctx.biosdir = new_bios_dir(&ctx, cb_config.multi_level); - integrate_bios_firmwares(&ctx, ctx.biosdir, NULL, + integrate_bios_firmwares(&ctx, amd_bios_table, BHD_COOKIE, &cb_config); } if (!cb_config.use_combo) { -- To view, visit https://review.coreboot.org/c/coreboot/+/81253?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia9d78bb8145855203048208fcd67f8b9cd9d3199 Gerrit-Change-Number: 81253 Gerrit-PatchSet: 4 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: amdfwtool: Add functions to link all the tables
by Felix Held (Code Review) 20 Mar '24

20 Mar '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81252?usp=email ) Change subject: amdfwtool: Add functions to link all the tables ...................................................................... amdfwtool: Add functions to link all the tables The purpose of integration function is to pack the FWs into table. We need to remove other process. Create a dedicate function to link all the tables together. And this linking function is only called when both the level 1 and level 2 directory are created. This simplifies the main function and logic. TEST=Identical test on all AMD SOC platform Change-Id: Ieaf97208e943c79d7b76ea62eea9355138c220b9 Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81252 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- M util/amdfwtool/amdfwtool.c 1 file changed, 89 insertions(+), 61 deletions(-) Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index aedd326..f85c36d 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -880,6 +880,60 @@ ctx->current_table = current_table_save; } +static void integrate_psp_levels(context *ctx, + amd_cb_config *cb_config) +{ + uint32_t current_table_save; + bool recovery_ab = cb_config->recovery_ab; + unsigned int count; + psp_directory_table *pspdir, *pspdir2, *pspdir2_b; + bool use_only_a = (cb_config->soc_id == PLATFORM_PHOENIX); /* TODO: b:285390041 */ + + pspdir = ctx->pspdir; + pspdir2 = ctx->pspdir2; + pspdir2_b = ctx->pspdir2_b; + count = pspdir->header.num_entries; + + current_table_save = ctx->current_table; + ctx->current_table = BUFF_TO_RUN_MODE(*ctx, pspdir, AMD_ADDR_REL_BIOS); + if (recovery_ab && (pspdir2 != NULL)) { + if (cb_config->need_ish) { /* Need ISH */ + ctx->ish_a_dir = new_ish_dir(ctx); + if (pspdir2_b != NULL) + ctx->ish_b_dir = new_ish_dir(ctx); + } + pspdir->header.num_entries = count; + integrate_psp_ab(ctx, pspdir, pspdir2, ctx->ish_a_dir, + AMD_FW_RECOVERYAB_A, cb_config->soc_id); + if (pspdir2_b != NULL) + integrate_psp_ab(ctx, pspdir, pspdir2_b, ctx->ish_b_dir, + use_only_a ? AMD_FW_RECOVERYAB_A : AMD_FW_RECOVERYAB_B, + cb_config->soc_id); + else + integrate_psp_ab(ctx, pspdir, pspdir2, ctx->ish_a_dir, + use_only_a ? AMD_FW_RECOVERYAB_A : AMD_FW_RECOVERYAB_B, + cb_config->soc_id); + + count = pspdir->header.num_entries; + } else if (pspdir2 != NULL) { + assert_fw_entry(count, MAX_PSP_ENTRIES, ctx); + pspdir->entries[count].type = AMD_FW_L2_PTR; + pspdir->entries[count].subprog = 0; + pspdir->entries[count].rsvd = 0; + pspdir->entries[count].size = sizeof(pspdir2->header) + + pspdir2->header.num_entries + * sizeof(psp_directory_entry); + + pspdir->entries[count].addr = + BUFF_TO_RUN_MODE(*ctx, pspdir2, AMD_ADDR_REL_BIOS); + pspdir->entries[count].address_mode = + SET_ADDR_MODE(pspdir, AMD_ADDR_REL_BIOS); + count++; + } + fill_dir_header(pspdir, count, PSP_COOKIE, ctx); + ctx->current_table = current_table_save; +} + static void integrate_psp_firmwares(context *ctx, psp_directory_table *pspdir, psp_directory_table *pspdir2, @@ -895,7 +949,6 @@ uint64_t addr; uint32_t current_table_save; bool recovery_ab = cb_config->recovery_ab; - bool use_only_a = (cb_config->soc_id == PLATFORM_PHOENIX); /* TODO: b:285390041 */ /* This function can create a primary table, a secondary table, or a * flattened table which contains all applicable types. These if-else @@ -1018,43 +1071,8 @@ } } - fill_dir_header(pspdir, count, cookie, ctx); - - if (recovery_ab && (pspdir2 != NULL)) { - if (cb_config->need_ish) { /* Need ISH */ - ctx->ish_a_dir = new_ish_dir(ctx); - if (pspdir2_b != NULL) - ctx->ish_b_dir = new_ish_dir(ctx); - } - pspdir->header.num_entries = count; - integrate_psp_ab(ctx, pspdir, pspdir2, ctx->ish_a_dir, - AMD_FW_RECOVERYAB_A, cb_config->soc_id); - if (pspdir2_b != NULL) - integrate_psp_ab(ctx, pspdir, pspdir2_b, ctx->ish_b_dir, - use_only_a ? AMD_FW_RECOVERYAB_A : AMD_FW_RECOVERYAB_B, - cb_config->soc_id); - else - integrate_psp_ab(ctx, pspdir, pspdir2, ctx->ish_a_dir, - use_only_a ? AMD_FW_RECOVERYAB_A : AMD_FW_RECOVERYAB_B, - cb_config->soc_id); - - count = pspdir->header.num_entries; - } else if (pspdir2 != NULL) { - assert_fw_entry(count, MAX_PSP_ENTRIES, ctx); - pspdir->entries[count].type = AMD_FW_L2_PTR; - pspdir->entries[count].subprog = 0; - pspdir->entries[count].rsvd = 0; - pspdir->entries[count].size = sizeof(pspdir2->header) - + pspdir2->header.num_entries - * sizeof(psp_directory_entry); - - pspdir->entries[count].addr = - BUFF_TO_RUN_MODE(*ctx, pspdir2, AMD_ADDR_REL_BIOS); - pspdir->entries[count].address_mode = - SET_ADDR_MODE(pspdir, AMD_ADDR_REL_BIOS); - count++; - } - + (void) (pspdir2); /* TODO: To be removed in next CLs */ + (void) (pspdir2_b); fill_dir_header(pspdir, count, cookie, ctx); ctx->current_table = current_table_save; } @@ -1183,6 +1201,37 @@ biosdir->entries[idx].address_mode = SET_ADDR_MODE_BY_TABLE(biosdir); } +static void integrate_bios_levels(context *ctx) +{ + unsigned int count = ctx->biosdir->header.num_entries; + uint32_t current_table_save; + + current_table_save = ctx->current_table; + ctx->current_table = (char *)ctx->biosdir - ctx->rom; + + if (ctx->biosdir2) { + assert_fw_entry(count, MAX_BIOS_ENTRIES, ctx); + ctx->biosdir->entries[count].type = AMD_BIOS_L2_PTR; + ctx->biosdir->entries[count].region_type = 0; + ctx->biosdir->entries[count].size = + + MAX_BIOS_ENTRIES + * sizeof(bios_directory_entry); + ctx->biosdir->entries[count].source = + BUFF_TO_RUN(*ctx, ctx->biosdir2); + ctx->biosdir->entries[count].address_mode = + SET_ADDR_MODE(ctx->biosdir, AMD_ADDR_REL_BIOS); + ctx->biosdir->entries[count].subprog = 0; + ctx->biosdir->entries[count].inst = 0; + ctx->biosdir->entries[count].copy = 0; + ctx->biosdir->entries[count].compressed = 0; + ctx->biosdir->entries[count].dest = -1; + ctx->biosdir->entries[count].reset = 0; + ctx->biosdir->entries[count].ro = 0; + count++; + } + fill_dir_header(ctx->biosdir, count, BHD_COOKIE, ctx); + ctx->current_table = current_table_save; +} static void integrate_bios_firmwares(context *ctx, bios_directory_table *biosdir, bios_directory_table *biosdir2, @@ -1394,29 +1443,6 @@ } fill_dir_header(biosdir, count, cookie, ctx); - - if (biosdir2) { - assert_fw_entry(count, MAX_BIOS_ENTRIES, ctx); - biosdir->entries[count].type = AMD_BIOS_L2_PTR; - biosdir->entries[count].region_type = 0; - biosdir->entries[count].size = - + MAX_BIOS_ENTRIES - * sizeof(bios_directory_entry); - biosdir->entries[count].source = - BUFF_TO_RUN(*ctx, biosdir2); - biosdir->entries[count].address_mode = - SET_ADDR_MODE(biosdir, AMD_ADDR_REL_BIOS); - biosdir->entries[count].subprog = 0; - biosdir->entries[count].inst = 0; - biosdir->entries[count].copy = 0; - biosdir->entries[count].compressed = 0; - biosdir->entries[count].dest = -1; - biosdir->entries[count].reset = 0; - biosdir->entries[count].ro = 0; - count++; - } - - fill_dir_header(biosdir, count, cookie, ctx); ctx->current_table = current_table_save; } @@ -1689,6 +1715,7 @@ ctx.pspdir = new_psp_dir(&ctx, cb_config.multi_level); integrate_psp_firmwares(&ctx, ctx.pspdir, ctx.pspdir2, ctx.pspdir2_b, amd_psp_fw_table, PSP_COOKIE, &cb_config); + integrate_psp_levels(&ctx, &cb_config); } else { /* flat: PSP 1 cookie and no pointer to 2nd table */ ctx.pspdir = new_psp_dir(&ctx, cb_config.multi_level); @@ -1735,6 +1762,7 @@ ctx.biosdir = new_bios_dir(&ctx, cb_config.multi_level); integrate_bios_firmwares(&ctx, ctx.biosdir, ctx.biosdir2, amd_bios_table, BHD_COOKIE, &cb_config); + integrate_bios_levels(&ctx); } } else { /* flat: BHD1 cookie and no pointer to 2nd table */ -- To view, visit https://review.coreboot.org/c/coreboot/+/81252?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ieaf97208e943c79d7b76ea62eea9355138c220b9 Gerrit-Change-Number: 81252 Gerrit-PatchSet: 4 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: amdfwtool: Move the address of tables to the context
by Felix Held (Code Review) 20 Mar '24

20 Mar '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/81225?usp=email ) Change subject: amdfwtool: Move the address of tables to the context ...................................................................... amdfwtool: Move the address of tables to the context Instead of being local variables. This can be easier to find all the tables anywhere. TEST=Identical test on all AMD SOC platform Change-Id: I98b7d01e32c75b4f13e23d496cd3de3da900678d Signed-off-by: Zheng Bao <fishbaozi(a)gmail.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/81225 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Held <felix-coreboot(a)felixheld.de> --- M util/amdfwtool/amdfwtool.c M util/amdfwtool/amdfwtool.h 2 files changed, 92 insertions(+), 62 deletions(-) Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved diff --git a/util/amdfwtool/amdfwtool.c b/util/amdfwtool/amdfwtool.c index ef97010..aedd326 100644 --- a/util/amdfwtool/amdfwtool.c +++ b/util/amdfwtool/amdfwtool.c @@ -813,6 +813,30 @@ } } +static void dump_image_addresses(context *ctx) +{ + printf("romsig offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->amd_romsig_ptr)); + printf("PSP L1 offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->pspdir)); + if (ctx->pspdir2 != NULL) + printf("PSP L2(A) offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->pspdir2)); + if (ctx->ish_a_dir != NULL) + printf("ISHA offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->ish_a_dir)); + if (ctx->ish_b_dir != NULL) + printf("ISHB offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->ish_b_dir)); + if (ctx->pspdir2_b != NULL) + printf("PSP L2B offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->pspdir2_b)); + if (ctx->biosdir != NULL) + printf("BHD offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->biosdir)); + if (ctx->biosdir2 != NULL) + printf("BHD L2(A) offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->biosdir2)); + if (ctx->biosdir2_b != NULL) + printf("BHD L2B offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->biosdir2_b)); + if (ctx->psp_combo_dir != NULL) + printf("PSP combo offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->psp_combo_dir)); + if (ctx->bhd_combo_dir != NULL) + printf("BHD combo offset:%lx\n", BUFF_TO_RUN(*ctx, ctx->bhd_combo_dir)); +} + static void integrate_psp_ab(context *ctx, psp_directory_table *pspdir, psp_directory_table *pspdir2, ish_directory_table *ish, amd_fw_type ab, enum platform soc_id) @@ -871,7 +895,6 @@ uint64_t addr; uint32_t current_table_save; bool recovery_ab = cb_config->recovery_ab; - ish_directory_table *ish_a_dir = NULL, *ish_b_dir = NULL; bool use_only_a = (cb_config->soc_id == PLATFORM_PHOENIX); /* TODO: b:285390041 */ /* This function can create a primary table, a secondary table, or a @@ -999,19 +1022,19 @@ if (recovery_ab && (pspdir2 != NULL)) { if (cb_config->need_ish) { /* Need ISH */ - ish_a_dir = new_ish_dir(ctx); + ctx->ish_a_dir = new_ish_dir(ctx); if (pspdir2_b != NULL) - ish_b_dir = new_ish_dir(ctx); + ctx->ish_b_dir = new_ish_dir(ctx); } pspdir->header.num_entries = count; - integrate_psp_ab(ctx, pspdir, pspdir2, ish_a_dir, + integrate_psp_ab(ctx, pspdir, pspdir2, ctx->ish_a_dir, AMD_FW_RECOVERYAB_A, cb_config->soc_id); if (pspdir2_b != NULL) - integrate_psp_ab(ctx, pspdir, pspdir2_b, ish_b_dir, + integrate_psp_ab(ctx, pspdir, pspdir2_b, ctx->ish_b_dir, use_only_a ? AMD_FW_RECOVERYAB_A : AMD_FW_RECOVERYAB_B, cb_config->soc_id); else - integrate_psp_ab(ctx, pspdir, pspdir2, ish_a_dir, + integrate_psp_ab(ctx, pspdir, pspdir2, ctx->ish_a_dir, use_only_a ? AMD_FW_RECOVERYAB_A : AMD_FW_RECOVERYAB_B, cb_config->soc_id); @@ -1511,11 +1534,6 @@ int main(int argc, char **argv) { int retval = 0; - embedded_firmware *amd_romsig; - psp_directory_table *pspdir = NULL; - psp_directory_table *pspdir2 = NULL; - psp_directory_table *pspdir2_b = NULL; - psp_combo_directory *psp_combo_dir = NULL, *bhd_combo_dir = NULL; int combo_index = 0; int targetfd; context ctx = { 0 }; @@ -1551,14 +1569,14 @@ romsig_offset = cb_config.efs_location ? cb_config.efs_location : AMD_ROMSIG_OFFSET; set_current_pointer(&ctx, romsig_offset); - amd_romsig = BUFF_OFFSET(ctx, romsig_offset); - amd_romsig->signature = EMBEDDED_FW_SIGNATURE; - amd_romsig->imc_entry = 0; - amd_romsig->gec_entry = 0; - amd_romsig->xhci_entry = 0; + ctx.amd_romsig_ptr = BUFF_OFFSET(ctx, romsig_offset); + ctx.amd_romsig_ptr->signature = EMBEDDED_FW_SIGNATURE; + ctx.amd_romsig_ptr->imc_entry = 0; + ctx.amd_romsig_ptr->gec_entry = 0; + ctx.amd_romsig_ptr->xhci_entry = 0; if (cb_config.soc_id != PLATFORM_UNKNOWN) { - retval = set_efs_table(cb_config.soc_id, &cb_config, amd_romsig); + retval = set_efs_table(cb_config.soc_id, &cb_config, ctx.amd_romsig_ptr); if (retval) { fprintf(stderr, "ERROR: Failed to initialize EFS table!\n"); return retval; @@ -1579,7 +1597,7 @@ else set_current_pointer(&ctx, romsig_offset + sizeof(embedded_firmware)); - integrate_firmwares(&ctx, amd_romsig, amd_fw_table); + integrate_firmwares(&ctx, ctx.amd_romsig_ptr, amd_fw_table); if (is_initial_alignment_required(cb_config.soc_id)) { /* TODO: Check for older platforms. */ @@ -1596,12 +1614,12 @@ cb_config.soc_id); if (cb_config.use_combo) { - psp_combo_dir = new_combo_dir(&ctx); + ctx.psp_combo_dir = new_combo_dir(&ctx); adjust_current_pointer(&ctx, 0, 0x1000U); if (!cb_config.recovery_ab) - bhd_combo_dir = new_combo_dir(&ctx); + ctx.bhd_combo_dir = new_combo_dir(&ctx); } combo_index = 0; @@ -1612,6 +1630,14 @@ if (cb_config.use_combo && cb_config.debug) printf("Processing %dth combo entry\n", combo_index); + ctx.pspdir = NULL; + ctx.pspdir2 = NULL; + ctx.pspdir2_b = NULL; + ctx.biosdir = NULL; + ctx.biosdir2 = NULL; + ctx.biosdir2_b = NULL; + ctx.ish_a_dir = NULL; + ctx.ish_b_dir = NULL; /* for non-combo image, combo_config[0] == config, and * it already is processed. Actually "combo_index > * 0" is enough. Put both of them here to make sure @@ -1640,14 +1666,14 @@ if (cb_config.multi_level) { /* Do 2nd PSP directory followed by 1st */ - pspdir2 = new_psp_dir(&ctx, cb_config.multi_level); - integrate_psp_firmwares(&ctx, pspdir2, NULL, NULL, + ctx.pspdir2 = new_psp_dir(&ctx, cb_config.multi_level); + integrate_psp_firmwares(&ctx, ctx.pspdir2, NULL, NULL, amd_psp_fw_table, PSPL2_COOKIE, &cb_config); 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, + ctx.pspdir2_b = new_psp_dir(&ctx, cb_config.multi_level); + integrate_psp_firmwares(&ctx, ctx.pspdir2_b, NULL, NULL, amd_psp_fw_table, PSPL2_COOKIE, &cb_config); } else { /* @@ -1658,88 +1684,87 @@ * Related biosdir2_b will be skipped * automatically. */ - pspdir2_b = NULL; /* More explicitly */ + ctx.pspdir2_b = NULL; /* More explicitly */ } - pspdir = new_psp_dir(&ctx, cb_config.multi_level); - integrate_psp_firmwares(&ctx, pspdir, pspdir2, pspdir2_b, + ctx.pspdir = new_psp_dir(&ctx, cb_config.multi_level); + integrate_psp_firmwares(&ctx, ctx.pspdir, ctx.pspdir2, ctx.pspdir2_b, amd_psp_fw_table, PSP_COOKIE, &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, + ctx.pspdir = new_psp_dir(&ctx, cb_config.multi_level); + integrate_psp_firmwares(&ctx, ctx.pspdir, NULL, NULL, amd_psp_fw_table, PSP_COOKIE, &cb_config); } if (!cb_config.use_combo) { - fill_psp_directory_to_efs(amd_romsig, pspdir, &ctx, &cb_config); + fill_psp_directory_to_efs(ctx.amd_romsig_ptr, ctx.pspdir, &ctx, &cb_config); } else { - fill_psp_directory_to_efs(amd_romsig, psp_combo_dir, &ctx, &cb_config); + fill_psp_directory_to_efs(ctx.amd_romsig_ptr, ctx.psp_combo_dir, &ctx, &cb_config); /* 0 -Compare PSP ID, 1 -Compare chip family ID */ assert_fw_entry(combo_index, MAX_COMBO_ENTRIES, &ctx); - psp_combo_dir->entries[combo_index].id_sel = 0; - psp_combo_dir->entries[combo_index].id = get_psp_id(cb_config.soc_id); - psp_combo_dir->entries[combo_index].lvl2_addr = - BUFF_TO_RUN_MODE(ctx, pspdir, AMD_ADDR_REL_BIOS); + ctx.psp_combo_dir->entries[combo_index].id_sel = 0; + ctx.psp_combo_dir->entries[combo_index].id = get_psp_id(cb_config.soc_id); + ctx.psp_combo_dir->entries[combo_index].lvl2_addr = + BUFF_TO_RUN_MODE(ctx, ctx.pspdir, AMD_ADDR_REL_BIOS); - fill_dir_header(psp_combo_dir, combo_index + 1, PSP2_COOKIE, &ctx); + fill_dir_header(ctx.psp_combo_dir, combo_index + 1, PSP2_COOKIE, &ctx); } if (have_bios_tables(amd_bios_table)) { - bios_directory_table *biosdir = NULL; if (cb_config.multi_level) { /* Do 2nd level BIOS directory followed by 1st */ - bios_directory_table *biosdir2 = NULL; - bios_directory_table *biosdir2_b = NULL; + ctx.biosdir2 = new_bios_dir(&ctx, cb_config.multi_level); - biosdir2 = new_bios_dir(&ctx, cb_config.multi_level); - - integrate_bios_firmwares(&ctx, biosdir2, NULL, + integrate_bios_firmwares(&ctx, ctx.biosdir2, NULL, amd_bios_table, BHDL2_COOKIE, &cb_config); if (cb_config.recovery_ab) { - if (pspdir2_b != NULL) { - biosdir2_b = new_bios_dir(&ctx, + if (ctx.pspdir2_b != NULL) { + ctx.biosdir2_b = new_bios_dir(&ctx, cb_config.multi_level); - integrate_bios_firmwares(&ctx, biosdir2_b, NULL, + integrate_bios_firmwares(&ctx, ctx.biosdir2_b, NULL, amd_bios_table, BHDL2_COOKIE, &cb_config); } - add_psp_firmware_entry(&ctx, pspdir2, biosdir2, + add_psp_firmware_entry(&ctx, ctx.pspdir2, ctx.biosdir2, AMD_FW_BIOS_TABLE, TABLE_ALIGNMENT); - if (pspdir2_b != NULL) - add_psp_firmware_entry(&ctx, pspdir2_b, - biosdir2_b, AMD_FW_BIOS_TABLE, + if (ctx.pspdir2_b != NULL) + add_psp_firmware_entry(&ctx, ctx.pspdir2_b, + ctx.biosdir2_b, AMD_FW_BIOS_TABLE, TABLE_ALIGNMENT); } else { - biosdir = new_bios_dir(&ctx, cb_config.multi_level); - integrate_bios_firmwares(&ctx, biosdir, biosdir2, + ctx.biosdir = new_bios_dir(&ctx, cb_config.multi_level); + integrate_bios_firmwares(&ctx, ctx.biosdir, ctx.biosdir2, amd_bios_table, BHD_COOKIE, &cb_config); } } else { /* flat: BHD1 cookie and no pointer to 2nd table */ - biosdir = new_bios_dir(&ctx, cb_config.multi_level); - integrate_bios_firmwares(&ctx, biosdir, NULL, + ctx.biosdir = new_bios_dir(&ctx, cb_config.multi_level); + integrate_bios_firmwares(&ctx, ctx.biosdir, NULL, amd_bios_table, BHD_COOKIE, &cb_config); } if (!cb_config.use_combo) { - fill_bios_directory_to_efs(amd_romsig, biosdir, + fill_bios_directory_to_efs(ctx.amd_romsig_ptr, ctx.biosdir, &ctx, &cb_config); - } else if (bhd_combo_dir != NULL) { + } else if (ctx.bhd_combo_dir != NULL) { /* In recovery A/B mode, there isn't a BHD combo directory. * Instead, the BIOS tables level 2 are linked by PSP tables. */ - fill_bios_directory_to_efs(amd_romsig, bhd_combo_dir, + fill_bios_directory_to_efs(ctx.amd_romsig_ptr, ctx.bhd_combo_dir, &ctx, &cb_config); assert_fw_entry(combo_index, MAX_COMBO_ENTRIES, &ctx); - bhd_combo_dir->entries[combo_index].id_sel = 0; - bhd_combo_dir->entries[combo_index].id = + ctx.bhd_combo_dir->entries[combo_index].id_sel = 0; + ctx.bhd_combo_dir->entries[combo_index].id = get_psp_id(cb_config.soc_id); - bhd_combo_dir->entries[combo_index].lvl2_addr = - BUFF_TO_RUN_MODE(ctx, biosdir, AMD_ADDR_REL_BIOS); + ctx.bhd_combo_dir->entries[combo_index].lvl2_addr = + BUFF_TO_RUN_MODE(ctx, ctx.biosdir, AMD_ADDR_REL_BIOS); - fill_dir_header(bhd_combo_dir, combo_index + 1, + fill_dir_header(ctx.bhd_combo_dir, combo_index + 1, BHD2_COOKIE, &ctx); + } } + if (cb_config.debug) + dump_image_addresses(&ctx); } while (cb_config.use_combo && ++combo_index < MAX_COMBO_ENTRIES && cb_config.combo_config[combo_index] != NULL); @@ -1747,7 +1772,7 @@ if (targetfd >= 0) { uint32_t offset = cb_config.efs_location; uint32_t bytes = cb_config.efs_location == cb_config.body_location ? - ctx.current - offset : sizeof(*amd_romsig); + ctx.current - offset : sizeof(embedded_firmware); uint32_t ret_bytes; ret_bytes = write_from_buf_to_file(targetfd, BUFF_OFFSET(ctx, offset), bytes); diff --git a/util/amdfwtool/amdfwtool.h b/util/amdfwtool/amdfwtool.h index fdcd6a8..1dc445c 100644 --- a/util/amdfwtool/amdfwtool.h +++ b/util/amdfwtool/amdfwtool.h @@ -452,6 +452,11 @@ uint8_t ins; uint8_t sub; } combo_apcb[MAX_COMBO_ENTRIES], combo_apcb_bk[MAX_COMBO_ENTRIES]; + embedded_firmware *amd_romsig_ptr; + psp_directory_table *pspdir, *pspdir2, *pspdir2_b; + bios_directory_table *biosdir, *biosdir2, *biosdir2_b; + psp_combo_directory *psp_combo_dir, *bhd_combo_dir; + ish_directory_table *ish_a_dir, *ish_b_dir; } context; uint8_t process_config(FILE *config, amd_cb_config *cb_config); -- To view, visit https://review.coreboot.org/c/coreboot/+/81225?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I98b7d01e32c75b4f13e23d496cd3de3da900678d Gerrit-Change-Number: 81225 Gerrit-PatchSet: 5 Gerrit-Owner: Bao Zheng <fishbaozi(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Zheng Bao Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: vc/amd/opensil/stub/mpio: change mpio_engine_type prefix to IFTYPE
by Varshit Pandya (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Felix Held. Varshit Pandya has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/81346?usp=email ) Change subject: vc/amd/opensil/stub/mpio: change mpio_engine_type prefix to IFTYPE ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/81346?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If81c5ea01ba147b71b423004a2199b348ffac99a Gerrit-Change-Number: 81346 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 20 Mar 2024 13:14:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: cpu/x86: Link page tables in stage if possible
by Arthur Heymans (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Arthur Heymans, Felix Held, Fred Reitberger, Jason Glenesk, Jérémy Compostella, Matt DeVillier, Patrick Rudolph. Hello Felix Held, Fred Reitberger, Jason Glenesk, Jérémy Compostella, Matt DeVillier, Patrick Rudolph, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80337?usp=email to look at the new patch set (#13). Change subject: cpu/x86: Link page tables in stage if possible ...................................................................... cpu/x86: Link page tables in stage if possible When switching back and forward from 32 to 64, for example to call a 32bit FSP or to call the payload, a new page tables in the respective stage will be linked. The advantages of this approach are: - No need to determine a good place for page tables in cbfs that does not overlap. - Works with non memory mapped flash (however all coreboot targets currently do support this) - If later stages can use their own page tables which fits better with the vboot RO/RW flow A disadvantage is that it increases the stage size. This could be improved upon by used 1G pages and generating the pages at runtime. Note: qemu cannot have the page tables in the RO boot medium and needs to relocate them at runtime. This is why keeping the existing code with page tables in cbfs is done for now. TESTED on google/vilbox and qemu/q35: boots to payload Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Change-Id: Ied54b66b930187cba5fbc578a81ed5859a616562 --- M src/arch/x86/Kconfig M src/cpu/intel/car/core2/cache_as_ram.S M src/cpu/intel/car/non-evict/cache_as_ram.S M src/cpu/intel/car/p4-netburst/cache_as_ram.S M src/cpu/x86/64bit/Makefile.mk M src/cpu/x86/64bit/entry64.inc M src/cpu/x86/64bit/mode_switch.S M src/cpu/x86/64bit/mode_switch2.S M src/cpu/x86/64bit/pt.S M src/mainboard/emulation/qemu-i440fx/Kconfig M src/mainboard/emulation/qemu-q35/Kconfig M src/soc/amd/common/block/cpu/noncar/pre_c.S M src/soc/intel/common/block/cpu/car/cache_as_ram.S 13 files changed, 28 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/37/80337/13 -- To view, visit https://review.coreboot.org/c/coreboot/+/80337?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ied54b66b930187cba5fbc578a81ed5859a616562 Gerrit-Change-Number: 80337 Gerrit-PatchSet: 13 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: mb/google/brya: Create yavista variant
by Subrata Banik (Code Review) 20 Mar '24

20 Mar '24

Attention is currently required from: Derek Huang, Dinesh Gehlot, Eric Lai, Garen Wu, Henry Sun, Kapil Porwal, Nick Vaccaro, Rasheed Hsueh. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80342?usp=email ) Change subject: mb/google/brya: Create yavista variant ...................................................................... Patch Set 8: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80342?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6fa464a4dcd9551a42e8746e64c724b3582dbe02 Gerrit-Change-Number: 80342 Gerrit-PatchSet: 8 Gerrit-Owner: Rasheed Hsueh <hsueh.rasheed(a)inventec.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Henry Sun <henrysun(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Garen Wu <wu.garen(a)inventec.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Ryan Lin <ryan.lin(a)intel.corp-partner.google.com> Gerrit-CC: Steven Yeh <yeh.stevenct(a)inventec.corp-partner.google.com> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Henry Sun <henrysun(a)google.com> Gerrit-Attention: Derek Huang <derekhuang(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Rasheed Hsueh <hsueh.rasheed(a)inventec.corp-partner.google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Garen Wu <wu.garen(a)inventec.corp-partner.google.com> Gerrit-Comment-Date: Wed, 20 Mar 2024 10:31:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2024