soc/intel/broadwell: decouple PEI memory struct from coreboot header

Recent changes to field lengths in include/memory_info.h resulted in
a mismatch between the memory_info struct the MRC blob writes to and
the struct used by coreboot to parse out data for the SMBIOS tables.
This mismatch caused type 17 SMBIOS tables to be filled incorrectly.

The solution used here is to define the memory_info struct as expected
by MRC in the pei_data header, and manually copy the data field by field
into the coreboot memory_info struct, observing the more restrictive
lengths for the two structs.

Test: build/boot google/lulu, verify SMBIOS type 17 tables correctly
populated.

Change-Id: I932b7b41ae1e3fd364d056a8c91f7ed5d25dbafc
Signed-off-by: Matt DeVillier <matt.devillier@gmail.com>
Reviewed-on: https://review.coreboot.org/26598
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>

2 files changed, 108 insertions, 4 deletions

diff --git a/src/soc/intel/broadwell/include/soc/pei_data.h b/src/soc/intel/broadwell/include/soc/pei_data.h
index 339dadd4d6..d12011e797 100644
--- a/src/soc/intel/broadwell/include/soc/pei_data.h
+++ b/src/soc/intel/broadwell/include/soc/pei_data.h
@@ -31,7 +31,6 @@
 
 #include <types.h>
 #include <compiler.h>
-#include <memory_info.h>
 
 #define PEI_VERSION 22
 
@@ -87,6 +86,87 @@ struct usb3_port_setting {
 	uint8_t fixed_eq;
 } __packed;
 
+#define PEI_DIMM_INFO_SERIAL_SIZE	5
+#define PEI_DIMM_INFO_PART_NUMBER_SIZE	19
+#define PEI_DIMM_INFO_TOTAL		8	/* Maximum num of dimm is 8 */
+
+
+/**
+ * This table is filled by the MRC blob and used to populate the mem_info
+ * struct, which is placed in CBMEM and then used to generate SMBIOS type
+ * 17 table(s)
+ *
+ * Values are specified according to the JEDEC SPD Standard.
+ */
+struct pei_dimm_info {
+	/*
+	 * Size of the module in MiB.
+	 */
+	uint32_t dimm_size;
+	/*
+	 * SMBIOS (not SPD) device type.
+	 *
+	 * See the smbios.h smbios_memory_device_type enum.
+	 */
+	uint16_t ddr_type;
+	uint16_t ddr_frequency;
+	uint8_t rank_per_dimm;
+	uint8_t channel_num;
+	uint8_t dimm_num;
+	uint8_t bank_locator;
+	/*
+	 * The last byte is '\0' for the end of string.
+	 *
+	 * Even though the SPD spec defines this field as a byte array the value
+	 * is passed directly to SMBIOS as a string, and thus must be printable
+	 * ASCII.
+	 */
+	uint8_t serial[PEI_DIMM_INFO_SERIAL_SIZE];
+	/*
+	 * The last byte is '\0' for the end of string
+	 *
+	 * Must contain only printable ASCII.
+	 */
+	uint8_t module_part_number[PEI_DIMM_INFO_PART_NUMBER_SIZE];
+	/*
+	 * SPD Manufacturer ID
+	 */
+	uint16_t mod_id;
+	/*
+	 * SPD Module Type.
+	 *
+	 * See spd.h for valid values.
+	 *
+	 * e.g., SPD_RDIMM, SPD_SODIMM, SPD_MICRO_DIMM
+	 */
+	uint8_t mod_type;
+	/*
+	 * SPD bus width.
+	 *
+	 * Bits 0 - 2 encode the primary bus width:
+	 *   0b000 = 8 bit width
+	 *   0b001 = 16 bit width
+	 *   0b010 = 32 bit width
+	 *   0b011 = 64 bit width
+	 *
+	 * Bits 3 - 4 encode the extension bits (ECC):
+	 *   0b00 = 0 extension bits
+	 *   0b01 = 8 bit of ECC
+	 *
+	 * e.g.,
+	 *   64 bit bus with 8 bits of ECC (72 bits total): 0b1011
+	 *   64 bit bus with 0 bits of ECC (64 bits total): 0b0011
+	 *
+	 * See the smbios.h smbios_memory_bus_width enum.
+	 */
+	uint8_t bus_width;
+} __packed;
+
+struct pei_memory_info {
+	uint8_t dimm_cnt;
+	struct pei_dimm_info dimm[PEI_DIMM_INFO_TOTAL];
+} __packed;
+
 struct pei_data {
 	uint32_t pei_version;
 
@@ -191,7 +271,7 @@ struct pei_data {
 	/* Data from MRC that should be saved to flash */
 	void *data_to_save;
 	int data_to_save_size;
-	struct memory_info meminfo;
+	struct pei_memory_info meminfo;
 } __packed;
 
 typedef struct pei_data PEI_DATA;
diff --git a/src/soc/intel/broadwell/romstage/raminit.c b/src/soc/intel/broadwell/romstage/raminit.c
index 665dad277e..54db3d1564 100644
--- a/src/soc/intel/broadwell/romstage/raminit.c
+++ b/src/soc/intel/broadwell/romstage/raminit.c
@@ -21,6 +21,7 @@
 #include <console/console.h>
 #include <device/pci_def.h>
 #include <lib.h>
+#include <memory_info.h>
 #include <mrc_cache.h>
 #include <string.h>
 #if IS_ENABLED(CONFIG_EC_GOOGLE_CHROMEEC)
@@ -121,6 +122,29 @@ void raminit(struct pei_data *pei_data)
 
 	printk(BIOS_DEBUG, "create cbmem for dimm information\n");
 	mem_info = cbmem_add(CBMEM_ID_MEMINFO, sizeof(struct memory_info));
-	memcpy(mem_info, &pei_data->meminfo, sizeof(struct memory_info));
-
+	memset(mem_info, 0, sizeof(*mem_info));
+	/* Translate pei_memory_info struct data into memory_info struct */
+	mem_info->dimm_cnt = pei_data->meminfo.dimm_cnt;
+	for (int i = 0; i < MIN(DIMM_INFO_TOTAL, PEI_DIMM_INFO_TOTAL); i++) {
+		struct dimm_info *dimm = &mem_info->dimm[i];
+		const struct pei_dimm_info *pei_dimm =
+			&pei_data->meminfo.dimm[i];
+		dimm->dimm_size = pei_dimm->dimm_size;
+		dimm->ddr_type = pei_dimm->ddr_type;
+		dimm->ddr_frequency = pei_dimm->ddr_frequency;
+		dimm->rank_per_dimm = pei_dimm->rank_per_dimm;
+		dimm->channel_num = pei_dimm->channel_num;
+		dimm->dimm_num = pei_dimm->dimm_num;
+		dimm->bank_locator = pei_dimm->bank_locator;
+		memcpy(&dimm->serial, &pei_dimm->serial,
+			MIN(sizeof(dimm->serial), sizeof(pei_dimm->serial)));
+		memcpy(&dimm->module_part_number,
+			&pei_dimm->module_part_number,
+			MIN(sizeof(dimm->module_part_number),
+			sizeof(pei_dimm->module_part_number)));
+		dimm->module_part_number[DIMM_INFO_PART_NUMBER_SIZE - 1] = '\0';
+		dimm->mod_id =  pei_dimm->mod_id;
+		dimm->mod_type = pei_dimm->mod_type;
+		dimm->bus_width = pei_dimm->bus_width;
+	}
 }