From 69cd62c75174d46993955cf67d9b6ee475308c32 Mon Sep 17 00:00:00 2001 From: Aamir Bohra Date: Mon, 8 Jan 2018 11:01:34 +0530 Subject: intel/fsp2_0: Set boot mode default as per s3wake status Currently bootmode default is set to FSP_BOOT_WITH_FULL_CONFIGURATION and bootmode UPD is updated in fsp_fill_mrc_cache based on mrc cache data validity. With current implementation in S3 resume path, if mrc cache data is invalid, the bootmode is not updated further and remains set at FSP_BOOT_WITH_FULL_CONFIGURATION. This results in fsp-m to get incorrect boot mode context and reinitialize memory in S3 resume path. In correct flow fspm should have correct bootmode context i.e. S3 resume and return error in case mrc cache data is invalid or not found. BUG=b:70973961 BRANCH=None TEST=Verify correct bootmode is set on S3 resume, even when mrc cache data is invalid. Change-Id: Idc0da6ffbfe5ce616d852908a9b0074dc8ce7cbe Signed-off-by: Aamir Bohra Reviewed-on: https://review.coreboot.org/23156 Tested-by: build bot (Jenkins) Reviewed-by: Furquan Shaikh Reviewed-by: Subrata Banik Reviewed-by: Aaron Durbin --- src/drivers/intel/fsp2_0/memory_init.c | 45 ++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 7b031040c8..368fafa5d7 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -190,8 +190,7 @@ static int mrc_cache_verify_tpm_hash(const uint8_t *data, size_t size) return 1; } -static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, bool s3wake, - uint32_t fsp_version) +static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, uint32_t fsp_version) { struct region_device rdev; void *data; @@ -228,11 +227,9 @@ static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, bool s3wake, /* MRC cache found */ arch_upd->NvsBufferPtr = data; - arch_upd->BootMode = s3wake ? - FSP_BOOT_ON_S3_RESUME : - FSP_BOOT_ASSUMING_NO_CONFIGURATION_CHANGES; - printk(BIOS_SPEW, "MRC cache found, size %zx bootmode:%d\n", - region_device_sz(&rdev), arch_upd->BootMode); + + printk(BIOS_SPEW, "MRC cache found, size %zx\n", + region_device_sz(&rdev)); } static enum cb_err check_region_overlap(const struct memranges *ranges, @@ -275,22 +272,28 @@ static enum cb_err fsp_fill_common_arch_params(FSPM_ARCH_UPD *arch_upd, arch_upd->StackBase = (void *)stack_begin; - arch_upd->BootMode = FSP_BOOT_WITH_FULL_CONFIGURATION; + fsp_fill_mrc_cache(arch_upd, fsp_version); - fsp_fill_mrc_cache(arch_upd, s3wake, fsp_version); + /* Configure bootmode */ + if (s3wake) { + /* + * For S3 resume case, if valid mrc cache data is not found or + * RECOVERY_MRC_CACHE hash verification fails, the S3 data + * pointer would be null and S3 resume fails with fsp-m + * returning error. Invoking a reset here saves time. + */ + if (!arch_upd->NvsBufferPtr) + hard_reset(); + arch_upd->BootMode = FSP_BOOT_ON_S3_RESUME; + } else { + if (arch_upd->NvsBufferPtr) + arch_upd->BootMode = + FSP_BOOT_ASSUMING_NO_CONFIGURATION_CHANGES; + else + arch_upd->BootMode = FSP_BOOT_WITH_FULL_CONFIGURATION; + } - /* - * For S3 resume case, if valid mrc cache data is not found - * or RECOVERY_MRC_CACHE hash verification fails, the S3 data - * pointer would be null and Bootmode is set to - * BOOT_WITH_FULL_CONFIGURATION. This gets memory to be retrained - * in S3 flow. Data context including that of imdr root pointer would - * be lost, invoking a hard reset in romstage post memory init. - * Issuing hard reset here, saves fsp memory initialization and - * training overhead. - */ - if (s3wake && !arch_upd->NvsBufferPtr) - hard_reset(); + printk(BIOS_SPEW, "bootmode is set to :%d\n", arch_upd->BootMode); return CB_SUCCESS; } -- cgit v1.2.3