[coreboot-gerrit] Change in coreboot[master]: src/arch/x86/smbios.c: Fix type 17 part number
Richard Spiegel (Code Review)
gerrit at coreboot.org
Thu Feb 22 18:05:03 CET 2018
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 at 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
--
To view, visit https://review.coreboot.org/23844
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: Iead2a4cb14ff28d263d7214111b637e62ebd2921
Gerrit-Change-Number: 23844
Gerrit-PatchSet: 1
Gerrit-Owner: Richard Spiegel <richard.spiegel at silverbackltd.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20180222/db295a00/attachment-0001.html>
More information about the coreboot-gerrit
mailing list