<p>Richard Spiegel has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/28055">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">vendorcode/amd/pi/00670F00/Lib: Validate AccessWidth<br><br>For most io/mem/pci functions, if a AccessWidth is invalid, a default within<br>a case statement will take care of it with no ill effects. However, for<br>read/modify/write functions, though default will avoid ill effects, it does<br>causes a static analysis error. Validate that the access width is within<br>valid values for the particular interface io/mem/pci.<br><br>BUG=b:112253891<br>TEST=Build and boot grunt.<br><br>Change-Id: I962c6002fca2239bf71036a1e231f5f1ecf14c94<br>Signed-off-by: Richard Spiegel <richard.spiegel@silverbackltd.com><br>---<br>M src/vendorcode/amd/pi/00670F00/Lib/amdlib.c<br>M src/vendorcode/amd/pi/00670F00/Lib/amdlib.h<br>2 files changed, 65 insertions(+), 12 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/28055/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/vendorcode/amd/pi/00670F00/Lib/amdlib.c b/src/vendorcode/amd/pi/00670F00/Lib/amdlib.c</span><br><span>index b400bca..289336b 100644</span><br><span>--- a/src/vendorcode/amd/pi/00670F00/Lib/amdlib.c</span><br><span>+++ b/src/vendorcode/amd/pi/00670F00/Lib/amdlib.c</span><br><span>@@ -503,6 +503,39 @@</span><br><span> __asm__ volatile ("finit");</span><br><span> }</span><br><span> #endif /* IS_ENABLED(CONFIG_VENDORCODE_FULL_SUPPORT) */</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 style="color: hsl(120, 100%, 40%);">+ * Check if AccessWidth is valid, used for read modify write</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%);">+ * @param[in] AccessWidth Access width</span><br><span style="color: hsl(120, 100%, 40%);">+ * @param[in out) Control (in) = IO/MEM/PCI (out) = set INVALID_ACCESS if width invalid</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%);">+static VOID</span><br><span style="color: hsl(120, 100%, 40%);">+LibAmdCheckWidth(</span><br><span style="color: hsl(120, 100%, 40%);">+ IN ACCESS_WIDTH AccessWidth,</span><br><span style="color: hsl(120, 100%, 40%);">+ IN OUT UINT16 *Control</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%);">+ ACCESS_WIDTH Checking;</span><br><span style="color: hsl(120, 100%, 40%);">+ ACCESS_WIDTH Limit;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* only width related bits */</span><br><span style="color: hsl(120, 100%, 40%);">+ Checking = AccessWidth & 7;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (Checking > 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ Checking--;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (*Control == AccessMem)</span><br><span style="color: hsl(120, 100%, 40%);">+ Limit = AccessWidth64;</span><br><span style="color: hsl(120, 100%, 40%);">+ else</span><br><span style="color: hsl(120, 100%, 40%);">+ Limit = AccessWidth32;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (Checking >= Limit)</span><br><span style="color: hsl(120, 100%, 40%);">+ *Control |= INVALID_ACCESS;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else</span><br><span style="color: hsl(120, 100%, 40%);">+ *Control |= INVALID_ACCESS; /* AccessWidthNone is invalid */</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /*---------------------------------------------------------------------------------------*/</span><br><span> /**</span><br><span> * Read IO port</span><br><span>@@ -603,10 +636,14 @@</span><br><span> UINT32 TempData;</span><br><span> UINT32 TempMask;</span><br><span> UINT32 Value;</span><br><span style="color: hsl(0, 100%, 40%);">- LibAmdGetDataFromPtr (AccessWidth, Data, DataMask, &TempData, &TempMask);</span><br><span style="color: hsl(0, 100%, 40%);">- LibAmdIoRead (AccessWidth, IoAddress, &Value, NULL);</span><br><span style="color: hsl(0, 100%, 40%);">- Value = (Value & (~TempMask)) | TempData;</span><br><span style="color: hsl(0, 100%, 40%);">- LibAmdIoWrite (AccessWidth, IoAddress, &Value, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ UINT16 Control = AccessIo;</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdCheckWidth(AccessWidth, &Control);</span><br><span style="color: hsl(120, 100%, 40%);">+ if ((Control & INVALID_ACCESS) == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdGetDataFromPtr (AccessWidth, Data, DataMask, &TempData, &TempMask);</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdIoRead (AccessWidth, IoAddress, &Value, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ Value = (Value & (~TempMask)) | TempData;</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdIoWrite (AccessWidth, IoAddress, &Value, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> #if IS_ENABLED(CONFIG_VENDORCODE_FULL_SUPPORT)</span><br><span>@@ -744,10 +781,14 @@</span><br><span> UINT32 TempData;</span><br><span> UINT32 TempMask;</span><br><span> UINT32 Value;</span><br><span style="color: hsl(0, 100%, 40%);">- LibAmdGetDataFromPtr (AccessWidth, Data, DataMask, &TempData, &TempMask);</span><br><span style="color: hsl(0, 100%, 40%);">- LibAmdMemRead (AccessWidth, MemAddress, &Value, NULL);</span><br><span style="color: hsl(0, 100%, 40%);">- Value = (Value & (~TempMask)) | TempData;</span><br><span style="color: hsl(0, 100%, 40%);">- LibAmdMemWrite (AccessWidth, MemAddress, &Value, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ UINT16 Control = AccessMem;</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdCheckWidth(AccessWidth, &Control);</span><br><span style="color: hsl(120, 100%, 40%);">+ if ((Control & INVALID_ACCESS) == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdGetDataFromPtr (AccessWidth, Data, DataMask, &TempData, &TempMask);</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdMemRead (AccessWidth, MemAddress, &Value, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ Value = (Value & (~TempMask)) | TempData;</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdMemWrite (AccessWidth, MemAddress, &Value, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> #if IS_ENABLED(CONFIG_VENDORCODE_FULL_SUPPORT)</span><br><span>@@ -915,10 +956,14 @@</span><br><span> UINT32 TempData = 0;</span><br><span> UINT32 TempMask = 0;</span><br><span> UINT32 Value;</span><br><span style="color: hsl(0, 100%, 40%);">- LibAmdGetDataFromPtr (AccessWidth, Data, DataMask, &TempData, &TempMask);</span><br><span style="color: hsl(0, 100%, 40%);">- LibAmdPciRead (AccessWidth, PciAddress, &Value, NULL);</span><br><span style="color: hsl(0, 100%, 40%);">- Value = (Value & (~TempMask)) | TempData;</span><br><span style="color: hsl(0, 100%, 40%);">- LibAmdPciWrite (AccessWidth, PciAddress, &Value, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ UINT16 Control = AccessPci;</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdCheckWidth(AccessWidth, &Control);</span><br><span style="color: hsl(120, 100%, 40%);">+ if ((Control & INVALID_ACCESS) == 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdGetDataFromPtr (AccessWidth, Data, DataMask, &TempData, &TempMask);</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdPciRead (AccessWidth, PciAddress, &Value, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ Value = (Value & (~TempMask)) | TempData;</span><br><span style="color: hsl(120, 100%, 40%);">+ LibAmdPciWrite (AccessWidth, PciAddress, &Value, NULL);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> #if IS_ENABLED(CONFIG_VENDORCODE_FULL_SUPPORT)</span><br><span>diff --git a/src/vendorcode/amd/pi/00670F00/Lib/amdlib.h b/src/vendorcode/amd/pi/00670F00/Lib/amdlib.h</span><br><span>index 3f7fbbd..04a164c 100644</span><br><span>--- a/src/vendorcode/amd/pi/00670F00/Lib/amdlib.h</span><br><span>+++ b/src/vendorcode/amd/pi/00670F00/Lib/amdlib.h</span><br><span>@@ -48,6 +48,14 @@</span><br><span> </span><br><span> #include <agesa_headers.h></span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+typedef enum ACCESS_TYPE {</span><br><span style="color: hsl(120, 100%, 40%);">+ AccessIo = 1,</span><br><span style="color: hsl(120, 100%, 40%);">+ AccessMem,</span><br><span style="color: hsl(120, 100%, 40%);">+ AccessPci,</span><br><span style="color: hsl(120, 100%, 40%);">+} ACCESS_TYPE;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+#define INVALID_ACCESS 0x0100</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> #define IOCF8 0xCF8</span><br><span> #define IOCFC 0xCFC</span><br><span> </span><br><span></span><br></pre><p>To view, visit <a href="https://review.coreboot.org/28055">change 28055</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/28055"/><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: I962c6002fca2239bf71036a1e231f5f1ecf14c94 </div>
<div style="display:none"> Gerrit-Change-Number: 28055 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Richard Spiegel <richard.spiegel@silverbackltd.com> </div>