New subject: Change in coreboot[master]: mb/prodrive/hermes: Improve board config EEPROM handling

24 Sep 2020

Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/45686 )
Change subject: [WIP]mb/prodrive/hermes: Improve board config EEPROM handling
......................................................................
[WIP]mb/prodrive/hermes: Improve board config EEPROM handling
* Check and print errors returned reading from I2C
* Rework offset calculation by using more macros
* Get rid of stage specific preprocessor code
* Define the EEPROM layout as struct
* Make use of the defined EEPROM layout to calculate the offset
* Read the UPD to disable Vtd from EEPROM
* Verify the checksum of the 'board settings'
* Enable DeepS5 depending on the configuration value in 'board settings'
Untested.
Change-Id: I7f83d1ddb455a3d0aaec9757666d161f2091cec2
Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com
---
M src/mainboard/prodrive/hermes/eeprom.c
M src/mainboard/prodrive/hermes/mainboard.c
M src/mainboard/prodrive/hermes/ramstage.c
M src/mainboard/prodrive/hermes/romstage.c
M src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h
5 files changed, 119 insertions(+), 37 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/86/45686/1

diff --git a/src/mainboard/prodrive/hermes/eeprom.c b/src/mainboard/prodrive/hermes/eeprom.c
index bd5db5c..763f8bd 100644
--- a/src/mainboard/prodrive/hermes/eeprom.c
+++ b/src/mainboard/prodrive/hermes/eeprom.c
@@ -3,19 +3,20 @@
 #include <device/pci_ops.h>
 #include <console/console.h>
 #include <device/smbus_host.h>
+#include <crc_byte.h>
 #include "variants/baseboard/include/eeprom.h"
/*
  * Check Signature in EEPROM (M24C32-FMN6TP)
  * If signature is there we assume that that the content is valid
  */
-int check_signature(u8 addr)
+int check_fsp_signature(u8 addr, size_t offset, uint64_t fsp_signature)
 {
    u8 blob[8] = {0};
-	if (!read_write_config(addr, blob, EEPROM_OFFSET_FSP_SIGNATURE, 0, ARRAY_SIZE(blob))) {
+	if (!read_write_config(addr, blob, offset, 0, ARRAY_SIZE(blob))) {
    	// Check Signature
-		if (*(uint64_t *)blob == FSP_UPD_SIGNATURE) {
+		if (*(uint64_t *)blob == fsp_signature) {
    		printk(BIOS_DEBUG, "CFG EEPROM: Signature valid.\n");
    		return 1;
    	}
@@ -25,6 +26,30 @@
    return 0;
 }
+/*
+ * Check Checksum of board settings in EEPROM (M24C32-FMN6TP)
+ * If signature is valid we assume that the settings are also sane.
+ */
+int check_board_settings_signature(u8 addr)
+{
+	const size_t off = offsetof(struct eeprom_layout, BoardSettings);
+	struct eeprom_board_settings BoardSettings;
+	uint32_t crc;
+
+	if (read_write_config(addr, &BoardSettings, off, 0, sizeof(BoardSettings))) {
+		printk(BIOS_ERR, "CFG EEPROM: Failed to read board settings\n");
+		return 0;
+	}
+
+	crc = CRC(((uint8_t *)&BoardSettings) + sizeof(uint32_t),
+		  sizeof(BoardSettings) - sizeof(uint32_t), crc32_byte);
+	if (crc != BoardSettings.signature) {
+		printk(BIOS_ERR, "CFG EEPROM: Board settings have invalid checksum\n");
+		return 0;
+	}
+	return 1;
+}
+
 // Read data from offset and write it to offset in UPD
 bool read_write_config(u8 addr, void *blob, size_t read_offset, size_t write_offset,
    	size_t size)
diff --git a/src/mainboard/prodrive/hermes/mainboard.c b/src/mainboard/prodrive/hermes/mainboard.c
index b24dd56..ca42afb 100644
--- a/src/mainboard/prodrive/hermes/mainboard.c
+++ b/src/mainboard/prodrive/hermes/mainboard.c
@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
#include <device/device.h>
+#include "variants/baseboard/include/eeprom.h"
 #include "gpio.h"
/* FIXME: Example code below */
@@ -84,6 +85,24 @@
    mb_usb2_fp2_pwr_enable(1);
 }
+static void mainboard_init(void *chip_info)
+{
+	uint8_t value = 0;
+
+	if (!check_board_settings_signature(I2C_ADDR_EEPROM)) {
+		return;
+	}
+
+	read_write_config(I2C_ADDR_EEPROM, &value, offsetof(struct eeprom_layout, BoardSettings) +
+			  offsetof(struct eeprom_board_settings, deep_s5), 0,
+			  sizeof(value));
+
+	config_t *config = config_of_soc();
+	config->deep_s5_enable_ac = value;
+	config->deep_s5_enable_dc = value;
+}
+
 struct chip_operations mainboard_ops = {
+	.init = mainboard_init,
    .enable_dev = mainboard_enable,
 };
diff --git a/src/mainboard/prodrive/hermes/ramstage.c b/src/mainboard/prodrive/hermes/ramstage.c
index 18fc915..9130823 100644
--- a/src/mainboard/prodrive/hermes/ramstage.c
+++ b/src/mainboard/prodrive/hermes/ramstage.c
@@ -1,16 +1,18 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
+#include <console/console.h>
 #include <soc/ramstage.h>
 #include <variant/gpio.h>
 #include "variants/baseboard/include/eeprom.h"
-static fsp_params parmas_list[] = {
-	// FIXME: Fill with additional options
+static const fsp_params parmas_list[] = {
+	GET_VALUE_FSP_S_TEST_CONFIG(VtdDisableDeprecated),
 };
void mainboard_silicon_init_params(FSP_S_CONFIG *params)
 {
    size_t num = 0;
+	bool ret;
    const struct pad_config *gpio_table = get_gpio_table(&num);
/* Configure pads prior to SiliconInit() in case there's any
@@ -20,15 +22,19 @@
    params->SataLedEnable = 1;
// Overwrite params
-	if (!check_signature(I2C_ADDR_EEPROM))
+	if (!check_fsp_signature(I2C_ADDR_EEPROM,
+				 offsetof(struct eeprom_layout, FSPS_UPD),
+				 FSPS_UPD_SIGNATURE))
    	return;
-	for (u8 i = 0; i <= ARRAY_SIZE(parmas_list); i++) {
-		if (ARRAY_SIZE(parmas_list) == 0)
-			break;
-		read_write_config(I2C_ADDR_EEPROM, params, EEPROM_OFFSET_FSP_CONFIG +
-			parmas_list[i].offset,
-			EEPROM_OFFSET_FSP_CONFIG + parmas_list[i].offset,
-			parmas_list[i].size);
+	for (size_t i = 0; i < ARRAY_SIZE(parmas_list); i++) {
+		ret = read_write_config(I2C_ADDR_EEPROM, params,
+					parmas_list[i].eeprom_offset,
+					parmas_list[i].fsp_upd_offset,
+					parmas_list[i].size);
+		if (ret) {
+			printk(BIOS_ERR, "MB: Failed to read from EEPROM at addr. 0x%zx\n",
+				parmas_list[i].eeprom_offset);
+		}
    }
 }
diff --git a/src/mainboard/prodrive/hermes/romstage.c b/src/mainboard/prodrive/hermes/romstage.c
index a25c43a..9202ad8 100644
--- a/src/mainboard/prodrive/hermes/romstage.c
+++ b/src/mainboard/prodrive/hermes/romstage.c
@@ -1,31 +1,40 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
+#include <console/console.h>
 #include <soc/cnl_memcfg_init.h>
 #include <soc/romstage.h>
 #include <variant/variants.h>
 #include "variants/baseboard/include/eeprom.h"
-static fsp_params parmas_list[] = {
-	GET_VALUE(RMT),
-	GET_VALUE(HyperThreading),
-	GET_VALUE(BootFrequency)
+static const fsp_params parmas_list[] = {
+	GET_VALUE_FSP_M_CONFIG(RMT),
+	GET_VALUE_FSP_M_CONFIG(HyperThreading),
+	GET_VALUE_FSP_M_CONFIG(BootFrequency),
+	GET_VALUE_FSP_M_TEST_CONFIG(VtdDisable),
 };
void mainboard_memory_init_params(FSPM_UPD *memupd)
 {
+	bool ret;
    memupd->FspmConfig.UserBd = 7;
    memupd->FspmTestConfig.SmbusSpdWriteDisable = 0;
    memupd->FspmConfig.IedSize = 0x400000;
    cannonlake_memcfg_init(&memupd->FspmConfig, variant_memcfg_config());
// Overwrite memupd
-	if (!check_signature(I2C_ADDR_EEPROM))
+	if (!check_fsp_signature(I2C_ADDR_EEPROM,
+				 offsetof(struct eeprom_layout, FSPM_UPD),
+				 FSPM_UPD_SIGNATURE))
    	return;
for (size_t i = 0; i < ARRAY_SIZE(parmas_list); i++) {
-		read_write_config(I2C_ADDR_EEPROM, memupd, EEPROM_OFFSET_FSP_CONFIG +
-			parmas_list[i].offset,
-			EEPROM_OFFSET_FSP_CONFIG + parmas_list[i].offset,
-			parmas_list[i].size);
+		ret = read_write_config(I2C_ADDR_EEPROM, memupd,
+					parmas_list[i].eeprom_offset,
+					parmas_list[i].fsp_upd_offset,
+					parmas_list[i].size);
+		if (ret) {
+			printk(BIOS_ERR, "MB: Failed to read from EEPROM at addr. 0x%zx\n",
+				parmas_list[i].eeprom_offset);
+		}
    }
 }
diff --git a/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h b/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h
index c41ead2..2da4317 100644
--- a/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h
+++ b/src/mainboard/prodrive/hermes/variants/baseboard/include/eeprom.h
@@ -7,27 +7,50 @@
 #define HOSTC_I2C_EN (1 << 2)
 #define I2C_ADDR_EEPROM 0x57
-#if ENV_ROMSTAGE
-#define FSP_UPD_SIGNATURE FSPM_UPD_SIGNATURE
-#define EEPROM_OFFSET_FSP_SIGNATURE 0
-#define EEPROM_OFFSET_FSP_CONFIG 0
+struct eeprom_board_settings {
+	uint32_t signature;
+	uint8_t secureboot;
+	uint8_t primary_video;
+	uint8_t deep_s5;
+	uint8_t wake_on_usb;
+	uint8_t wake_on_lan;
+} __packed;
-#define GET_VALUE(x) {.offset = sizeof(FSP_UPD_HEADER) + sizeof(FSPM_ARCH_UPD) \
-		+ offsetof(FSP_M_CONFIG, x), .size = member_size(FSP_M_CONFIG, x)}
-#else
+/* The EEPROM on address 0x57 has the following vendor defined layout: */
+struct eeprom_layout {
+	uint8_t FSPM_UPD[0x600];
+	uint8_t FSPS_UPD[0x600];
+	uint8_t BoardLayout[0x400];
+	uint8_t BootOrder[0xF00];
+	union {
+		uint8_t RawBoardSetting[0x100];
+		struct eeprom_board_settings BoardSettings;
+	};
+} __packed;
-#define FSP_UPD_SIGNATURE FSPS_UPD_SIGNATURE
-#define EEPROM_OFFSET_FSP_SIGNATURE 0x0600
-#define EEPROM_OFFSET_FSP_CONFIG (EEPROM_OFFSET_FSP_SIGNATURE + sizeof(FSP_UPD_HEADER))
-#define GET_VALUE(x) {.offset = offsetof(FSP_S_CONFIG, x), \
-					.size = member_size(FSP_S_CONFIG, x)}
-#endif // ENV_ROMSTAGE
+_Static_assert(sizeof(struct eeprom_layout) == 0x2000, "EEPROM layout size missmatch");
+/*
+ * As reading the whole FSP UPD would take too much time only single UPDs of interrest are
+ * loaded at runtime. The following code eases the EEPROM to FSP UPD offset mappings.
+ */
 typedef struct {
-	size_t offset;
+	size_t eeprom_offset;
+	size_t fsp_upd_offset;
    size_t size;
 } fsp_params;
+/* Example: RMT, FSP_M_CONFIG, x:FspmTestConfig, y: FSPM_UPD */
+#define GET_VALUE(a, b, x, y) {.eeprom_offset = offsetof(struct eeprom_layout, y) + \
+			    offsetof(y, x) + offsetof(b, a), .fsp_upd_offset = \
+			    offsetof(y, x) + offsetof(b, a), .size = member_size(b, a)}
+
+#define GET_VALUE_FSP_M_CONFIG(x) GET_VALUE(x, FSP_M_CONFIG, FspmConfig, FSPM_UPD)
+#define GET_VALUE_FSP_M_TEST_CONFIG(x) GET_VALUE(x, FSP_M_TEST_CONFIG, FspmTestConfig, FSPM_UPD)
+#define GET_VALUE_FSP_S_CONFIG(x) GET_VALUE(x, FSP_S_CONFIG, FspsConfig, FSPS_UPD)
+#define GET_VALUE_FSP_S_TEST_CONFIG(x) GET_VALUE(x, FSP_S_TEST_CONFIG, FspsTestConfig, FSPS_UPD)
+
 bool read_write_config(u8 addr, void *blob, size_t read_offset, size_t write_offset,
    	size_t size);
-int check_signature(u8 addr);
+int check_fsp_signature(u8 addr, size_t offset, uint64_t fsp_signature);
+int check_board_settings_signature(u8 addr);
-- 
To view, visit https://review.coreboot.org/c/coreboot/+/45686
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7f83d1ddb455a3d0aaec9757666d161f2091cec2
Gerrit-Change-Number: 45686
Gerrit-PatchSet: 1
Gerrit-Owner: Patrick Rudolph patrick.rudolph@9elements.com
Gerrit-MessageType: newchange



    

Change in coreboot[master]: [WIP]mb/prodrive/hermes: Improve board config EEPROM handling