drivers/intel/fsp2_0: Fix running on x86_64

Add new Kconfig symbols to mark FSP binary as x86_32.
Fix the FSP headers and replace void pointers by fixed sized integers
depending on the used mode to compile the FSP.
This issue has been reported here:
intel/FSP#59

This is necessary to run on x86_64, as pointers have different size.

Add preprocessor error to warn that x86_64 FSP isn't supported by the
current code.

Tested on Intel Skylake. FSP-M no longer returns the error "Invalid
Parameter".

Change-Id: I6015005c4ee3fc2f361985cf8cff896bcefd04fb
Signed-off-by: Patrick Rudolph <[email protected]>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/48174
Tested-by: build bot (Jenkins) <[email protected]>
Reviewed-by: Angel Pons <[email protected]>

src/drivers/intel/fsp2_0/Kconfig

@@ -31,6 +31,13 @@ config PLATFORM_USES_FSP2_2
 
 if PLATFORM_USES_FSP2_0
 
+config PLATFORM_USES_FSP2_X86_32
+	bool
+	default y
+	help
+	  The FSP 2.0 runs in x86_32 protected mode.
+	  Once there's a x86_64 FSP this needs to default to n.
+
 config HAVE_INTEL_FSP_REPO
 	bool
 	help

src/drivers/intel/fsp2_0/header_display.c

@@ -19,24 +19,24 @@ void fsp_print_header_info(const struct fsp_header *hdr)
 	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%lx + 0x%zx\n",
-		hdr->image_id, hdr->image_base, hdr->image_size);
+	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, "\tConfig region        0x%zx + 0x%zx\n",
-		hdr->cfg_region_offset, hdr->cfg_region_size);
+		(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",
-						hdr->memory_init_entry_offset);
+		       (size_t)hdr->memory_init_entry_offset);
 	}
 
 	if ((hdr->component_attribute >> 12) == FSP_HDR_ATTRIB_FSPS) {
 		printk(BIOS_SPEW, "\tSilicon init offset  0x%zx\n",
-						hdr->silicon_init_entry_offset);
+		       (size_t)hdr->silicon_init_entry_offset);
 		if (CONFIG(PLATFORM_USES_FSP2_2))
 			printk(BIOS_SPEW, "\tMultiPhaseSiInit offset  0x%zx\n",
-						hdr->multi_phase_si_init_entry_offset);
+			       (size_t)hdr->multi_phase_si_init_entry_offset);
 		printk(BIOS_SPEW, "\tNotify phase offset  0x%zx\n",
-						hdr->notify_phase_entry_offset);
+		       (size_t)hdr->notify_phase_entry_offset);
 	}
 
 }

src/drivers/intel/fsp2_0/include/fsp/info_header.h

@@ -4,6 +4,7 @@
 #define _FSP2_0_INFO_HEADER_H_
 
 #include <types.h>
+#include <commonlib/bsd/compiler.h>
 
 #define FSP_HDR_OFFSET			0x94
 #if CONFIG(PLATFORM_USES_FSP2_2)
@@ -16,24 +17,29 @@
 #define FSP_HDR_ATTRIB_FSPM		2
 #define FSP_HDR_ATTRIB_FSPS		3
 
+#if CONFIG(PLATFORM_USES_FSP2_X86_32)
 struct fsp_header {
 	uint32_t fsp_revision;
-	size_t image_size;
-	uintptr_t image_base;
+	uint32_t image_size;
+	uint32_t image_base;
 	uint16_t image_attribute;
 	uint8_t spec_version;
 	uint16_t component_attribute;
-	size_t cfg_region_offset;
-	size_t cfg_region_size;
-	size_t temp_ram_init_entry;
-	size_t temp_ram_exit_entry;
-	size_t notify_phase_entry_offset;
-	size_t memory_init_entry_offset;
-	size_t silicon_init_entry_offset;
-	size_t multi_phase_si_init_entry_offset;
+	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;
+#else
+#error You need to implement this struct for x86_64 FSP
+#endif
+
 
 enum cb_err fsp_identify(struct fsp_header *hdr, const void *fsp_blob);
 

src/drivers/intel/fsp2_0/include/fsp/upd.h

@@ -21,6 +21,7 @@ struct FSP_UPD_HEADER {
 	uint8_t                       Reserved[23];
 } __packed;
 
+#if CONFIG(PLATFORM_USES_FSP2_X86_32)
 struct FSPM_ARCH_UPD {
 	///
 	/// Revision of the structure. For FSP v2.0 value is 1.
@@ -31,12 +32,12 @@ struct FSPM_ARCH_UPD {
 	/// Pointer to the non-volatile storage (NVS) data buffer.
 	/// If it is NULL it indicates the NVS data is not available.
 	///
-	void                        *NvsBufferPtr;
+	uint32_t                      NvsBufferPtr;
 	///
 	/// Pointer to the temporary stack base address to be
 	/// consumed inside FspMemoryInit() API.
 	///
-	void                        *StackBase;
+	uint32_t                      StackBase;
 	///
 	/// Temporary stack size to be consumed inside
 	/// FspMemoryInit() API.
@@ -53,7 +54,11 @@ struct FSPM_ARCH_UPD {
 	uint32_t                      BootMode;
 	uint8_t                       Reserved1[8];
 } __packed;
+#else
+#error You need to implement this struct for x86_64 FSP
+#endif
 
+#endif
 struct FSPS_ARCH_UPD {
 	///
 	/// Revision of the structure. For FSP v2.2 value is 1.

src/drivers/intel/fsp2_0/memory_init.c

@@ -87,7 +87,7 @@ static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, uint32_t fsp_version)
 	void *data;
 	size_t mrc_size;
 
-	arch_upd->NvsBufferPtr = NULL;
+	arch_upd->NvsBufferPtr = 0;
 
 	if (!CONFIG(CACHE_MRC_SETTINGS))
 		return;
@@ -101,7 +101,7 @@ static void fsp_fill_mrc_cache(FSPM_ARCH_UPD *arch_upd, uint32_t fsp_version)
 		return;
 
 	/* MRC cache found */
-	arch_upd->NvsBufferPtr = data;
+	arch_upd->NvsBufferPtr = (uintptr_t)data;
 
 	printk(BIOS_SPEW, "MRC cache found, size %zx\n", mrc_size);
 }
@@ -142,7 +142,7 @@ static enum cb_err setup_fsp_stack_frame(FSPM_ARCH_UPD *arch_upd,
 				stack_end) != CB_SUCCESS)
 		return CB_ERR;
 
-	arch_upd->StackBase = (void *)stack_begin;
+	arch_upd->StackBase = stack_begin;
 	return CB_SUCCESS;
 }
 
@@ -159,7 +159,7 @@ static enum cb_err fsp_fill_common_arch_params(FSPM_ARCH_UPD *arch_upd,
 	 * Non-CAR FSP 2.0 platforms pass a DRAM location for the FSP stack.
 	 */
 	if (CONFIG(FSP_USES_CB_STACK) || !ENV_CACHE_AS_RAM) {
-		arch_upd->StackBase = temp_ram;
+		arch_upd->StackBase = (uintptr_t)temp_ram;
 		arch_upd->StackSize = sizeof(temp_ram);
 	} else if (setup_fsp_stack_frame(arch_upd, memmap)) {
 		return CB_ERR;
@@ -237,7 +237,7 @@ static void do_fsp_memory_init(const struct fspm_context *context, bool s3wake)
 
 	fsp_version = fsp_memory_settings_version(hdr);
 
-	upd = (FSPM_UPD *)(hdr->cfg_region_offset + hdr->image_base);
+	upd = (FSPM_UPD *)(uintptr_t)(hdr->cfg_region_offset + hdr->image_base);
 
 	fsp_verify_upd_header_signature(upd->FspUpdHeader.Signature, FSPM_UPD_SIGNATURE);
 
@@ -289,12 +289,12 @@ static void do_fsp_memory_init(const struct fspm_context *context, bool s3wake)
 	post_code(POST_MEM_PREINIT_PREP_END);
 
 	/* Call FspMemoryInit */
-	fsp_raminit = (void *)(hdr->image_base + hdr->memory_init_entry_offset);
+	fsp_raminit = (void *)(uintptr_t)(hdr->image_base + hdr->memory_init_entry_offset);
 	fsp_debug_before_memory_init(fsp_raminit, upd, &fspm_upd);
 
 	post_code(POST_FSP_MEMORY_INIT);
 	timestamp_add_now(TS_FSP_MEMORY_INIT_START);
-	if (ENV_X86_64)
+	if (ENV_X86_64 && CONFIG(PLATFORM_USES_FSP2_X86_32))
 		status = protected_mode_call_2arg(fsp_raminit,
 						  (uintptr_t)&fspm_upd,
 						  (uintptr_t)fsp_get_hob_list_ptr());

src/drivers/intel/fsp2_0/notify.c

@@ -16,7 +16,7 @@ static void fsp_notify(enum fsp_notify_phase phase)
 	if (!fsps_hdr.notify_phase_entry_offset)
 		die("Notify_phase_entry_offset is zero!\n");
 
-	fspnotify = (void *) (fsps_hdr.image_base +
+	fspnotify = (void *) (uintptr_t)(fsps_hdr.image_base +
 			    fsps_hdr.notify_phase_entry_offset);
 	fsp_before_debug_notify(fspnotify, &notify_params);
 
@@ -31,7 +31,7 @@ static void fsp_notify(enum fsp_notify_phase phase)
 		post_code(POST_FSP_NOTIFY_BEFORE_END_OF_FIRMWARE);
 	}
 
-	if (ENV_X86_64)
+	if (ENV_X86_64 && CONFIG(PLATFORM_USES_FSP2_X86_32))
 		ret = protected_mode_call_1arg(fspnotify, (uintptr_t)&notify_params);
 	else
 		ret = fspnotify(&notify_params);

src/drivers/intel/fsp2_0/silicon_init.c

@@ -86,7 +86,7 @@ static void do_silicon_init(struct fsp_header *hdr)
 	struct fsp_multi_phase_params multi_phase_params;
 	struct fsp_multi_phase_get_number_of_phases_params multi_phase_get_number;
 
-	supd = (FSPS_UPD *) (hdr->cfg_region_offset + hdr->image_base);
+	supd = (FSPS_UPD *) (uintptr_t)(hdr->cfg_region_offset + hdr->image_base);
 
 	fsp_verify_upd_header_signature(supd->FspUpdHeader.Signature, FSPS_UPD_SIGNATURE);
 
@@ -110,14 +110,14 @@ static void do_silicon_init(struct fsp_header *hdr)
 		logo_entry = soc_load_logo(upd);
 
 	/* Call SiliconInit */
-	silicon_init = (void *) (hdr->image_base +
+	silicon_init = (void *) (uintptr_t)(hdr->image_base +
 				 hdr->silicon_init_entry_offset);
 	fsp_debug_before_silicon_init(silicon_init, supd, upd);
 
 	timestamp_add_now(TS_FSP_SILICON_INIT_START);
 	post_code(POST_FSP_SILICON_INIT);
 
-	if (ENV_X86_64)
+	if (ENV_X86_64 && CONFIG(PLATFORM_USES_FSP2_X86_32))
 		status = protected_mode_call_1arg(silicon_init, (uintptr_t)upd);
 	else
 		status = silicon_init(upd);
@@ -145,7 +145,7 @@ static void do_silicon_init(struct fsp_header *hdr)
 		return;
 
 	/* Call MultiPhaseSiInit */
-	multi_phase_si_init = (void *) (hdr->image_base +
+	multi_phase_si_init = (void *) (uintptr_t)(hdr->image_base +
 			 hdr->multi_phase_si_init_entry_offset);
 
 	/* Implementing multi_phase_si_init() is optional as per FSP 2.2 spec */

src/mainboard/google/hatch/romstage_spd_smbus.c

@@ -28,10 +28,10 @@ void mainboard_memory_init_params(FSPM_UPD *memupd)
 			printk(BIOS_WARNING, "Invalid SPD cache\n");
 		} else {
 			dimm_changed = check_if_dimm_changed(spd_cache, &blk);
-			if (dimm_changed && memupd->FspmArchUpd.NvsBufferPtr != NULL) {
+			if (dimm_changed && memupd->FspmArchUpd.NvsBufferPtr != 0) {
 				/* Set mrc_cache as invalid */
 				printk(BIOS_INFO, "Set mrc_cache as invalid\n");
-				memupd->FspmArchUpd.NvsBufferPtr = NULL;
+				memupd->FspmArchUpd.NvsBufferPtr = 0;
 			}
 		}
 		need_update_cache = true;

src/mainboard/purism/librem_cnl/romstage.c

@@ -59,10 +59,10 @@ void mainboard_memory_init_params(FSPM_UPD *memupd)
 			printk(BIOS_WARNING, "Invalid SPD cache\n");
 		} else {
 			dimm_changed = check_if_dimm_changed(spd_cache, &blk);
-			if (dimm_changed && memupd->FspmArchUpd.NvsBufferPtr != NULL) {
+			if (dimm_changed && memupd->FspmArchUpd.NvsBufferPtr != 0) {
 				/* Set mrc_cache as invalid */
 				printk(BIOS_INFO, "Set mrc_cache as invalid\n");
-				memupd->FspmArchUpd.NvsBufferPtr = NULL;
+				memupd->FspmArchUpd.NvsBufferPtr = 0;
 			}
 		}
 		need_update_cache = true;

src/soc/amd/picasso/romstage.c

@@ -92,7 +92,7 @@ void platform_fsp_memory_init_params_cb(FSPM_UPD *mupd, uint32_t version)
 	FSP_M_CONFIG *mcfg = &mupd->FspmConfig;
 	const struct soc_amd_picasso_config *config = config_of_soc();
 
-	mupd->FspmArchUpd.NvsBufferPtr = soc_fill_mrc_cache();
+	mupd->FspmArchUpd.NvsBufferPtr = (uintptr_t)soc_fill_mrc_cache();
 
 	mcfg->pci_express_base_addr = CONFIG_MMCONF_BASE_ADDRESS;
 	mcfg->tseg_size = CONFIG_SMM_TSEG_SIZE;

src/soc/intel/quark/romstage/fsp_params.c

@@ -84,7 +84,7 @@ void platform_fsp_memory_init_params_cb(FSPM_UPD *fspm_upd, uint32_t version)
 	/* Update the architectural UPD values. */
 	aupd = &fspm_upd->FspmArchUpd;
 	aupd->BootLoaderTolumSize = cbmem_overhead_size();
-	aupd->StackBase = (void *)(CONFIG_FSP_ESRAM_LOC - aupd->StackSize);
+	aupd->StackBase = (uintptr_t)(CONFIG_FSP_ESRAM_LOC - aupd->StackSize);
 	aupd->BootMode = FSP_BOOT_WITH_FULL_CONFIGURATION;
 
 	/* Display the ESRAM layout */
@@ -97,8 +97,8 @@ void platform_fsp_memory_init_params_cb(FSPM_UPD *fspm_upd, uint32_t version)
 			"+-------------------+ 0x%08x (CONFIG_FSP_ESRAM_LOC)\n",
 			CONFIG_FSP_ESRAM_LOC);
 		printk(BIOS_SPEW, "| FSP stack         |\n");
-		printk(BIOS_SPEW, "+-------------------+ %p\n",
-			aupd->StackBase);
+		printk(BIOS_SPEW, "+-------------------+ 0x%zx\n",
+			(size_t)aupd->StackBase);
 		printk(BIOS_SPEW, "|                   |\n");
 		printk(BIOS_SPEW, "+-------------------+ %p\n",
 			_car_unallocated_start);

src/vendorcode/amd/fsp/cezanne/fsp_h_c99.h

@@ -35,17 +35,24 @@ typedef struct __packed {
 
 _Static_assert(sizeof(FSP_UPD_HEADER) == 32, "FSP_UPD_HEADER not packed");
 
+
+#if CONFIG(PLATFORM_USES_FSP2_X86_32)
 typedef struct __packed {
 	uint8_t		Revision;
 	uint8_t		Reserved[3];
-	void		*NvsBufferPtr;
-	void		*StackBase;
+	/* Note: This ought to be void*, but that won't allow calling this binary on x86_64. */
+	uint32_t	NvsBufferPtr;
+	/* Note: This ought to be void*, but that won't allow calling this binary on x86_64. */
+	uint32_t	StackBase;
 	uint32_t	StackSize;
 	uint32_t	BootLoaderTolumSize;
 	uint32_t	BootMode;
 	uint8_t		Reserved1[8];
 } FSPM_ARCH_UPD;
 
 _Static_assert(sizeof(FSPM_ARCH_UPD) == 32, "FSPM_ARCH_UPD not packed");
+#else
+#error You need to implement this struct for x86_64 FSP
+#endif
 
 #endif /* FSP_H_C99_H */

src/vendorcode/amd/fsp/picasso/fsp_h_c99.h

@@ -35,17 +35,23 @@ typedef struct __packed {
 
 _Static_assert(sizeof(FSP_UPD_HEADER) == 32, "FSP_UPD_HEADER not packed");
 
+#if CONFIG(PLATFORM_USES_FSP2_X86_32)
 typedef struct __packed {
 	uint8_t		Revision;
 	uint8_t		Reserved[3];
-	void		*NvsBufferPtr;
-	void		*StackBase;
+	/* Note: This ought to be void*, but that won't allow calling this binary on x86_64. */
+	uint32_t	NvsBufferPtr;
+	/* Note: This ought to be void*, but that won't allow calling this binary on x86_64. */
+	uint32_t	StackBase;
 	uint32_t	StackSize;
 	uint32_t	BootLoaderTolumSize;
 	uint32_t	BootMode;
 	uint8_t		Reserved1[8];
 } FSPM_ARCH_UPD;
 
 _Static_assert(sizeof(FSPM_ARCH_UPD) == 32, "FSPM_ARCH_UPD not packed");
+#else
+#error You need to implement this struct for x86_64 FSP
+#endif
 
 #endif /* FSP_H_C99_H */

src/vendorcode/intel/edk2/UDK2015/IntelFsp2Pkg/Include/FspEas/FspApi.h

@@ -50,6 +50,7 @@ typedef struct {
   UINT8                       Reserved[23];
 } FSP_UPD_HEADER;
 
+#if CONFIG(PLATFORM_USES_FSP2_X86_32)
 ///
 /// FSPM_ARCH_UPD Configuration.
 ///
@@ -63,12 +64,16 @@ typedef struct {
   /// Pointer to the non-volatile storage (NVS) data buffer.
   /// If it is NULL it indicates the NVS data is not available.
   ///
-  VOID                        *NvsBufferPtr;
+  /// Note: This ought to be VOID*, but that won't allow calling this binary on x86_64.
+  ///
+  UINT32                      NvsBufferPtr;
   ///
   /// Pointer to the temporary stack base address to be
   /// consumed inside FspMemoryInit() API.
   ///
-  VOID                        *StackBase;
+  /// Note: This ought to be VOID*, but that won't allow calling this binary on x86_64.
+  ///
+  UINT32                      StackBase;
   ///
   /// Temporary stack size to be consumed inside
   /// FspMemoryInit() API.
@@ -85,6 +90,9 @@ typedef struct {
   UINT32                      BootMode;
   UINT8                       Reserved1[8];
 } FSPM_ARCH_UPD;
+#else
+#error You need to implement this struct for x86_64 FSP
+#endif
 
 ///
 /// FSPT_UPD_COMMON Configuration.