Change in coreboot[master]: src/arch/x86/smbios.c: Fix type 17 part number

[Richard Spiegel (Code Review)]

Richard Spiegel has uploaded this change for review. ( https://review.coreboot.org/23844

Change subject: src/arch/x86/smbios.c: Fix type 17 part number
......................................................................

src/arch/x86/smbios.c: Fix type 17 part number

Some DIMMs have invalid strings when it comes to part number (bytes
0x149-0x15c). It should be ASCIIZ with unused space filled with white
spaces (ASCII 0x20). Byte 20 should be 0, all others should be ASCII.
Create a test that detects invalid strings and replace invalid
characters with *. If a replacement was made the output string then must
be <Invalid (replaced string)>.

BUG=b:73122207
TEST=Build, boot and record serial output for kahlee while injecting
different strings to dmi17->PartNumber. Use code to examine SMBIOS,
while testing different valid and invalid strings.
Remove string injection before committing.

Change-Id: Iead2a4cb14ff28d263d7214111b637e62ebd2921
Signed-off-by: Richard Spiegel 
---
M src/arch/x86/smbios.c
M src/include/memory_info.h
2 files changed, 41 insertions(+), 14 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/23844/1

diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c
index 548e6ed..e83b739 100644
--- a/src/arch/x86/smbios.c
+++ b/src/arch/x86/smbios.c
@@ -32,6 +32,7 @@
 #if IS_ENABLED(CONFIG_CHROMEOS)
 #include <vendorcode/google/chromeos/gnvs.h>
 #endif
+#include <soc/southbridge.h>

static u8 smbios_checksum(u8 *p, u32 length)
 {
@@ -195,11 +196,34 @@
    }
 }

+/* this function will fill the corresponding part number */
+static void smbios_fill_dimm_part_number(char *part_number,
+	struct smbios_type17 *t)
+{
+	int i, invalid;
+
+	invalid = 0; /* assume valid*/
+	for (i = 0; i < DIMM_INFO_PART_NUMBER_SIZE - 1; i++) {
+		if (part_number[i] < 0x20) {
+			invalid = 1;
+			part_number[i] = '*';
+		}
+	}
+	if (invalid) {
+		char string_buffer[256];
+
+		snprintf(string_buffer, sizeof(string_buffer), "Invalid (%s)",
+								part_number);
+		t->part_number = smbios_add_string(t->eos, string_buffer);
+	} else
+		t->part_number = smbios_add_string(t->eos, part_number);
+
+}
+
 static int create_smbios_type17_for_dimm(struct dimm_info *dimm,
    				 unsigned long *current, int *handle)
 {
    struct smbios_type17 *t = (struct smbios_type17 *)*current;
-	uint8_t length;
    char locator[40];

memset(t, 0, sizeof(struct smbios_type17));
@@ -236,8 +260,7 @@

smbios_fill_dimm_manufacturer_from_id(dimm->mod_id, t);
    /* put '\0' in the end of data */
-	length = sizeof(dimm->serial);
-	dimm->serial[length - 1] = '\0';
+	dimm->serial[DIMM_INFO_SERIAL_SIZE - 1] = '\0';
    if (dimm->serial[0] == 0)
    	t->serial_number = smbios_add_string(t->eos, "None");
    else
@@ -252,10 +275,8 @@
    t->bank_locator = smbios_add_string(t->eos, locator);

/* put '\0' in the end of data */
-	length = sizeof(dimm->module_part_number);
-	dimm->module_part_number[length - 1] = '\0';
-	t->part_number = smbios_add_string(t->eos,
-				      (const char *)dimm->module_part_number);
+	dimm->module_part_number[DIMM_INFO_PART_NUMBER_SIZE - 1] = '\0';
+	smbios_fill_dimm_part_number((char *)dimm->module_part_number, t);

/* Synchronous = 1 */
    t->type_detail = 0x0080;
@@ -607,13 +628,14 @@
 unsigned long smbios_write_tables(unsigned long current)
 {
    struct smbios_entry *se;
-	unsigned long tables;
+	unsigned long tables, temp;
    int len = 0;
    int max_struct_size = 0;
    int handle = 0;

current = ALIGN(current, 16);
    printk(BIOS_DEBUG, "%s: %08lx\n", __func__, current);
+	temp = current;

se = (struct smbios_entry *)current;
    current += sizeof(struct smbios_entry);
@@ -662,5 +684,7 @@
    					    sizeof(struct smbios_entry)
    					    - 0x10);
    se->checksum = smbios_checksum((u8 *)se, sizeof(struct smbios_entry));
+	PrintBuffer(temp, 0x280, "SMBIOS");
+	PrintSmbios(temp);
    return current;
 }
diff --git a/src/include/memory_info.h b/src/include/memory_info.h
index 8569ee4..a1b3553 100644
--- a/src/include/memory_info.h
+++ b/src/include/memory_info.h
@@ -19,6 +19,10 @@
 #include <stdint.h>
 #include <compiler.h>

+#define DIMM_INFO_SERIAL_SIZE		5
+#define DIMM_INFO_PART_NUMBER_SIZE	19
+#define DIMM_INFO_TOTAL			8	/* Maximum num of dimm is 8 */
+
 /*
  * If this table is filled and put in CBMEM,
  * then these info in CBMEM will be used to generate smbios type 17 table
@@ -31,10 +35,10 @@
    uint8_t channel_num;
    uint8_t dimm_num;
    uint8_t bank_locator;
-	/* The 5th byte is '\0' for the end of string */
-	uint8_t serial[5];
-	/* The 19th byte is '\0' for the end of string */
-	uint8_t module_part_number[19];
+	/* The last byte is '\0' for the end of string */
+	uint8_t serial[DIMM_INFO_SERIAL_SIZE];
+	/* The last byte is '\0' for the end of string */
+	uint8_t module_part_number[DIMM_INFO_PART_NUMBER_SIZE];
    uint16_t mod_id;
    uint8_t mod_type;
    uint8_t bus_width;
@@ -42,8 +46,7 @@

struct memory_info {
    uint8_t dimm_cnt;
-	/* Maximum num of dimm is 8 */
-	struct dimm_info dimm[8];
+	struct dimm_info dimm[DIMM_INFO_TOTAL];
 } __packed;

#endif