[edk2-devel] [PATCH edk2-platforms v2 2/3] IpmiFeaturePkg: remove buffer temporary buffer from BMC instance structure

Mike Maslenkin posted 3 patches 1 year, 2 months ago
[edk2-devel] [PATCH edk2-platforms v2 2/3] IpmiFeaturePkg: remove buffer temporary buffer from BMC instance structure
Posted by Mike Maslenkin 1 year, 2 months ago
There is no point to have temporary buffer in BMC instance data
used only for synchronous IPMI command as a transfer buffer.
Using local variables make things much simpler.

Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
---
 .../GenericIpmi/Common/IpmiBmc.c              |  5 ++-
 .../GenericIpmi/Common/IpmiBmcCommon.h        |  1 -
 .../IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c | 39 ++++++++++---------
 .../GenericIpmi/Pei/PeiGenericIpmi.c          | 11 +++---
 .../GenericIpmi/Pei/PeiIpmiBmc.c              |  5 ++-
 .../GenericIpmi/Pei/PeiIpmiBmcDef.h           |  1 -
 .../GenericIpmi/Smm/SmmGenericIpmi.c          |  5 ++-
 7 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmc.c b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmc.c
index 03b8174e3786..a6be2f46e84e 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmc.c
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmc.c
@@ -101,6 +101,7 @@ Returns:
   IPMI_RESPONSE           *IpmiResponse;
   UINT8                   RetryCnt = IPMI_SEND_COMMAND_MAX_RETRY;
   UINT8                   Index;
+  UINT8                   TempData[MAX_TEMP_DATA];
 
   IpmiInstance = INSTANCE_FROM_SM_IPMI_BMC_THIS (This);
 
@@ -110,8 +111,8 @@ Returns:
     // response data.  Since the command format is different from the response
     // format, the buffer is cast to both structure definitions.
     //
-    IpmiCommand  = (IPMI_COMMAND*)  IpmiInstance->TempData;
-    IpmiResponse = (IPMI_RESPONSE*) IpmiInstance->TempData;
+    IpmiCommand  = (IPMI_COMMAND*)  TempData;
+    IpmiResponse = (IPMI_RESPONSE*) TempData;
 
     //
     // Send IPMI command to BMC
diff --git a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmcCommon.h b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmcCommon.h
index 1e5dfd81f1fb..06eab62aaec9 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmcCommon.h
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmcCommon.h
@@ -50,7 +50,6 @@ typedef struct {
   UINTN               Signature;
   UINT64              KcsTimeoutPeriod;
   UINT8               SlaveAddress;
-  UINT8               TempData[MAX_TEMP_DATA];
   BMC_STATUS          BmcStatus;
   UINT64              ErrorStatus;
   UINT8               SoftErrorCount;
diff --git a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c
index aeaefaad642e..8a0c596a6434 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c
@@ -84,6 +84,7 @@ Returns:
   UINT8       *TempPtr;
   UINT32      Retries;
   BOOLEAN     bResultFlag = FALSE;
+  UINT8       TempData[MAX_TEMP_DATA];
 
   //
   // Get the SELF TEST Results.
@@ -97,9 +98,9 @@ Returns:
     Retries = PcdGet8 (PcdIpmiBmcReadyDelayTimer);
   }
 
-  DataSize = sizeof (IpmiInstance->TempData);
+  DataSize = sizeof (TempData);
 
-  IpmiInstance->TempData[1] = 0;
+  TempData[1] = 0;
 
   do {
     Status = IpmiSendCommand (
@@ -109,11 +110,11 @@ Returns:
                IPMI_APP_GET_SELFTEST_RESULTS,
                NULL,
                0,
-               IpmiInstance->TempData,
+               TempData,
                &DataSize
                );
     if (Status == EFI_SUCCESS) {
-      switch (IpmiInstance->TempData[1]) {
+      switch (TempData[1]) {
         case IPMI_APP_SELFTEST_NO_ERROR:
         case IPMI_APP_SELFTEST_NOT_IMPLEMENTED:
         case IPMI_APP_SELFTEST_ERROR:
@@ -146,7 +147,7 @@ Returns:
     IpmiInstance->BmcStatus = BMC_HARDFAIL;
     return Status;
   } else {
-    DEBUG ((EFI_D_INFO, "[IPMI] BMC self-test result: %02X-%02X\n", IpmiInstance->TempData[1], IpmiInstance->TempData[2]));
+    DEBUG ((DEBUG_INFO, "[IPMI] BMC self-test result: %02X-%02X\n", TempData[1], TempData[2]));
     //
     // Copy the Self test results to Error Status.  Data will be copied as long as it
     // does not exceed the size of the ErrorStatus variable.
@@ -155,13 +156,13 @@ Returns:
          (Index < DataSize) && (Index < sizeof (IpmiInstance->ErrorStatus));
          Index++, TempPtr++
          ) {
-      *TempPtr = IpmiInstance->TempData[Index];
+      *TempPtr = TempData[Index];
     }
     //
     // Check the IPMI defined self test results.
     // Additional Cases are device specific test results.
     //
-    switch (IpmiInstance->TempData[1]) {
+    switch (TempData[1]) {
       case IPMI_APP_SELFTEST_NO_ERROR:
       case IPMI_APP_SELFTEST_NOT_IMPLEMENTED:
         IpmiInstance->BmcStatus = BMC_OK;
@@ -173,7 +174,7 @@ Returns:
         // BootBlock Firmware corruption, and Operational Firmware Corruption.  All
         // other errors are BMC soft failures.
         //
-        if ((IpmiInstance->TempData[2] & (IPMI_APP_SELFTEST_FRU_CORRUPT | IPMI_APP_SELFTEST_FW_BOOTBLOCK_CORRUPT | IPMI_APP_SELFTEST_FW_CORRUPT)) != 0) {
+        if ((TempData[2] & (IPMI_APP_SELFTEST_FRU_CORRUPT | IPMI_APP_SELFTEST_FW_BOOTBLOCK_CORRUPT | IPMI_APP_SELFTEST_FW_CORRUPT)) != 0) {
           IpmiInstance->BmcStatus = BMC_HARDFAIL;
         } else {
           IpmiInstance->BmcStatus = BMC_SOFTFAIL;
@@ -181,7 +182,7 @@ Returns:
         //
         // Check if SDR repository is empty and report it if it is.
         //
-        if ((IpmiInstance->TempData[2] & IPMI_APP_SELFTEST_SDR_REPOSITORY_EMPTY) != 0) {
+        if ((TempData[2] & IPMI_APP_SELFTEST_SDR_REPOSITORY_EMPTY) != 0) {
           if (*ErrorCount < MAX_SOFT_COUNT) {
             StatusCodeValue[*ErrorCount] = EFI_COMPUTING_UNIT_FIRMWARE_PROCESSOR | CU_FP_EC_SDR_EMPTY;
             (*ErrorCount)++;
@@ -244,6 +245,8 @@ Returns:
   SM_CTRL_INFO                    *pBmcInfo;
   IPMI_MSG_GET_BMC_EXEC_RSP       *pBmcExecContext;
   UINT32                          Retries;
+  UINT8                           TempData[MAX_TEMP_DATA];
+
 #ifdef FAST_VIDEO_SUPPORT
   EFI_VIDEOPRINT_PROTOCOL         *VideoPrintProtocol;
   EFI_STATUS                      VideoPrintStatus;
@@ -266,16 +269,16 @@ Returns:
   //
   // Get the device ID information for the BMC.
   //
-  DataSize = sizeof (IpmiInstance->TempData);
+  DataSize = sizeof (TempData);
   while (EFI_ERROR (Status = IpmiSendCommand (
                                &IpmiInstance->IpmiTransport,
                                IPMI_NETFN_APP, 0,
                                IPMI_APP_GET_DEVICE_ID,
                                NULL, 0,
-                               IpmiInstance->TempData, &DataSize))
+                               TempData, &DataSize))
          ) {
     DEBUG ((DEBUG_ERROR, "[IPMI] BMC does not respond by Get BMC DID (status: %r), %d retries left, ResponseData: 0x%lx\n",
-            Status, Retries, IpmiInstance->TempData));
+            Status, Retries, TempData));
 
     if (Retries-- == 0) {
       IpmiInstance->BmcStatus = BMC_HARDFAIL;
@@ -287,7 +290,7 @@ Returns:
     MicroSecondDelay (1*1000*1000);
   }
 
-  pBmcInfo = (SM_CTRL_INFO*)&IpmiInstance->TempData[0];
+  pBmcInfo = (SM_CTRL_INFO*)&TempData[0];
   DEBUG ((EFI_D_ERROR, "[IPMI] BMC Device ID: 0x%02X, firmware version: %d.%02X UpdateMode:%x\n", pBmcInfo->DeviceId, pBmcInfo->MajorFirmwareRev, pBmcInfo->MinorFirmwareRev,pBmcInfo->UpdateMode));
   //
   // In OpenBMC, UpdateMode: the bit 7 of byte 4 in get device id command is used for the BMC status:
@@ -303,10 +306,10 @@ Returns:
                IPMI_NETFN_FIRMWARE, 0,
                IPMI_GET_BMC_EXECUTION_CONTEXT,
                NULL, 0,
-               IpmiInstance->TempData, &DataSize
+               TempData, &DataSize
                );
 
-    pBmcExecContext = (IPMI_MSG_GET_BMC_EXEC_RSP*)&IpmiInstance->TempData[0];
+    pBmcExecContext = (IPMI_MSG_GET_BMC_EXEC_RSP*)&TempData[0];
     DEBUG ((DEBUG_INFO, "[IPMI] Operational status of BMC: 0x%x\n", pBmcExecContext->CurrentExecutionContext));
     if ((pBmcExecContext->CurrentExecutionContext == IPMI_BMC_IN_FORCED_UPDATE_MODE) &&
         !EFI_ERROR (Status)) {
@@ -324,12 +327,12 @@ Returns:
                    IPMI_NETFN_APP, 0,
                    IPMI_APP_GET_DEVICE_ID,
                    NULL, 0,
-                   IpmiInstance->TempData, &DataSize
+                   TempData, &DataSize
                    );
 
         if (!EFI_ERROR (Status)) {
-          pBmcInfo = (SM_CTRL_INFO*)&IpmiInstance->TempData[0];
-          DEBUG ((EFI_D_ERROR, "[IPMI] UpdateMode Retries: %d   pBmcInfo->UpdateMode:%x, Status: %r, Response Data: 0x%lx\n",Retries, pBmcInfo->UpdateMode, Status, IpmiInstance->TempData));
+          pBmcInfo = (SM_CTRL_INFO*)&TempData[0];
+          DEBUG ((DEBUG_ERROR, "[IPMI] UpdateMode Retries: %d   pBmcInfo->UpdateMode:%x, Status: %r, Response Data: 0x%lx\n",Retries, pBmcInfo->UpdateMode, Status, TempData));
           if (pBmcInfo->UpdateMode == BMC_READY) {
             mIpmiInstance->BmcStatus = BMC_OK;
             return EFI_SUCCESS;
diff --git a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c
index 3efb772b684f..e8b99b6900c5 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c
@@ -288,6 +288,7 @@ Returns:
   UINT32              DataSize;
   SM_CTRL_INFO        *pBmcInfo;
   UINTN               Retries;
+  UINT8               TempData[MAX_TEMP_DATA];
 
   //
   // Set up a loop to retry for up to PcdIpmiBmcReadyDelayTimer seconds. Calculate retries not timeout
@@ -298,7 +299,7 @@ Returns:
   //
   // Get the device ID information for the BMC.
   //
-  DataSize = sizeof (mIpmiInstance->TempData);
+  DataSize = sizeof (TempData);
   while (EFI_ERROR (Status = PeiIpmiSendCommand (
                                &mIpmiInstance->IpmiTransportPpi,
                                IPMI_NETFN_APP,
@@ -306,7 +307,7 @@ Returns:
                                IPMI_APP_GET_DEVICE_ID,
                                NULL,
                                0,
-                               mIpmiInstance->TempData,
+                               TempData,
                                &DataSize
                                ))) {
     DEBUG ((EFI_D_ERROR, "[IPMI] BMC does not respond (status: %r), %d retries left\n",
@@ -322,7 +323,7 @@ Returns:
     //
     MicroSecondDelay (1*1000*1000);
   }
-  pBmcInfo = (SM_CTRL_INFO*) &mIpmiInstance->TempData[0];
+  pBmcInfo = (SM_CTRL_INFO*) &TempData[0];
   DEBUG ((DEBUG_INFO, "[IPMI PEI] BMC Device ID: 0x%02X, firmware version: %d.%02X UpdateMode:%x\n",
           pBmcInfo->DeviceId, pBmcInfo->MajorFirmwareRev, pBmcInfo->MinorFirmwareRev, pBmcInfo->UpdateMode));
   //
@@ -347,11 +348,11 @@ Returns:
                  IPMI_APP_GET_DEVICE_ID,
                  NULL,
                  0,
-                 mIpmiInstance->TempData,
+                 TempData,
                  &DataSize
                  );
       if (!EFI_ERROR (Status)) {
-        pBmcInfo = (SM_CTRL_INFO*) &mIpmiInstance->TempData[0];
+        pBmcInfo = (SM_CTRL_INFO*) &TempData[0];
         DEBUG ((DEBUG_INFO, "[IPMI PEI] UpdateMode Retries:%x   pBmcInfo->UpdateMode:%x\n", Retries, pBmcInfo->UpdateMode));
         if (pBmcInfo->UpdateMode == BMC_READY) {
           mIpmiInstance->BmcStatus = BMC_OK;
diff --git a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmc.c b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmc.c
index 32665b3e2244..dbe25421ae49 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmc.c
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmc.c
@@ -99,6 +99,7 @@ Returns:
   IPMI_COMMAND                *IpmiCommand;
   IPMI_RESPONSE               *IpmiResponse;
   UINT8                       Index;
+  UINT8                       TempData[MAX_TEMP_DATA];
 
   IpmiInstance = INSTANCE_FROM_PEI_SM_IPMI_BMC_THIS (This);
 
@@ -107,8 +108,8 @@ Returns:
   // response data.  Since the command format is different from the response
   // format, the buffer is cast to both structure definitions.
   //
-  IpmiCommand   = (IPMI_COMMAND*)  IpmiInstance->TempData;
-  IpmiResponse  = (IPMI_RESPONSE*) IpmiInstance->TempData;
+  IpmiCommand   = (IPMI_COMMAND*)  TempData;
+  IpmiResponse  = (IPMI_RESPONSE*) TempData;
 
   //
   // Send IPMI command to BMC
diff --git a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmcDef.h b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmcDef.h
index 3fbe70ce629d..fc9fbacf1a2b 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmcDef.h
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmcDef.h
@@ -49,7 +49,6 @@ typedef struct {
   UINTN                  Signature;
   UINT64                 KcsTimeoutPeriod;
   UINT8                  SlaveAddress;
-  UINT8                  TempData[MAX_TEMP_DATA];
   BMC_STATUS             BmcStatus;
   UINT64                 ErrorStatus;
   UINT8                  SoftErrorCount;
diff --git a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Smm/SmmGenericIpmi.c b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Smm/SmmGenericIpmi.c
index fda215baaad0..1af2d18f791b 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Smm/SmmGenericIpmi.c
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Smm/SmmGenericIpmi.c
@@ -56,6 +56,7 @@ Returns:
   SM_CTRL_INFO             *ControllerInfo;
   UINT8                    TimeOut;
   UINT8                    Retries;
+  UINT8                    TempData[MAX_TEMP_DATA];
 
   TimeOut = 0;
   Retries = PcdGet8 (PcdIpmiBmcReadyDelayTimer);
@@ -72,7 +73,7 @@ Returns:
                IPMI_APP_GET_DEVICE_ID,
                NULL,
                0,
-               IpmiInstance->TempData,
+               TempData,
                &DataSize
                );
     if (Status == EFI_SUCCESS) {
@@ -96,7 +97,7 @@ Returns:
   // If there is no error then proceed to check the data returned by the BMC
   //
   if (!EFI_ERROR (Status)) {
-    ControllerInfo = (SM_CTRL_INFO *) IpmiInstance->TempData;
+    ControllerInfo = (SM_CTRL_INFO *) TempData;
     //
     // If the controller is in Update Mode and the maximum number of errors has not been exceeded, then
     // save the error code to the StatusCode array and increment the counter.  Set the BMC Status to indicate
-- 
2.35.3



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100883): https://edk2.groups.io/g/devel/message/100883
Mute This Topic: https://groups.io/mt/97485242/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-