From 175e4c59a0025dc4636669734ae4ed756cb1af5f Mon Sep 17 00:00:00 2001 From: Nikolai Vyssotski Date: Thu, 11 Feb 2021 18:25:43 -0600 Subject: [PATCH] drivers/intel/fsp2_0: Allow larger FSPS UPD than expected in coreboot Enforcing the exact match of FSPS UPD block size between FSP and coreboot mandates simultaneous updates to coreboot and FSP repos. Allow coreboot to proceed if its UPD structure is smaller than FSP one. This usually indicates that FSPS has an updated (larger) UPD structure which should be soon matched/updated on the coreboot side to keep them in sync. While this is an undesirable situation that should be corrected ASAP, it is safe from coreboot perspective. It is safe (as long as default values in FSP UPD are sane enough to boot) because FSPS UPD buffer is allocated on the heap with the size specified in FSPS (larger) and filled with FSPS default values. This allows FSP UPD changes to be submitted first followed by changes in coreboot repo. Note that this only applies to the case when entire FSPS UPD structure grows which should be rare as FSP should allocate enough reserve space, anticipating future expansion, to keep the structure from growing when new members are added. BUG=b:171234996 BRANCH=Zork TEST=build Trembyle Change-Id: I557fd3a1f208b5b444ccf76e1552e74ecf4decad Signed-off-by: Nikolai Vyssotski Reviewed-on: https://review.coreboot.org/c/coreboot/+/50576 Tested-by: build bot (Jenkins) Reviewed-by: Felix Held Reviewed-by: Martin Roth --- src/drivers/intel/fsp2_0/silicon_init.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index 6a2a73dbb93d..3975a9611141 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -90,13 +90,17 @@ static void do_silicon_init(struct fsp_header *hdr) fsp_verify_upd_header_signature(supd->FspUpdHeader.Signature, FSPS_UPD_SIGNATURE); - /* Disallow invalid config regions. Default settings are likely bad - * choices for coreboot, and different sized UPD from what the region - * allows is potentially a build problem. + /* FSPS UPD and coreboot structure sizes should match. However, enforcing the exact + * match mandates simultaneous updates to coreboot and FSP repos. Allow coreboot + * to proceed if its UPD structure is smaller than FSP one to enable staggered UPD + * update process on both sides. The mismatch indicates a temporary build problem, + * don't leave it like this as FSP default settings can be bad choices for coreboot. */ - if (!hdr->cfg_region_size || hdr->cfg_region_size != sizeof(FSPS_UPD)) + if (!hdr->cfg_region_size || hdr->cfg_region_size < sizeof(FSPS_UPD)) die_with_post_code(POST_INVALID_VENDOR_BINARY, "Invalid FSPS UPD region\n"); + else if (hdr->cfg_region_size > sizeof(FSPS_UPD)) + printk(BIOS_ERR, "FSP and coreboot are out of sync! FSPS UPD size > coreboot\n"); upd = xmalloc(hdr->cfg_region_size);