[coreboot-gerrit] Change in coreboot[master]: soc/intel/broadwell: decouple PEI memory struct from coreboot header

Matt DeVillier (Code Review) gerrit at coreboot.org
Mon May 28 05:21:12 CEST 2018


Matt DeVillier has uploaded this change for review. ( https://review.coreboot.org/26598


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

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 at gmail.com>
---
M src/soc/intel/broadwell/include/soc/pei_data.h
M src/soc/intel/broadwell/romstage/raminit.c
2 files changed, 120 insertions(+), 3 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/98/26598/1

diff --git a/src/soc/intel/broadwell/include/soc/pei_data.h b/src/soc/intel/broadwell/include/soc/pei_data.h
index 339dadd..852af01 100644
--- a/src/soc/intel/broadwell/include/soc/pei_data.h
+++ b/src/soc/intel/broadwell/include/soc/pei_data.h
@@ -87,6 +87,84 @@
 	uint8_t fixed_eq;
 } __packed;
 
+#define PEI_DIMM_INFO_SERIAL_SIZE	5
+#define PEI_DIMM_INFO_PART_NUMBER_SIZE	19
+
+/**
+ * If this table is filled and put in CBMEM,
+ * then these info in CBMEM will be used to generate smbios type 17 table
+ *
+ * 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[DIMM_INFO_TOTAL];
+} __packed;
+
 struct pei_data {
 	uint32_t pei_version;
 
@@ -191,7 +269,7 @@
 	/* 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 665dad2..844ed7c 100644
--- a/src/soc/intel/broadwell/romstage/raminit.c
+++ b/src/soc/intel/broadwell/romstage/raminit.c
@@ -121,6 +121,45 @@
 
 	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));
-
+	/* Translate pei_memory_info struct data into memory_info struct */
+	memcpy(&mem_info->dimm_cnt,
+			&pei_data->meminfo.dimm_cnt, sizeof(uint8_t));
+	for (int i=0; i < DIMM_INFO_TOTAL; i++) {
+		memcpy(&mem_info->dimm[i].dimm_size,
+			&pei_data->meminfo.dimm[i].dimm_size,
+			sizeof(uint32_t));
+		memcpy(&mem_info->dimm[i].ddr_type,
+			&pei_data->meminfo.dimm[i].ddr_type,
+			sizeof(uint16_t));
+		memcpy(&mem_info->dimm[i].ddr_frequency,
+			&pei_data->meminfo.dimm[i].ddr_frequency,
+			sizeof(uint16_t));
+		memcpy(&mem_info->dimm[i].rank_per_dimm,
+			&pei_data->meminfo.dimm[i].rank_per_dimm,
+			sizeof(uint8_t));
+		memcpy(&mem_info->dimm[i].channel_num,
+			&pei_data->meminfo.dimm[i].channel_num,
+			sizeof(uint8_t));
+		memcpy(&mem_info->dimm[i].dimm_num,
+			&pei_data->meminfo.dimm[i].dimm_num,
+			sizeof(uint8_t));
+		memcpy(&mem_info->dimm[i].bank_locator,
+			&pei_data->meminfo.dimm[i].bank_locator,
+			sizeof(uint8_t));
+		memcpy(&mem_info->dimm[i].serial,
+			&pei_data->meminfo.dimm[i].serial,
+			sizeof(uint8_t) * DIMM_INFO_SERIAL_SIZE);
+		memcpy(&mem_info->dimm[i].module_part_number,
+			&pei_data->meminfo.dimm[i].module_part_number,
+			sizeof(uint32_t) * PEI_DIMM_INFO_PART_NUMBER_SIZE);
+		memcpy(&mem_info->dimm[i].mod_id,
+			&pei_data->meminfo.dimm[i].mod_id,
+			sizeof(uint16_t));
+		memcpy(&mem_info->dimm[i].mod_type,
+			&pei_data->meminfo.dimm[i].mod_type,
+			sizeof(uint8_t));
+		memcpy(&mem_info->dimm[i].bus_width,
+			&pei_data->meminfo.dimm[i].bus_width,
+			sizeof(uint8_t));
+	}
 }

-- 
To view, visit https://review.coreboot.org/26598
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I932b7b41ae1e3fd364d056a8c91f7ed5d25dbafc
Gerrit-Change-Number: 26598
Gerrit-PatchSet: 1
Gerrit-Owner: Matt DeVillier <matt.devillier at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180528/f0778223/attachment-0001.html>


More information about the coreboot-gerrit mailing list