* [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response
@ 2023-03-08 23:18 Mike Maslenkin
2023-03-08 23:18 ` [PATCH edk2-platforms v2 1/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response parsing Mike Maslenkin
` (3 more replies)
0 siblings, 4 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
This patchset fixes "Get Self Test Results" IPMI command response processing.
The first patch just makes a fix.
The second patch removes a transfer buffer from IPMI instance data as a
preparation of further improvement. It's not clear why a buffer of
a maximum size used for all commands. For the command mentioned above
response contains only 3 bytes.
The third patch drops raw byte array usage while parsing command
response because structure for this response is already defined in edk2.
Checked compilation for the Intel’s Aowanda platform using GCC5
toolchain.
v2:
patch 1:
added R-b
patch 2:
added changes to SmmGenericIpmi.c
patch 3:
renamed pSelfTestResult to SelfTestResult,
fixed initialization of SelfTestResult->CompletionCode before
IpmiSendCommand call.
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH edk2-platforms v2 1/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response parsing
2023-03-08 23:18 [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response Mike Maslenkin
@ 2023-03-08 23:18 ` Mike Maslenkin
2023-03-08 23:18 ` [PATCH edk2-platforms v2 2/3] IpmiFeaturePkg: remove buffer temporary buffer from BMC instance structure Mike Maslenkin
` (2 subsequent siblings)
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
Byte 0 of a response contains completion code for the command.
So, the examined data starts from byte 1. It's easy to make a mistake
here since specification counts response data from 1.
For the "Get Self Test Results" command Intelligent Platform Management
Interface Specification v2.0 rev 1.1 paragraph 20.4 defines response as:
+-----+---------------------------------------------------------------+
|byte | data field |
+-----+---------------------------------------------------------------+
| 1 | Completion Code |
| | |
| 2 | 55h = No error. All Self Tests Passed. |
| | 56h = Self Test function not implemented in this controller. |
| | 57h = Corrupted or inaccessible data or devices |
| | 58h = Fatal hardware error |
| | |
| 3 | For byte 2 = 55h, 56h, FFh: 00h |
| | For byte 2 = 58h, all other: Device-specific |
| | For byte 2 = 57h: self-test error bitfield. |
+-----+---------------------------------------------------------------+
Signed-off-by: Mike Maslenkin <mike.maslenkin@gmail.com>
Reviewed-by: Isaac Oram <Isaac.w.oram@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
---
.../IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c
index d788b4886723..aeaefaad642e 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c
@@ -161,7 +161,7 @@ Returns:
// Check the IPMI defined self test results.
// Additional Cases are device specific test results.
//
- switch (IpmiInstance->TempData[0]) {
+ switch (IpmiInstance->TempData[1]) {
case IPMI_APP_SELFTEST_NO_ERROR:
case IPMI_APP_SELFTEST_NOT_IMPLEMENTED:
IpmiInstance->BmcStatus = BMC_OK;
@@ -173,7 +173,7 @@ Returns:
// BootBlock Firmware corruption, and Operational Firmware Corruption. All
// other errors are BMC soft failures.
//
- if ((IpmiInstance->TempData[1] & (IPMI_APP_SELFTEST_FRU_CORRUPT | IPMI_APP_SELFTEST_FW_BOOTBLOCK_CORRUPT | IPMI_APP_SELFTEST_FW_CORRUPT)) != 0) {
+ if ((IpmiInstance->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 +181,7 @@ Returns:
//
// Check if SDR repository is empty and report it if it is.
//
- if ((IpmiInstance->TempData[1] & IPMI_APP_SELFTEST_SDR_REPOSITORY_EMPTY) != 0) {
+ if ((IpmiInstance->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)++;
--
2.35.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [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
* [PATCH edk2-platforms v2 3/3] IpmiFeaturePkg: refine GetSelfTest function
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 ` [PATCH edk2-platforms v2 2/3] IpmiFeaturePkg: remove buffer temporary buffer from BMC instance structure Mike Maslenkin
@ 2023-03-08 23:18 ` Mike Maslenkin
2023-03-09 12:01 ` [edk2-devel] " Arun K
2023-03-09 2:43 ` [edk2-devel] [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response Isaac Oram
3 siblings, 1 reply; 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
Use predefined type while accessing IPMI command returned data
instead of raw byte array.
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>
---
.../IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c
index 8a0c596a6434..79eb5f2b86e9 100644
--- a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c
@@ -86,6 +86,8 @@ Returns:
BOOLEAN bResultFlag = FALSE;
UINT8 TempData[MAX_TEMP_DATA];
+ IPMI_SELF_TEST_RESULT_RESPONSE *SelfTestResult;
+
//
// Get the SELF TEST Results.
//
@@ -100,7 +102,8 @@ Returns:
DataSize = sizeof (TempData);
- TempData[1] = 0;
+ SelfTestResult = (IPMI_SELF_TEST_RESULT_RESPONSE *) &TempData[0];
+ SelfTestResult->Result = 0;
do {
Status = IpmiSendCommand (
@@ -114,7 +117,7 @@ Returns:
&DataSize
);
if (Status == EFI_SUCCESS) {
- switch (TempData[1]) {
+ switch (SelfTestResult->Result) {
case IPMI_APP_SELFTEST_NO_ERROR:
case IPMI_APP_SELFTEST_NOT_IMPLEMENTED:
case IPMI_APP_SELFTEST_ERROR:
@@ -147,7 +150,7 @@ Returns:
IpmiInstance->BmcStatus = BMC_HARDFAIL;
return Status;
} else {
- DEBUG ((DEBUG_INFO, "[IPMI] BMC self-test result: %02X-%02X\n", TempData[1], TempData[2]));
+ DEBUG ((DEBUG_INFO, "[IPMI] BMC self-test result: %02X-%02X\n", SelfTestResult->Result, SelfTestResult->Param));
//
// 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.
@@ -162,7 +165,7 @@ Returns:
// Check the IPMI defined self test results.
// Additional Cases are device specific test results.
//
- switch (TempData[1]) {
+ switch (SelfTestResult->Result) {
case IPMI_APP_SELFTEST_NO_ERROR:
case IPMI_APP_SELFTEST_NOT_IMPLEMENTED:
IpmiInstance->BmcStatus = BMC_OK;
@@ -174,7 +177,7 @@ Returns:
// BootBlock Firmware corruption, and Operational Firmware Corruption. All
// other errors are BMC soft failures.
//
- if ((TempData[2] & (IPMI_APP_SELFTEST_FRU_CORRUPT | IPMI_APP_SELFTEST_FW_BOOTBLOCK_CORRUPT | IPMI_APP_SELFTEST_FW_CORRUPT)) != 0) {
+ if ((SelfTestResult->Param & (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;
@@ -182,7 +185,7 @@ Returns:
//
// Check if SDR repository is empty and report it if it is.
//
- if ((TempData[2] & IPMI_APP_SELFTEST_SDR_REPOSITORY_EMPTY) != 0) {
+ if ((SelfTestResult->Param & 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)++;
--
2.35.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response
2023-03-08 23:18 [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response Mike Maslenkin
` (2 preceding siblings ...)
2023-03-08 23:18 ` [PATCH edk2-platforms v2 3/3] IpmiFeaturePkg: refine GetSelfTest function Mike Maslenkin
@ 2023-03-09 2:43 ` Isaac Oram
2023-03-09 3:00 ` Isaac Oram
3 siblings, 1 reply; 7+ messages in thread
From: Isaac Oram @ 2023-03-09 2:43 UTC (permalink / raw)
To: Mike Maslenkin, devel
[-- Attachment #1: Type: text/plain, Size: 57 bytes --]
Series Reviewed-by: Isaac Oram <isaac.w.oram@intel.com>
[-- Attachment #2: Type: text/html, Size: 63 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response
2023-03-09 2:43 ` [edk2-devel] [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response Isaac Oram
@ 2023-03-09 3:00 ` Isaac Oram
0 siblings, 0 replies; 7+ messages in thread
From: Isaac Oram @ 2023-03-09 3:00 UTC (permalink / raw)
To: devel@edk2.groups.io, Oram, Isaac W, Mike Maslenkin
[-- Attachment #1: Type: text/plain, Size: 423 bytes --]
Pushed as 90ea518edf..95b58f71d9
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Isaac Oram
Sent: Wednesday, March 8, 2023 6:43 PM
To: Mike Maslenkin <mike.maslenkin@gmail.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response
Series Reviewed-by: Isaac Oram <isaac.w.oram@intel.com<mailto:isaac.w.oram@intel.com>>
[-- Attachment #2: Type: text/html, Size: 2524 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [edk2-devel] [PATCH edk2-platforms v2 3/3] IpmiFeaturePkg: refine GetSelfTest function
2023-03-08 23:18 ` [PATCH edk2-platforms v2 3/3] IpmiFeaturePkg: refine GetSelfTest function Mike Maslenkin
@ 2023-03-09 12:01 ` Arun K
0 siblings, 0 replies; 7+ messages in thread
From: Arun K @ 2023-03-09 12:01 UTC (permalink / raw)
To: Mike Maslenkin, devel
[-- Attachment #1: Type: text/plain, Size: 668 bytes --]
We are not updating the BMC status in the switch()( *default case* ), which may lead to installing the IPMI protocol for the failure case too. Could you please initialize the BMC status in the default case also?
switch ( SelfTestResult->Result ) {
case IPMI_APP_SELFTEST_NO_ERROR:
case IPMI_APP_SELFTEST_NOT_IMPLEMENTED:
.......
.....
...
case IPMI_APP_SELFTEST_ERROR:
.......
.....
...
case IPMI_APP_SELFTEST_FATAL_HW_ERROR:
.......
.....
...
*default:*
IpmiInstance->BmcStatus = BMC_HARDFAIL;
//
// Call routine to check device specific failures.
//
GetDeviceSpecificTestResults (IpmiInstance);
}
Thanks,
Arun
[-- Attachment #2: Type: text/html, Size: 2453 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-03-09 12:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 ` [PATCH edk2-platforms v2 2/3] IpmiFeaturePkg: remove buffer temporary buffer from BMC instance structure Mike Maslenkin
2023-03-08 23:18 ` [PATCH edk2-platforms v2 3/3] IpmiFeaturePkg: refine GetSelfTest function Mike Maslenkin
2023-03-09 12:01 ` [edk2-devel] " Arun K
2023-03-09 2:43 ` [edk2-devel] [PATCH edk2-platforms v2 0/3] IpmiFeaturePkg: fix IPMI GetSelfTest command response Isaac Oram
2023-03-09 3:00 ` Isaac Oram
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox