* [PATCH edk2-platforms v2 2/3] IpmiFeaturePkg: remove buffer temporary buffer from BMC instance structure
2023-03-08 23:18 [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response Mike Maslenkin
2023-03-08 23:18 ` [PATCH edk2-platforms v2 1/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response parsing Mike Maslenkin
@ 2023-03-08 23:18 ` Mike Maslenkin
2023-03-08 23:18 ` [PATCH edk2-platforms v2 3/3] IpmiFeaturePkg: refine GetSelfTest function Mike Maslenkin
2023-03-09 2:43 ` [edk2-devel] [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response Isaac Oram
3 siblings, 0 replies; 7+ messages in thread
From: Mike Maslenkin @ 2023-03-08 23:18 UTC (permalink / raw)
Cc: devel, Mike Maslenkin, Isaac Oram, Nate DeSimone, Liming Gao
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
^ permalink raw reply related [flat|nested] 7+ messages in thread