Change in coreboot[master]: xeon_sp/cpx: Check FSP version for WW45 and die if version mismatch
Johnny Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/47560 )
Change subject: xeon_sp/cpx: Check FSP version for WW45 and die if version mismatch ......................................................................
xeon_sp/cpx: Check FSP version for WW45 and die if version mismatch
Change-Id: I19082ec210a270649e203779757fe39cb2e284f7 Signed-off-by: Johnny Lin johnny_lin@wiwynn.com --- M src/soc/intel/xeon_sp/cpx/Makefile.inc A src/soc/intel/xeon_sp/cpx/fsp.c M src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h 3 files changed, 62 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/60/47560/1
diff --git a/src/soc/intel/xeon_sp/cpx/Makefile.inc b/src/soc/intel/xeon_sp/cpx/Makefile.inc index b2e653e..a0d9797 100644 --- a/src/soc/intel/xeon_sp/cpx/Makefile.inc +++ b/src/soc/intel/xeon_sp/cpx/Makefile.inc @@ -8,7 +8,7 @@ subdirs-y += ../../../../cpu/x86/tsc subdirs-y += ../../../../cpu/intel/microcode
-romstage-y += romstage.c ddr.c +romstage-y += romstage.c ddr.c fsp.c romstage-$(CONFIG_DISPLAY_UPD_DATA) += upd_display.c romstage-$(CONFIG_DISPLAY_HOBS) += hob_display.c
diff --git a/src/soc/intel/xeon_sp/cpx/fsp.c b/src/soc/intel/xeon_sp/cpx/fsp.c new file mode 100644 index 0000000..0358143 --- /dev/null +++ b/src/soc/intel/xeon_sp/cpx/fsp.c @@ -0,0 +1,54 @@ +#include <console/console.h> +#include <fsp/util.h> +#include <FspmUpd.h> + +void soc_validate_fsp_version(const struct fsp_header *hdr) +{ + bool mismatch = false; + union { + uint32_t val; + struct { + uint8_t bld_num; + uint8_t revision; + uint8_t minor; + uint8_t major; + } rev; + } revision; + revision.val = hdr->fsp_revision; + + if (((hdr->spec_version >> 4) != CPX_SPEC_MAJOR)) { + printk(BIOS_ALERT, "FSP spec major version %d doesn't match!\n", + hdr->spec_version >> 4); + mismatch = true; + } + if (((hdr->spec_version & 0xf) != CPX_SPEC_MINOR)) { + printk(BIOS_ALERT, "FSP spec minor version %d doesn't match!\n", + hdr->spec_version & 0xf); + mismatch = true; + } + if (revision.rev.major != CPX_REV_MAJOR) { + printk(BIOS_ALERT, "FSP revision major version %d doesn't match!\n", + revision.rev.major); + mismatch = true; + } + if (revision.rev.minor != CPX_REV_MINOR) { + printk(BIOS_ALERT, "FSP revision minor version %d doesn't match!\n", + revision.rev.minor); + mismatch = true; + } + if (revision.rev.revision != CPX_REV_VER) { + printk(BIOS_ALERT, "FSP revision version %d doesn't match!\n", + revision.rev.revision); + mismatch = true; + } + if (revision.rev.bld_num != CPX_REV_BUILD) { + printk(BIOS_ALERT, "FSP revision build number %d doesn't match!\n", + revision.rev.bld_num); + mismatch = true; + } + + if (mismatch) + die("FSP version is not correct\n"); + + printk(BIOS_INFO, "FSP version is expected\n"); +} diff --git a/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h b/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h index b4d4cad..15de4be 100644 --- a/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h +++ b/src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h @@ -64,7 +64,13 @@ {0x1, (1 << SPEED_REC_96GT) | (1 << SPEED_REC_104GT), (1 << KTI_LINK5), 0x2C35363F, ADAPTIVE_CTLE}
#define CPXSP_2S6KTI_EPARAM_TABLE_COUNT 12 // NOTE - needs to match number of elements in CPXSP_2S6KTI_EPARAM_TABLE - +/* FSP version */ +#define CPX_SPEC_MAJOR 2 +#define CPX_SPEC_MINOR 1 +#define CPX_REV_MAJOR 0 +#define CPX_REV_MINOR 2 +#define CPX_REV_VER 1 +#define CPX_REV_BUILD 45
#pragma pack(1)
Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47560 )
Change subject: xeon_sp/cpx: Check FSP version for WW45 and die if version mismatch ......................................................................
Patch Set 1:
(1 comment)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/47560/1/src/vendorcode/intel/fsp/fs... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/47560/1/src/vendorcode/intel/fsp/fs... PS1, Line 73: #define CPX_REV_BUILD 45 Instead of: CPX_REV_BUILD 45 suggest changing to: CPX_REV_BUILD_YEAR_MIN 1 CPX_REV_BUILD_YEAR_MAX 1 CPX_REV_BUILD_WW_MAX 45 CPX_REV_BUILD_WW_MIN 45
Support next FSP release is ww47. If there is no interface change, we would update CPX_REV_BUILD_WW_MAX to 47; If there is interface change, we would update both CPX_REV_BUILD_WW_MAX and CPX_REV_BUILD_WW_MIN to 47.
Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47560 )
Change subject: xeon_sp/cpx: Check FSP version for WW45 and die if version mismatch ......................................................................
Patch Set 1:
(4 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/47560/1/src/soc/intel/xeon_sp/cpx/f... File src/soc/intel/xeon_sp/cpx/fsp.c:
https://review.coreboot.org/c/coreboot/+/47560/1/src/soc/intel/xeon_sp/cpx/f... PS1, Line 8: union { : uint32_t val; : struct { : uint8_t bld_num; : uint8_t revision; : uint8_t minor; : uint8_t major; : } rev; : } revision; this is duplicate code and should be defined in fsp/util.h. It is used in fsp/util.c and should be updated there as well.
https://review.coreboot.org/c/coreboot/+/47560/1/src/soc/intel/xeon_sp/cpx/f... PS1, Line 19: spec_version How about do the shift in the preprocessor and have a single check for major and minor version. #define CPX_SPEC_VER ...
if (hdr->spec_version != CPX_SPEC_VERSION) { print... mismatch... }
You can probably do similar for the checks below. It is easy to see what is wrong when it is printed.
https://review.coreboot.org/c/coreboot/+/47560/1/src/soc/intel/xeon_sp/cpx/f... PS1, Line 53: Print out the version.
https://review.coreboot.org/c/coreboot/+/47560/1/src/vendorcode/intel/fsp/fs... File src/vendorcode/intel/fsp/fsp2_0/cooperlake_sp/FspmUpd.h:
https://review.coreboot.org/c/coreboot/+/47560/1/src/vendorcode/intel/fsp/fs... PS1, Line 73: #define CPX_REV_BUILD 45
Instead of: […]
This should go in a new file version.h and have an explanation on the version scheme for this fsp.
Johnny Lin has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/47560 )
Change subject: xeon_sp/cpx: Override soc_validate_fsp_version and soc_fsp_get_version ......................................................................
Abandoned
-
Johnny Lin (Code Review) -
Jonathan Zhang (Code Review)
-
Marc Jones (Code Review)