Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/58869 )
Change subject: drivers/intel/fsp2_0/include/fsp: fix fsp_header ......................................................................
drivers/intel/fsp2_0/include/fsp: fix fsp_header
This patch aligns fsp_header with the Intel specification 2.0 and 2.3. The main impetus for this change is to make the fsp_info_header fully accessible in soc/vendor code. Here items such as image_revision can be checked.
TEST=verify image revision output in the coreboot serial log. compare to FSP version shown in serial debug output. verify Google Guybrush machine boots into OS.
Signed-off-by: Julian Schroeder julianmarcusschroeder@gmail.com Change-Id: Ibf50f16b5e9793d946a95970fcdabc4c07289646 Reviewed-on: https://review.coreboot.org/c/coreboot/+/58869 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de Reviewed-by: Tim Wawrzynczak twawrzynczak@chromium.org --- M src/drivers/intel/fsp2_0/header_display.c M src/drivers/intel/fsp2_0/include/fsp/info_header.h M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/silicon_init.c M src/drivers/intel/fsp2_0/temp_ram_exit.c M src/drivers/intel/fsp2_0/util.c 6 files changed, 52 insertions(+), 62 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, but someone else must approve Tim Wawrzynczak: Looks good to me, approved
diff --git a/src/drivers/intel/fsp2_0/header_display.c b/src/drivers/intel/fsp2_0/header_display.c index 4c8085e..d209e4c 100644 --- a/src/drivers/intel/fsp2_0/header_display.c +++ b/src/drivers/intel/fsp2_0/header_display.c @@ -8,13 +8,14 @@ union fsp_revision revision; union extended_fsp_revision ext_revision; ext_revision.val = 0; + int i;
/* For FSP 2.3 and later use extended image revision field present in header * for build number and revision calculation */ if (CONFIG(PLATFORM_USES_FSP2_3)) - ext_revision.val = hdr->extended_fsp_revision; + ext_revision.val = hdr->extended_image_revision;
- revision.val = hdr->fsp_revision; + revision.val = hdr->image_revision; printk(BIOS_SPEW, "Spec version: v%u.%u\n", (hdr->spec_version >> 4), hdr->spec_version & 0xf); printk(BIOS_SPEW, "Revision: %u.%u.%u, Build Number %u\n", @@ -25,22 +26,28 @@ printk(BIOS_SPEW, "Type: %s/%s\n", (hdr->component_attribute & 1) ? "release" : "debug", (hdr->component_attribute & 2) ? "official" : "test"); - printk(BIOS_SPEW, "image ID: %s, base 0x%zx + 0x%zx\n", - hdr->image_id, (size_t)hdr->image_base, (size_t)hdr->image_size); + + printk(BIOS_SPEW, "image ID: "); + for (i = 0; i < FSP_IMAGE_ID_LENGTH; i++) + printk(BIOS_SPEW, "%c", hdr->image_id[i]); + printk(BIOS_SPEW, "\n"); + + printk(BIOS_SPEW, " base 0x%zx + 0x%zx\n", + (size_t)hdr->image_base, (size_t)hdr->image_size); printk(BIOS_SPEW, "\tConfig region 0x%zx + 0x%zx\n", (size_t)hdr->cfg_region_offset, (size_t)hdr->cfg_region_size);
if ((hdr->component_attribute >> 12) == FSP_HDR_ATTRIB_FSPM) { printk(BIOS_SPEW, "\tMemory init offset 0x%zx\n", - (size_t)hdr->memory_init_entry_offset); + (size_t)hdr->fsp_memory_init_entry_offset); }
if ((hdr->component_attribute >> 12) == FSP_HDR_ATTRIB_FSPS) { printk(BIOS_SPEW, "\tSilicon init offset 0x%zx\n", - (size_t)hdr->silicon_init_entry_offset); + (size_t)hdr->fsp_silicon_init_entry_offset); if (CONFIG(PLATFORM_USES_FSP2_2)) printk(BIOS_SPEW, "\tMultiPhaseSiInit offset 0x%zx\n", - (size_t)hdr->multi_phase_si_init_entry_offset); + (size_t)hdr->fsp_multi_phase_si_init_entry_offset); printk(BIOS_SPEW, "\tNotify phase offset 0x%zx\n", (size_t)hdr->notify_phase_entry_offset); } diff --git a/src/drivers/intel/fsp2_0/include/fsp/info_header.h b/src/drivers/intel/fsp2_0/include/fsp/info_header.h index 136fc2b..fceebec 100644 --- a/src/drivers/intel/fsp2_0/include/fsp/info_header.h +++ b/src/drivers/intel/fsp2_0/include/fsp/info_header.h @@ -10,27 +10,34 @@ #define FSP_HDR_ATTRIB_FSPT 1 #define FSP_HDR_ATTRIB_FSPM 2 #define FSP_HDR_ATTRIB_FSPS 3 +#define FSP_IMAGE_ID_LENGTH 8
#if CONFIG(PLATFORM_USES_FSP2_X86_32) struct fsp_header { - uint32_t fsp_revision; - uint16_t extended_fsp_revision; - uint32_t image_size; - uint32_t image_base; - uint16_t image_attribute; - uint8_t spec_version; - uint16_t component_attribute; - uint32_t cfg_region_offset; - uint32_t cfg_region_size; - uint32_t temp_ram_init_entry; - uint32_t temp_ram_exit_entry; - uint32_t notify_phase_entry_offset; - uint32_t memory_init_entry_offset; - uint32_t silicon_init_entry_offset; - uint32_t multi_phase_si_init_entry_offset; - char image_id[sizeof(uint64_t) + 1]; - uint8_t revision; -} __packed; + uint32_t signature; //FSPH + uint32_t header_length; + uint8_t res1[2]; + uint8_t spec_version; + uint8_t header_revision; + uint32_t image_revision; + char image_id[FSP_IMAGE_ID_LENGTH]; // not zero terminated + uint32_t image_size; + uint32_t image_base; + uint16_t image_attribute; + uint16_t component_attribute; + uint32_t cfg_region_offset; + uint32_t cfg_region_size; + uint32_t res2; + uint32_t temp_ram_init_entry_offset; //initial stack + uint32_t res3; + uint32_t notify_phase_entry_offset; + uint32_t fsp_memory_init_entry_offset; + uint32_t temp_ram_exit_entry_offset; + uint32_t fsp_silicon_init_entry_offset; + uint32_t fsp_multi_phase_si_init_entry_offset; + uint16_t extended_image_revision; + uint16_t res4; +} __packed; #else #error You need to implement this struct for x86_64 FSP #endif diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c index 3c1a088..3643866 100644 --- a/src/drivers/intel/fsp2_0/memory_init.c +++ b/src/drivers/intel/fsp2_0/memory_init.c @@ -206,7 +206,7 @@ static uint32_t fsp_memory_settings_version(const struct fsp_header *hdr) { /* Use the full FSP version by default. */ - uint32_t ver = hdr->fsp_revision; + uint32_t ver = hdr->image_revision;
if (!CONFIG(FSP_PLATFORM_MEMORY_SETTINGS_VERSIONS)) return ver; @@ -291,7 +291,7 @@ post_code(POST_MEM_PREINIT_PREP_END);
/* Call FspMemoryInit */ - fsp_raminit = (void *)(uintptr_t)(hdr->image_base + hdr->memory_init_entry_offset); + fsp_raminit = (void *)(uintptr_t)(hdr->image_base + hdr->fsp_memory_init_entry_offset); fsp_debug_before_memory_init(fsp_raminit, upd, &fspm_upd);
post_code(POST_FSP_MEMORY_INIT); diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c index 83d44b1..05cea11 100644 --- a/src/drivers/intel/fsp2_0/silicon_init.c +++ b/src/drivers/intel/fsp2_0/silicon_init.c @@ -75,7 +75,7 @@ bool fsp_is_multi_phase_init_enabled(void) { return CONFIG(FSPS_USE_MULTI_PHASE_INIT) && - (fsps_hdr.multi_phase_si_init_entry_offset != 0); + (fsps_hdr.fsp_multi_phase_si_init_entry_offset != 0); }
static void fsp_fill_common_arch_params(FSPS_UPD *supd) @@ -127,7 +127,7 @@
/* Call SiliconInit */ silicon_init = (void *) (uintptr_t)(hdr->image_base + - hdr->silicon_init_entry_offset); + hdr->fsp_silicon_init_entry_offset); fsp_debug_before_silicon_init(silicon_init, supd, upd);
timestamp_add_now(TS_FSP_SILICON_INIT_START); @@ -162,7 +162,7 @@
/* Call MultiPhaseSiInit */ multi_phase_si_init = (void *) (uintptr_t)(hdr->image_base + - hdr->multi_phase_si_init_entry_offset); + hdr->fsp_multi_phase_si_init_entry_offset);
/* Implementing multi_phase_si_init() is optional as per FSP 2.2 spec */ if (multi_phase_si_init == NULL) diff --git a/src/drivers/intel/fsp2_0/temp_ram_exit.c b/src/drivers/intel/fsp2_0/temp_ram_exit.c index 2192543..87b77bc 100644 --- a/src/drivers/intel/fsp2_0/temp_ram_exit.c +++ b/src/drivers/intel/fsp2_0/temp_ram_exit.c @@ -25,7 +25,7 @@ if (fsp_validate_component(&hdr, mapping, size) != CB_SUCCESS) die("Invalid FSPM header!\n");
- temp_ram_exit = (void *)(hdr.image_base + hdr.temp_ram_exit_entry); + temp_ram_exit = (void *)(hdr.image_base + hdr.temp_ram_exit_entry_offset); printk(BIOS_DEBUG, "Calling TempRamExit: %p\n", temp_ram_exit); status = temp_ram_exit(NULL);
diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c index ff79fc7..2537b38 100644 --- a/src/drivers/intel/fsp2_0/util.c +++ b/src/drivers/intel/fsp2_0/util.c @@ -26,11 +26,9 @@ return dead_code_t(uint32_t); }
-static bool looks_like_fsp_header(const uint8_t *raw_hdr) +static bool looks_like_fsp_header(struct fsp_header *hdr) { - uint32_t fsp_header_length = read32(raw_hdr + 4); - - if (memcmp(raw_hdr, FSP_HDR_SIGNATURE, 4)) { + if (memcmp(&hdr->signature, FSP_HDR_SIGNATURE, 4)) { printk(BIOS_ALERT, "Did not find a valid FSP signature\n"); return false; } @@ -39,8 +37,8 @@ fields in FSP_INFO_HEADER. The new fields will be ignored based on the reported FSP version. This check ensures that the reported header length is at least what the reported FSP version requires so that we do not access any out-of-bound bytes. */ - if (fsp_header_length < fsp_hdr_get_expected_min_length()) { - printk(BIOS_ALERT, "FSP header has invalid length: %d\n", fsp_header_length); + if (hdr->header_length < fsp_hdr_get_expected_min_length()) { + printk(BIOS_ALERT, "FSP header has invalid length: %d\n", hdr->header_length); return false; }
@@ -49,32 +47,10 @@
enum cb_err fsp_identify(struct fsp_header *hdr, const void *fsp_blob) { - const uint8_t *raw_hdr = fsp_blob; - - if (!looks_like_fsp_header(raw_hdr)) + memcpy(hdr, fsp_blob, sizeof(struct fsp_header)); + if (!looks_like_fsp_header(hdr)) return CB_ERR;
- hdr->spec_version = read8(raw_hdr + 10); - hdr->revision = read8(raw_hdr + 11); - hdr->fsp_revision = read32(raw_hdr + 12); - memcpy(hdr->image_id, raw_hdr + 16, ARRAY_SIZE(hdr->image_id)); - hdr->image_id[ARRAY_SIZE(hdr->image_id) - 1] = '\0'; - hdr->image_size = read32(raw_hdr + 24); - hdr->image_base = read32(raw_hdr + 28); - hdr->image_attribute = read16(raw_hdr + 32); - hdr->component_attribute = read16(raw_hdr + 34); - hdr->cfg_region_offset = read32(raw_hdr + 36); - hdr->cfg_region_size = read32(raw_hdr + 40); - hdr->temp_ram_init_entry = read32(raw_hdr + 48); - hdr->temp_ram_exit_entry = read32(raw_hdr + 64); - hdr->notify_phase_entry_offset = read32(raw_hdr + 56); - hdr->memory_init_entry_offset = read32(raw_hdr + 60); - hdr->silicon_init_entry_offset = read32(raw_hdr + 68); - if (CONFIG(PLATFORM_USES_FSP2_2)) - hdr->multi_phase_si_init_entry_offset = read32(raw_hdr + 72); - if (CONFIG(PLATFORM_USES_FSP2_3)) - hdr->extended_fsp_revision = read16(raw_hdr + 76); - return CB_SUCCESS; }
@@ -192,7 +168,7 @@ struct fsp_header *hdr = &fsps_hdr; union fsp_revision revision;
- revision.val = hdr->fsp_revision; + revision.val = hdr->image_revision; snprintf(buf, FSP_VER_LEN, "%u.%u-%u.%u.%u.%u", (hdr->spec_version >> 4), hdr->spec_version & 0xf, revision.rev.major, revision.rev.minor, revision.rev.revision, revision.rev.bld_num);