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 richard.spiegel@silverbackltd.com --- 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