[coreboot-gerrit] Change in coreboot[master]: vendorcode/amd/pi/00670F00/Lib: Validate AccessWidth

[Richard Spiegel (Code Review)]

Richard Spiegel has uploaded this change for review. ( https://review.coreboot.org/28055

Change subject: vendorcode/amd/pi/00670F00/Lib: Validate AccessWidth
......................................................................

vendorcode/amd/pi/00670F00/Lib: Validate AccessWidth

For most io/mem/pci functions, if a AccessWidth is invalid, a default within
a case statement will take care of it with no ill effects. However, for
read/modify/write functions, though default will avoid ill effects, it does
causes a static analysis error. Validate that the access width is within
valid values for the particular interface io/mem/pci.

BUG=b:112253891
TEST=Build and boot grunt.

Change-Id: I962c6002fca2239bf71036a1e231f5f1ecf14c94
Signed-off-by: Richard Spiegel 
---
M src/vendorcode/amd/pi/00670F00/Lib/amdlib.c
M src/vendorcode/amd/pi/00670F00/Lib/amdlib.h
2 files changed, 65 insertions(+), 12 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/28055/1

diff --git a/src/vendorcode/amd/pi/00670F00/Lib/amdlib.c b/src/vendorcode/amd/pi/00670F00/Lib/amdlib.c
index b400bca..289336b 100644
--- a/src/vendorcode/amd/pi/00670F00/Lib/amdlib.c
+++ b/src/vendorcode/amd/pi/00670F00/Lib/amdlib.c
@@ -503,6 +503,39 @@
    __asm__ volatile ("finit");
 }
 #endif /* IS_ENABLED(CONFIG_VENDORCODE_FULL_SUPPORT) */
+
+/*---------------------------------------------------------------------------------------*/
+/**
+ * Check if AccessWidth is valid, used for read modify write
+ *
+ *
+ * @param[in] AccessWidth   Access width
+ * @param[in out) Control   (in) = IO/MEM/PCI    (out) = set INVALID_ACCESS if width invalid
+ *
+ */
+static VOID
+LibAmdCheckWidth(
+  IN       ACCESS_WIDTH AccessWidth,
+  IN OUT   UINT16 *Control
+  )
+{
+  ACCESS_WIDTH Checking;
+  ACCESS_WIDTH Limit;
+
+  /* only width related bits */
+  Checking = AccessWidth & 7;
+  if (Checking > 0) {
+    Checking--;
+    if (*Control == AccessMem)
+      Limit = AccessWidth64;
+    else
+      Limit = AccessWidth32;
+    if (Checking >= Limit)
+      *Control |= INVALID_ACCESS;
+  } else
+    *Control |= INVALID_ACCESS;  /* AccessWidthNone is invalid */
+}
+
 /*---------------------------------------------------------------------------------------*/
 /**
  * Read IO port
@@ -603,10 +636,14 @@
   UINT32  TempData;
   UINT32  TempMask;
   UINT32  Value;
-  LibAmdGetDataFromPtr (AccessWidth, Data,  DataMask, &TempData, &TempMask);
-  LibAmdIoRead (AccessWidth, IoAddress, &Value, NULL);
-  Value = (Value & (~TempMask)) | TempData;
-  LibAmdIoWrite (AccessWidth, IoAddress, &Value, NULL);
+  UINT16  Control = AccessIo;
+  LibAmdCheckWidth(AccessWidth, &Control);
+  if ((Control & INVALID_ACCESS) == 0) {
+    LibAmdGetDataFromPtr (AccessWidth, Data,  DataMask, &TempData, &TempMask);
+    LibAmdIoRead (AccessWidth, IoAddress, &Value, NULL);
+    Value = (Value & (~TempMask)) | TempData;
+    LibAmdIoWrite (AccessWidth, IoAddress, &Value, NULL);
+  }
 }

#if IS_ENABLED(CONFIG_VENDORCODE_FULL_SUPPORT)
@@ -744,10 +781,14 @@
   UINT32  TempData;
   UINT32  TempMask;
   UINT32  Value;
-  LibAmdGetDataFromPtr (AccessWidth, Data,  DataMask, &TempData, &TempMask);
-  LibAmdMemRead (AccessWidth, MemAddress, &Value, NULL);
-  Value = (Value & (~TempMask)) | TempData;
-  LibAmdMemWrite (AccessWidth, MemAddress, &Value, NULL);
+  UINT16  Control = AccessMem;
+  LibAmdCheckWidth(AccessWidth, &Control);
+  if ((Control & INVALID_ACCESS) == 0) {
+    LibAmdGetDataFromPtr (AccessWidth, Data,  DataMask, &TempData, &TempMask);
+    LibAmdMemRead (AccessWidth, MemAddress, &Value, NULL);
+    Value = (Value & (~TempMask)) | TempData;
+    LibAmdMemWrite (AccessWidth, MemAddress, &Value, NULL);
+  }
 }

#if IS_ENABLED(CONFIG_VENDORCODE_FULL_SUPPORT)
@@ -915,10 +956,14 @@
   UINT32  TempData = 0;
   UINT32  TempMask = 0;
   UINT32  Value;
-  LibAmdGetDataFromPtr (AccessWidth, Data,  DataMask, &TempData, &TempMask);
-  LibAmdPciRead (AccessWidth, PciAddress, &Value, NULL);
-  Value = (Value & (~TempMask)) | TempData;
-  LibAmdPciWrite (AccessWidth, PciAddress, &Value, NULL);
+  UINT16  Control = AccessPci;
+  LibAmdCheckWidth(AccessWidth, &Control);
+  if ((Control & INVALID_ACCESS) == 0) {
+    LibAmdGetDataFromPtr (AccessWidth, Data,  DataMask, &TempData, &TempMask);
+    LibAmdPciRead (AccessWidth, PciAddress, &Value, NULL);
+    Value = (Value & (~TempMask)) | TempData;
+    LibAmdPciWrite (AccessWidth, PciAddress, &Value, NULL);
+  }
 }

#if IS_ENABLED(CONFIG_VENDORCODE_FULL_SUPPORT)
diff --git a/src/vendorcode/amd/pi/00670F00/Lib/amdlib.h b/src/vendorcode/amd/pi/00670F00/Lib/amdlib.h
index 3f7fbbd..04a164c 100644
--- a/src/vendorcode/amd/pi/00670F00/Lib/amdlib.h
+++ b/src/vendorcode/amd/pi/00670F00/Lib/amdlib.h
@@ -48,6 +48,14 @@

#include <agesa_headers.h>

+typedef enum ACCESS_TYPE {
+  AccessIo = 1,
+  AccessMem,
+  AccessPci,
+} ACCESS_TYPE;
+
+#define INVALID_ACCESS 0x0100
+
 #define IOCF8 0xCF8
 #define IOCFC 0xCFC