<p>Richard Spiegel has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/23844">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">src/arch/x86/smbios.c: Fix type 17 part number<br><br>Some DIMMs have invalid strings when it comes to part number (bytes<br>0x149-0x15c). It should be ASCIIZ with unused space filled with white<br>spaces (ASCII 0x20). Byte 20 should be 0, all others should be ASCII.<br>Create a test that detects invalid strings and replace invalid<br>characters with *. If a replacement was made the output string then must<br>be <Invalid (replaced string)>.<br><br>BUG=b:73122207<br>TEST=Build, boot and record serial output for kahlee while injecting<br>different strings to dmi17->PartNumber. Use code to examine SMBIOS,<br>while testing different valid and invalid strings.<br>Remove string injection before committing.<br><br>Change-Id: Iead2a4cb14ff28d263d7214111b637e62ebd2921<br>Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com><br>---<br>M src/arch/x86/smbios.c<br>M src/include/memory_info.h<br>2 files changed, 41 insertions(+), 14 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/23844/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/arch/x86/smbios.c b/src/arch/x86/smbios.c</span><br><span>index 548e6ed..e83b739 100644</span><br><span>--- a/src/arch/x86/smbios.c</span><br><span>+++ b/src/arch/x86/smbios.c</span><br><span>@@ -32,6 +32,7 @@</span><br><span> #if IS_ENABLED(CONFIG_CHROMEOS)</span><br><span> #include <vendorcode/google/chromeos/gnvs.h></span><br><span> #endif</span><br><span style="color: hsl(120, 100%, 40%);">+#include <soc/southbridge.h></span><br><span> </span><br><span> static u8 smbios_checksum(u8 *p, u32 length)</span><br><span> {</span><br><span>@@ -195,11 +196,34 @@</span><br><span> }</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* this function will fill the corresponding part number */</span><br><span style="color: hsl(120, 100%, 40%);">+static void smbios_fill_dimm_part_number(char *part_number,</span><br><span style="color: hsl(120, 100%, 40%);">+ struct smbios_type17 *t)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ int i, invalid;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ invalid = 0; /* assume valid*/</span><br><span style="color: hsl(120, 100%, 40%);">+ for (i = 0; i < DIMM_INFO_PART_NUMBER_SIZE - 1; i++) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (part_number[i] < 0x20) {</span><br><span style="color: hsl(120, 100%, 40%);">+ invalid = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ part_number[i] = '*';</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (invalid) {</span><br><span style="color: hsl(120, 100%, 40%);">+ char string_buffer[256];</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ snprintf(string_buffer, sizeof(string_buffer), "Invalid (%s)",</span><br><span style="color: hsl(120, 100%, 40%);">+ part_number);</span><br><span style="color: hsl(120, 100%, 40%);">+ t->part_number = smbios_add_string(t->eos, string_buffer);</span><br><span style="color: hsl(120, 100%, 40%);">+ } else</span><br><span style="color: hsl(120, 100%, 40%);">+ t->part_number = smbios_add_string(t->eos, part_number);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static int create_smbios_type17_for_dimm(struct dimm_info *dimm,</span><br><span> unsigned long *current, int *handle)</span><br><span> {</span><br><span> struct smbios_type17 *t = (struct smbios_type17 *)*current;</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t length;</span><br><span> char locator[40];</span><br><span> </span><br><span> memset(t, 0, sizeof(struct smbios_type17));</span><br><span>@@ -236,8 +260,7 @@</span><br><span> </span><br><span> smbios_fill_dimm_manufacturer_from_id(dimm->mod_id, t);</span><br><span> /* put '\0' in the end of data */</span><br><span style="color: hsl(0, 100%, 40%);">- length = sizeof(dimm->serial);</span><br><span style="color: hsl(0, 100%, 40%);">- dimm->serial[length - 1] = '\0';</span><br><span style="color: hsl(120, 100%, 40%);">+ dimm->serial[DIMM_INFO_SERIAL_SIZE - 1] = '\0';</span><br><span> if (dimm->serial[0] == 0)</span><br><span> t->serial_number = smbios_add_string(t->eos, "None");</span><br><span> else</span><br><span>@@ -252,10 +275,8 @@</span><br><span> t->bank_locator = smbios_add_string(t->eos, locator);</span><br><span> </span><br><span> /* put '\0' in the end of data */</span><br><span style="color: hsl(0, 100%, 40%);">- length = sizeof(dimm->module_part_number);</span><br><span style="color: hsl(0, 100%, 40%);">- dimm->module_part_number[length - 1] = '\0';</span><br><span style="color: hsl(0, 100%, 40%);">- t->part_number = smbios_add_string(t->eos,</span><br><span style="color: hsl(0, 100%, 40%);">- (const char *)dimm->module_part_number);</span><br><span style="color: hsl(120, 100%, 40%);">+ dimm->module_part_number[DIMM_INFO_PART_NUMBER_SIZE - 1] = '\0';</span><br><span style="color: hsl(120, 100%, 40%);">+ smbios_fill_dimm_part_number((char *)dimm->module_part_number, t);</span><br><span> </span><br><span> /* Synchronous = 1 */</span><br><span> t->type_detail = 0x0080;</span><br><span>@@ -607,13 +628,14 @@</span><br><span> unsigned long smbios_write_tables(unsigned long current)</span><br><span> {</span><br><span> struct smbios_entry *se;</span><br><span style="color: hsl(0, 100%, 40%);">- unsigned long tables;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned long tables, temp;</span><br><span> int len = 0;</span><br><span> int max_struct_size = 0;</span><br><span> int handle = 0;</span><br><span> </span><br><span> current = ALIGN(current, 16);</span><br><span> printk(BIOS_DEBUG, "%s: %08lx\n", __func__, current);</span><br><span style="color: hsl(120, 100%, 40%);">+ temp = current;</span><br><span> </span><br><span> se = (struct smbios_entry *)current;</span><br><span> current += sizeof(struct smbios_entry);</span><br><span>@@ -662,5 +684,7 @@</span><br><span> sizeof(struct smbios_entry)</span><br><span> - 0x10);</span><br><span> se->checksum = smbios_checksum((u8 *)se, sizeof(struct smbios_entry));</span><br><span style="color: hsl(120, 100%, 40%);">+ PrintBuffer(temp, 0x280, "SMBIOS");</span><br><span style="color: hsl(120, 100%, 40%);">+ PrintSmbios(temp);</span><br><span> return current;</span><br><span> }</span><br><span>diff --git a/src/include/memory_info.h b/src/include/memory_info.h</span><br><span>index 8569ee4..a1b3553 100644</span><br><span>--- a/src/include/memory_info.h</span><br><span>+++ b/src/include/memory_info.h</span><br><span>@@ -19,6 +19,10 @@</span><br><span> #include <stdint.h></span><br><span> #include <compiler.h></span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+#define DIMM_INFO_SERIAL_SIZE 5</span><br><span style="color: hsl(120, 100%, 40%);">+#define DIMM_INFO_PART_NUMBER_SIZE 19</span><br><span style="color: hsl(120, 100%, 40%);">+#define DIMM_INFO_TOTAL 8 /* Maximum num of dimm is 8 */</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*</span><br><span> * If this table is filled and put in CBMEM,</span><br><span> * then these info in CBMEM will be used to generate smbios type 17 table</span><br><span>@@ -31,10 +35,10 @@</span><br><span> uint8_t channel_num;</span><br><span> uint8_t dimm_num;</span><br><span> uint8_t bank_locator;</span><br><span style="color: hsl(0, 100%, 40%);">- /* The 5th byte is '\0' for the end of string */</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t serial[5];</span><br><span style="color: hsl(0, 100%, 40%);">- /* The 19th byte is '\0' for the end of string */</span><br><span style="color: hsl(0, 100%, 40%);">- uint8_t module_part_number[19];</span><br><span style="color: hsl(120, 100%, 40%);">+ /* The last byte is '\0' for the end of string */</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t serial[DIMM_INFO_SERIAL_SIZE];</span><br><span style="color: hsl(120, 100%, 40%);">+ /* The last byte is '\0' for the end of string */</span><br><span style="color: hsl(120, 100%, 40%);">+ uint8_t module_part_number[DIMM_INFO_PART_NUMBER_SIZE];</span><br><span> uint16_t mod_id;</span><br><span> uint8_t mod_type;</span><br><span> uint8_t bus_width;</span><br><span>@@ -42,8 +46,7 @@</span><br><span> </span><br><span> struct memory_info {</span><br><span> uint8_t dimm_cnt;</span><br><span style="color: hsl(0, 100%, 40%);">- /* Maximum num of dimm is 8 */</span><br><span style="color: hsl(0, 100%, 40%);">- struct dimm_info dimm[8];</span><br><span style="color: hsl(120, 100%, 40%);">+ struct dimm_info dimm[DIMM_INFO_TOTAL];</span><br><span> } __packed;</span><br><span> </span><br><span> #endif</span><br><span></span><br></pre><p>To view, visit <a href="https://review.coreboot.org/23844">change 23844</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/23844"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: Iead2a4cb14ff28d263d7214111b637e62ebd2921 </div>
<div style="display:none"> Gerrit-Change-Number: 23844 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Spiegel <richard.spiegel@silverbackltd.com> </div>