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

Some DIMMs have invalid strings when it comes to device part number
(bytes 0x149-0x15c). From DDR4 SPD specs it should be ASCIIZ with unused
space filled with white spaces (ASCII 0x20). Byte 20 should be 0 (ASCIIZ),
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>
Reviewed-on: https://review.coreboot.org/23844
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Martin Roth <martinroth@google.com>
This commit is contained in:
Richard Spiegel 2018-02-22 10:03:39 -07:00 committed by Patrick Georgi
parent 99fd08d324
commit bd65480593
2 changed files with 36 additions and 13 deletions

View File

@ -195,11 +195,34 @@ void smbios_fill_dimm_manufacturer_from_id(uint16_t mod_id,
} }
} }
/* 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] < ' ') {
invalid = 1;
part_number[i] = '*';
}
}
if (invalid) {
char string_buffer[64];
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, static int create_smbios_type17_for_dimm(struct dimm_info *dimm,
unsigned long *current, int *handle) unsigned long *current, int *handle)
{ {
struct smbios_type17 *t = (struct smbios_type17 *)*current; struct smbios_type17 *t = (struct smbios_type17 *)*current;
uint8_t length;
char locator[40]; char locator[40];
memset(t, 0, sizeof(struct smbios_type17)); memset(t, 0, sizeof(struct smbios_type17));
@ -236,8 +259,7 @@ static int create_smbios_type17_for_dimm(struct dimm_info *dimm,
smbios_fill_dimm_manufacturer_from_id(dimm->mod_id, t); smbios_fill_dimm_manufacturer_from_id(dimm->mod_id, t);
/* put '\0' in the end of data */ /* put '\0' in the end of data */
length = sizeof(dimm->serial); dimm->serial[DIMM_INFO_SERIAL_SIZE - 1] = '\0';
dimm->serial[length - 1] = '\0';
if (dimm->serial[0] == 0) if (dimm->serial[0] == 0)
t->serial_number = smbios_add_string(t->eos, "None"); t->serial_number = smbios_add_string(t->eos, "None");
else else
@ -252,10 +274,8 @@ static int create_smbios_type17_for_dimm(struct dimm_info *dimm,
t->bank_locator = smbios_add_string(t->eos, locator); t->bank_locator = smbios_add_string(t->eos, locator);
/* put '\0' in the end of data */ /* put '\0' in the end of data */
length = sizeof(dimm->module_part_number); dimm->module_part_number[DIMM_INFO_PART_NUMBER_SIZE - 1] = '\0';
dimm->module_part_number[length - 1] = '\0'; smbios_fill_dimm_part_number((char *)dimm->module_part_number, t);
t->part_number = smbios_add_string(t->eos,
(const char *)dimm->module_part_number);
/* Synchronous = 1 */ /* Synchronous = 1 */
t->type_detail = 0x0080; t->type_detail = 0x0080;

View File

@ -19,6 +19,10 @@
#include <stdint.h> #include <stdint.h>
#include <compiler.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, * If this table is filled and put in CBMEM,
* then these info in CBMEM will be used to generate smbios type 17 table * then these info in CBMEM will be used to generate smbios type 17 table
@ -31,10 +35,10 @@ struct dimm_info {
uint8_t channel_num; uint8_t channel_num;
uint8_t dimm_num; uint8_t dimm_num;
uint8_t bank_locator; uint8_t bank_locator;
/* The 5th byte is '\0' for the end of string */ /* The last byte is '\0' for the end of string */
uint8_t serial[5]; uint8_t serial[DIMM_INFO_SERIAL_SIZE];
/* The 19th byte is '\0' for the end of string */ /* The last byte is '\0' for the end of string */
uint8_t module_part_number[19]; uint8_t module_part_number[DIMM_INFO_PART_NUMBER_SIZE];
uint16_t mod_id; uint16_t mod_id;
uint8_t mod_type; uint8_t mod_type;
uint8_t bus_width; uint8_t bus_width;
@ -42,8 +46,7 @@ struct dimm_info {
struct memory_info { struct memory_info {
uint8_t dimm_cnt; uint8_t dimm_cnt;
/* Maximum num of dimm is 8 */ struct dimm_info dimm[DIMM_INFO_TOTAL];
struct dimm_info dimm[8];
} __packed; } __packed;
#endif #endif