Mike,
Thanks for the fixes. There is one more instance using TempData in
SmmGenericIpmi.c. Could you please fix that as well? I noted that it isn't
currently built/included by the IpmiFeaturePkg.dsc. I will fix that.
Also, please add maintainers and reviewers for the package to commit messages,
e.g.:
Cc: Isaac Oram <isaac.w.o...@intel.com>
Cc: Nate DeSimone <nathaniel.l.desim...@intel.com>
Cc: Liming Gao <gaolim...@byosoft.com.cn>
Regards,
Isaac
-----Original Message-----
From: Mike Maslenkin <mike.maslen...@gmail.com>
Sent: Monday, February 27, 2023 3:28 PM
To: devel@edk2.groups.io
Cc: Mike Maslenkin <mike.maslen...@gmail.com>; Oram, Isaac W
<isaac.w.o...@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desim...@intel.com>; Gao, Liming <gaolim...@byosoft.com.cn>
Subject: [PATCH edk2-platforms 2/3] IpmiFeaturePkg: remove buffer temporary
buffer from BMC instance structure
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.maslen...@gmail.com>
---
.../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 -
6 files changed, 33 insertions(+), 29 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/Comm
+++ on/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 BMCdiff --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/Comm
+++ on/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
BMCdiff --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;--
2.35.3
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#100816): https://edk2.groups.io/g/devel/message/100816
Mute This Topic: https://groups.io/mt/97279449/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
