* [edk2-devel] [PATCH edk2-platforms 01/10] ManageabilityPkg: Add definition for the MCTP KCS TRAILER structure
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
@ 2023-10-16 13:18 ` Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 02/10] ManageabilityPkg: Check MCTP EIDs for reserved values Konstantin Aladyshev
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Konstantin Aladyshev @ 2023-10-16 13:18 UTC (permalink / raw)
To: devel
Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
Konstantin Aladyshev
Currently there is only a definition for the MCTP KCS HEADER structure.
Add definition for the MCTP KCS TRAILER structure as well.
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
Signed-off-by: Abner Chang <abner.chang@amd.com>
---
.../Library/ManageabilityTransportMctpLib.h | 5 +++++
.../MctpProtocol/Common/MctpProtocolCommon.c | 14 +++++++-------
2 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportMctpLib.h b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportMctpLib.h
index 43bd142f4c..462e7436e6 100644
--- a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportMctpLib.h
+++ b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportMctpLib.h
@@ -39,6 +39,11 @@ typedef struct {
UINT8 DefiningBody; ///< Message type.
UINT8 ByteCount; ///< Byte count of payload.
} MANAGEABILITY_MCTP_KCS_HEADER;
+
+typedef struct {
+ UINT8 Pec; ///< MCTP over KCS Packet Error Code.
+} MANAGEABILITY_MCTP_KCS_TRAILER;
+
#define MCTP_KCS_NETFN_LUN 0xb0
#define DEFINING_BODY_DMTF_PRE_OS_WORKING_GROUP 0x01
diff --git a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
index 1ad48efdc7..7576007f77 100644
--- a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
@@ -132,7 +132,7 @@ SetupMctpRequestTransportPacket (
MANAGEABILITY_MCTP_KCS_HEADER *MctpKcsHeader;
MCTP_TRANSPORT_HEADER *MctpTransportHeader;
MCTP_MESSAGE_HEADER *MctpMessageHeader;
- UINT8 *Pec;
+ MANAGEABILITY_MCTP_KCS_TRAILER *MctpKcsTrailer;
UINT8 *ThisPackage;
if ((PacketHeader == NULL) || (PacketHeaderSize == NULL) ||
@@ -151,8 +151,8 @@ SetupMctpRequestTransportPacket (
return EFI_OUT_OF_RESOURCES;
}
- Pec = (UINT8 *)AllocateZeroPool (sizeof (UINT8));
- if (Pec == NULL) {
+ MctpKcsTrailer = (MANAGEABILITY_MCTP_KCS_TRAILER *)AllocateZeroPool (sizeof (MANAGEABILITY_MCTP_KCS_TRAILER));
+ if (MctpKcsTrailer == NULL) {
DEBUG ((DEBUG_ERROR, "%a: Not enough resource for PEC.\n", __func__));
FreePool (MctpKcsHeader);
return EFI_OUT_OF_RESOURCES;
@@ -167,7 +167,7 @@ SetupMctpRequestTransportPacket (
if (ThisPackage == NULL) {
DEBUG ((DEBUG_ERROR, "%a: Not enough resource for package.\n", __func__));
FreePool (MctpKcsHeader);
- FreePool (Pec);
+ FreePool (MctpKcsTrailer);
return EFI_OUT_OF_RESOURCES;
}
@@ -193,14 +193,14 @@ SetupMctpRequestTransportPacket (
//
// Generate PEC follow SMBUS 2.0 specification.
- *Pec = HelperManageabilityGenerateCrc8 (MCTP_KCS_PACKET_ERROR_CODE_POLY, 0, ThisPackage, MctpKcsHeader->ByteCount);
+ MctpKcsTrailer->Pec = HelperManageabilityGenerateCrc8 (MCTP_KCS_PACKET_ERROR_CODE_POLY, 0, ThisPackage, MctpKcsHeader->ByteCount);
*PacketBody = (UINT8 *)ThisPackage;
*PacketBodySize = MctpKcsHeader->ByteCount;
- *PacketTrailer = (MANAGEABILITY_TRANSPORT_TRAILER)Pec;
+ *PacketTrailer = (MANAGEABILITY_TRANSPORT_TRAILER)MctpKcsTrailer;
*PacketHeader = (MANAGEABILITY_TRANSPORT_HEADER)MctpKcsHeader;
*PacketHeaderSize = sizeof (MANAGEABILITY_MCTP_KCS_HEADER);
- *PacketTrailerSize = 1;
+ *PacketTrailerSize = sizeof (MANAGEABILITY_MCTP_KCS_TRAILER);
return EFI_SUCCESS;
} else {
DEBUG ((DEBUG_ERROR, "%a: No implementation of building up packet.", __func__));
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109634): https://edk2.groups.io/g/devel/message/109634
Mute This Topic: https://groups.io/mt/101994938/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH edk2-platforms 02/10] ManageabilityPkg: Check MCTP EIDs for reserved values
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 01/10] ManageabilityPkg: Add definition for the MCTP KCS TRAILER structure Konstantin Aladyshev
@ 2023-10-16 13:18 ` Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 03/10] ManageabilityPkg: Support both MCTP and IPMI in KCS tranport library Konstantin Aladyshev
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Konstantin Aladyshev @ 2023-10-16 13:18 UTC (permalink / raw)
To: devel
Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
Konstantin Aladyshev
MTCP base specification marks EIDs 1-7 as reserved. Therefore return
EFI_INVALID_PARAMETER if such EIDs were provided to the
MctpSubmitMessage function.
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
Signed-off-by: Abner Chang <abner.chang@amd.com>
---
.../Universal/MctpProtocol/Dxe/MctpProtocol.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c b/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c
index 88bfd9b7e7..d0f49a1abb 100644
--- a/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c
+++ b/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c
@@ -78,6 +78,23 @@ MctpSubmitMessage (
return EFI_INVALID_PARAMETER;
}
+ //
+ // Check source EID and destination EID
+ //
+ if ((MctpSourceEndpointId >= MCTP_RESERVED_ENDPOINT_START_ID) &&
+ (MctpSourceEndpointId <= MCTP_RESERVED_ENDPOINT_END_ID)
+ ) {
+ DEBUG ((DEBUG_ERROR, "%a: The value of MCTP source EID (%x) is reserved.\n", __func__, MctpSourceEndpointId));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ if ((MctpDestinationEndpointId >= MCTP_RESERVED_ENDPOINT_START_ID) &&
+ (MctpDestinationEndpointId <= MCTP_RESERVED_ENDPOINT_END_ID)
+ ) {
+ DEBUG ((DEBUG_ERROR, "%a: The value of MCTP destination EID (%x) is reserved.\n", __func__, MctpDestinationEndpointId));
+ return EFI_INVALID_PARAMETER;
+ }
+
Status = CommonMctpSubmitMessage (
mTransportToken,
MctpType,
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109635): https://edk2.groups.io/g/devel/message/109635
Mute This Topic: https://groups.io/mt/101994939/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH edk2-platforms 03/10] ManageabilityPkg: Support both MCTP and IPMI in KCS tranport library
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 01/10] ManageabilityPkg: Add definition for the MCTP KCS TRAILER structure Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 02/10] ManageabilityPkg: Check MCTP EIDs for reserved values Konstantin Aladyshev
@ 2023-10-16 13:18 ` Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 04/10] ManageabilityPkg: Check header fields in the MCTP response Konstantin Aladyshev
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Konstantin Aladyshev @ 2023-10-16 13:18 UTC (permalink / raw)
To: devel
Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
Konstantin Aladyshev
The Manageability KCS transport library needs to support requests both
from MCTP and IPMI transports. Currently the code only handles IPMI
case correctly.
In the MCTP case the communication should be based on the MCTP-over-KCS
specification (DSP0254). This specification defines a special KCS
binding header and trailer structures that need to be present in every
MCTP message.
The header structure contains a length field, therefore response packet
size is not needed to be known beforehand.
The trailer structure contains a PEC checksum that can be used to check
itegrity of the response message.
Modify Manageability KCS transport library code to check which message
is processed (IPMI or MCTP) and handle each case correctly based on its
own specification.
Tested:
- The IPMI KCS communication is tested by Abner Chang,
- The MCTP KCS communication is tested by Konstantin Aladyshev on the
AMD EthanolX CRB.
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
Signed-off-by: Abner Chang <abner.chang@amd.com>
---
.../Common/KcsCommon.c | 284 +++++++++++++++---
.../MctpProtocol/Common/MctpProtocolCommon.c | 14 +-
2 files changed, 260 insertions(+), 38 deletions(-)
diff --git a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/KcsCommon.c b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/KcsCommon.c
index d5b54c04be..4f7e7d450f 100644
--- a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/KcsCommon.c
+++ b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/KcsCommon.c
@@ -8,16 +8,19 @@
**/
#include <Uefi.h>
#include <IndustryStandard/IpmiKcs.h>
+#include <IndustryStandard/Mctp.h>
#include <Library/BaseMemoryLib.h>
#include <Library/IoLib.h>
#include <Library/DebugLib.h>
#include <Library/ManageabilityTransportHelperLib.h>
+#include <Library/ManageabilityTransportMctpLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/TimerLib.h>
#include "ManageabilityTransportKcs.h"
extern MANAGEABILITY_TRANSPORT_KCS_HARDWARE_INFO mKcsHardwareInfo;
+extern MANAGEABILITY_TRANSPORT_KCS *mSingleSessionToken;
/**
This function waits for parameter Flag to set.
@@ -379,6 +382,218 @@ KcsTransportRead (
return EFI_SUCCESS;
}
+/**
+ This funciton checks the KCS response data according to
+ manageability protocol.
+
+ @param[in] ResponseData Pointer to response data.
+ @param[in] ResponseDataSize Size of response data.
+ @param[out] AdditionalStatus Pointer to receive the additional status.
+
+ @retval EFI_SUCCESS KCS response header is checked without error
+ @retval EFI_DEVICE_ERROR KCS response header has problem.
+**/
+EFI_STATUS
+KcsCheckResponseData (
+ IN UINT8 *ResponseData,
+ IN UINT32 ResponseDataSize,
+ OUT MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS *AdditionalStatus
+ )
+{
+ EFI_STATUS Status;
+ MANAGEABILITY_MCTP_KCS_TRAILER MctpKcsPec;
+ UINT32 PecSize;
+ UINT8 CalculatedPec;
+ CHAR16 *CompletionCodeStr;
+
+ Status = EFI_SUCCESS;
+ *AdditionalStatus = MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_NO_ERRORS;
+ if (CompareGuid (&gManageabilityProtocolMctpGuid, mSingleSessionToken->Token.ManageabilityProtocolSpecification)) {
+ //
+ // For MCTP over KCS, check PEC
+ //
+ PecSize = sizeof (MANAGEABILITY_MCTP_KCS_TRAILER) + 1; // +1 to read last dummy byte that finishes KCS transfer
+ Status = KcsTransportRead (&MctpKcsPec.Pec, &PecSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Failed to read PEC with Status(%r)\n",
+ __func__,
+ Status
+ ));
+ *AdditionalStatus = MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_ERROR;
+ return Status;
+ }
+
+ if (PecSize != sizeof (MctpKcsPec)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Received PEC size is %d instead of %d\n",
+ __func__,
+ PecSize,
+ sizeof (MctpKcsPec)
+ ));
+ *AdditionalStatus = MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_ERROR;
+ return EFI_DEVICE_ERROR;
+ }
+
+ HelperManageabilityDebugPrint ((VOID *)&MctpKcsPec.Pec, PecSize - 1, "MCTP over KCS Response PEC:\n");
+ CalculatedPec = HelperManageabilityGenerateCrc8 (MCTP_KCS_PACKET_ERROR_CODE_POLY, 0, ResponseData, ResponseDataSize);
+ if (CalculatedPec != MctpKcsPec.Pec) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Received PEC is 0x%02x instead of 0x%02x\n",
+ __func__,
+ MctpKcsPec.Pec,
+ CalculatedPec
+ ));
+ Status = EFI_DEVICE_ERROR;
+ }
+ } else if (CompareGuid (&gManageabilityProtocolIpmiGuid, mSingleSessionToken->Token.ManageabilityProtocolSpecification)) {
+ //
+ // For IPMI over KCS
+ // Check and print Completion Code
+ //
+ Status = IpmiHelperCheckCompletionCode (*ResponseData, &CompletionCodeStr, AdditionalStatus);
+ if (!EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_MANAGEABILITY_INFO, "Cc: %02x %s.\n", *((UINT8 *)ResponseData), CompletionCodeStr));
+ } else if (Status == EFI_NOT_FOUND) {
+ DEBUG ((DEBUG_ERROR, "Cc: %02x not defined in IpmiCompletionCodeMapping or invalid.\n", *((UINT8 *)ResponseData)));
+ }
+ }
+
+ return Status;
+}
+
+/**
+ This funciton reads the KCS response header according to
+ manageability protocol. Caller has to free the memory
+ allocated for response header.
+
+ @param[in] ResponseHeader Pointer to receive the response header.
+ @param[out] AdditionalStatus Pointer to receive the additional status.
+
+ @retval EFI_SUCCESS KCS response header is checked and returned
+ to caller.
+ @retval EFI_INVALID_PARAMETER One of the given parameter is incorrect.
+ @retval EFI_OUT_OF_RESOURCE Memory allocation is failed for ResponseHeader.
+ @retval EFI_DEVICE_ERROR Incorrect response header.
+**/
+EFI_STATUS
+KcsReadResponseHeader (
+ IN UINT8 **ResponseHeader,
+ OUT MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS *AdditionalStatus
+ )
+{
+ EFI_STATUS Status;
+ UINT32 RspHeaderSize;
+ UINT32 ExpectedHeaderSize;
+ UINT8 *RspHeader;
+
+ if ((ResponseHeader == NULL) || (AdditionalStatus == NULL)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ *ResponseHeader = NULL;
+ if (CompareGuid (&gManageabilityProtocolMctpGuid, mSingleSessionToken->Token.ManageabilityProtocolSpecification)) {
+
+ // For MCTP over KCS
+ ExpectedHeaderSize = sizeof (MANAGEABILITY_MCTP_KCS_HEADER);
+ DEBUG ((
+ DEBUG_MANAGEABILITY_INFO,
+ "%a: Reading MCTP over KCS response header.\n",
+ __func__
+ ));
+ } else if (CompareGuid (&gManageabilityProtocolIpmiGuid, mSingleSessionToken->Token.ManageabilityProtocolSpecification)) {
+ // For IPMI over KCS
+
+ ExpectedHeaderSize = sizeof (IPMI_KCS_RESPONSE_HEADER);
+ DEBUG ((
+ DEBUG_MANAGEABILITY_INFO,
+ "%a: Reading IPMI over KCS response header.\n",
+ __func__
+ ));
+ } else {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Unsupportted manageability protocol over KCS: %g.\n",
+ __func__,
+ mSingleSessionToken->Token.ManageabilityProtocolSpecification
+ ));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ RspHeader = (UINT8 *)AllocateZeroPool (ExpectedHeaderSize);
+ if (RspHeader == NULL) {
+ DEBUG ((DEBUG_ERROR, "Memory allocation failed for KCS response header!\n"));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ RspHeaderSize = ExpectedHeaderSize;
+ Status = KcsTransportRead (RspHeader, &RspHeaderSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Failed to read KCS response header Status(%r)\n",
+ __func__,
+ Status
+ ));
+ FreePool (RspHeader);
+ *AdditionalStatus = MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_ERROR;
+ return Status;
+ }
+
+ if (RspHeaderSize != 0) {
+ HelperManageabilityDebugPrint ((VOID *)RspHeader, RspHeaderSize, "KCS Response Header:\n");
+ }
+
+ if (ExpectedHeaderSize != RspHeaderSize) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "The size (%d bytes) of returned resposne header is not the same as expection (%d bytes)!\n",
+ RspHeaderSize,
+ ExpectedHeaderSize
+ ));
+ FreePool (RspHeader);
+ return EFI_DEVICE_ERROR;
+ }
+
+ if (CompareGuid (&gManageabilityProtocolMctpGuid, mSingleSessionToken->Token.ManageabilityProtocolSpecification)) {
+ //
+ // MCTP over KCS
+ //
+ if (((MANAGEABILITY_MCTP_KCS_HEADER *)RspHeader)->NetFunc != MCTP_KCS_NETFN_LUN) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! MANAGEABILITY_MCTP_KCS_HEADER.NetFunc is equal 0x%02x instead of 0x%02x\n",
+ __func__,
+ ((MANAGEABILITY_MCTP_KCS_HEADER *)RspHeader)->NetFunc,
+ MCTP_KCS_NETFN_LUN
+ ));
+ FreePool (RspHeader);
+ *AdditionalStatus = MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_ERROR;
+ return EFI_DEVICE_ERROR;
+ }
+
+ if (((MANAGEABILITY_MCTP_KCS_HEADER *)RspHeader)->DefiningBody != DEFINING_BODY_DMTF_PRE_OS_WORKING_GROUP) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! MANAGEABILITY_MCTP_KCS_HEADER.DefiningBody is equal 0x%02x instead of 0x%02x\n",
+ __func__,
+ ((MANAGEABILITY_MCTP_KCS_HEADER *)RspHeader)->DefiningBody,
+ DEFINING_BODY_DMTF_PRE_OS_WORKING_GROUP
+ ));
+ FreePool (RspHeader);
+ *AdditionalStatus = MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_ERROR;
+ return EFI_DEVICE_ERROR;
+ }
+ }
+
+ *ResponseHeader = RspHeader;
+ *AdditionalStatus = MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_NO_ERRORS;
+ return EFI_SUCCESS;
+}
+
/**
This service communicates with BMC using KCS protocol.
@@ -423,11 +638,9 @@ KcsTransportSendCommand (
OUT MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS *AdditionalStatus
)
{
- EFI_STATUS Status;
- UINT32 RspHeaderSize;
- IPMI_KCS_RESPONSE_HEADER RspHeader;
- UINT32 ExpectedResponseDataSize;
- CHAR16 *CompletionCodeStr;
+ EFI_STATUS Status;
+ UINT8 *RspHeader;
+ UINT32 ExpectedResponseDataSize;
if ((RequestData != NULL) && (RequestDataSize == 0)) {
DEBUG ((DEBUG_ERROR, "%a: Mismatched values of RequestData and RequestDataSize\n", __func__));
@@ -467,56 +680,59 @@ KcsTransportSendCommand (
RequestDataSize
);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "KCS Write Failed with Status(%r)", Status));
+ DEBUG ((DEBUG_ERROR, "KCS Write Failed with Status(%r)\n", Status));
return Status;
}
+ }
+ if ((ResponseData != NULL) && (ResponseDataSize != NULL) && (*ResponseDataSize != 0)) {
//
// Read the response header
- RspHeaderSize = sizeof (IPMI_KCS_RESPONSE_HEADER);
- Status = KcsTransportRead ((UINT8 *)&RspHeader, &RspHeaderSize);
+ //
+ Status = KcsReadResponseHeader (&RspHeader, AdditionalStatus);
if (EFI_ERROR (Status)) {
- DEBUG ((
- DEBUG_ERROR,
- "KCS read response header failed Status(%r), " \
- "RspNetFunctionLun = 0x%x, " \
- "Comamnd = 0x%x \n",
- Status,
- RspHeader.NetFunc,
- RspHeader.Command
- ));
return (Status);
}
//
- // Print out the response payloads.
- HelperManageabilityDebugPrint ((VOID *)&RspHeader, RspHeaderSize, "KCS Response Header:\n");
- }
+ // Override ResposeDataSize if the manageability protocol is MCTP.
+ //
+ if (CompareGuid (&gManageabilityProtocolMctpGuid, mSingleSessionToken->Token.ManageabilityProtocolSpecification)) {
+ if (*ResponseDataSize < ((MANAGEABILITY_MCTP_KCS_HEADER *)RspHeader)->ByteCount) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! MANAGEABILITY_MCTP_KCS_HEADER.ByteCount (0x%02x) is bigger than provided buffer (0x%02x)\n",
+ __func__,
+ ((MANAGEABILITY_MCTP_KCS_HEADER *)RspHeader)->ByteCount,
+ *ResponseDataSize
+ ));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ *ResponseDataSize = ((MANAGEABILITY_MCTP_KCS_HEADER *)RspHeader)->ByteCount;
+ }
+ FreePool (RspHeader);
- if ((ResponseData != NULL) && (ResponseDataSize != NULL) && (*ResponseDataSize != 0)) {
ExpectedResponseDataSize = *ResponseDataSize;
- Status = KcsTransportRead ((UINT8 *)ResponseData, ResponseDataSize);
+ Status = KcsTransportRead (ResponseData, ResponseDataSize);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "KCS response read Failed with Status(%r)", Status));
+ DEBUG ((DEBUG_ERROR, "KCS response read Failed with Status(%r)\n", Status));
}
- //
// Print out the response payloads.
if (*ResponseDataSize != 0) {
if (ExpectedResponseDataSize != *ResponseDataSize) {
- DEBUG ((DEBUG_ERROR, "Expected KCS response size : %d is not matched to returned size : %d.\n", ExpectedResponseDataSize, *ResponseDataSize));
- Status = EFI_DEVICE_ERROR;
+ DEBUG ((
+ DEBUG_ERROR,
+ "Expected KCS response size : %d is not matched to returned size : %d.\n",
+ ExpectedResponseDataSize,
+ *ResponseDataSize
+ ));
+ return EFI_DEVICE_ERROR;
}
HelperManageabilityDebugPrint ((VOID *)ResponseData, (UINT32)*ResponseDataSize, "KCS Response Data:\n");
-
- // Print Completion Code
- Status = IpmiHelperCheckCompletionCode (*((UINT8 *)ResponseData), &CompletionCodeStr, AdditionalStatus);
- if (!EFI_ERROR (Status)) {
- DEBUG ((DEBUG_MANAGEABILITY_INFO, "Cc: %02x %s.\n", *((UINT8 *)ResponseData), CompletionCodeStr));
- } else if (Status == EFI_NOT_FOUND) {
- DEBUG ((DEBUG_MANAGEABILITY_INFO, "Cc: %02x not defined in IpmiCompletionCodeMapping or invalid.\n", *((UINT8 *)ResponseData)));
- }
+ Status = KcsCheckResponseData (ResponseData, *ResponseDataSize, AdditionalStatus);
} else {
DEBUG ((DEBUG_ERROR, "No response, can't determine Completion Code.\n"));
}
diff --git a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
index 7576007f77..e560c638d5 100644
--- a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
@@ -267,6 +267,9 @@ CommonMctpSubmitMessage (
MANAGEABILITY_TRANSPORT_TRAILER MctpTransportTrailer;
MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES *MultiPackages;
MANAGEABILITY_TRANSMISSION_PACKAGE_ATTR *ThisPackage;
+ UINT8 *ResponseBuffer;
+ MCTP_TRANSPORT_HEADER *MctpTransportResponseHeader;
+ MCTP_MESSAGE_HEADER *MctpMessageResponseHeader;
if (TransportToken == NULL) {
DEBUG ((DEBUG_ERROR, "%a: No transport toke for MCTP\n", __func__));
@@ -435,11 +438,12 @@ CommonMctpSubmitMessage (
ThisPackage++;
}
+ ResponseBuffer = (UINT8 *)AllocatePool (*ResponseDataSize + sizeof (MCTP_TRANSPORT_HEADER) + sizeof (MCTP_MESSAGE_HEADER));
// Receive packet.
TransferToken.TransmitPackage.TransmitPayload = NULL;
TransferToken.TransmitPackage.TransmitSizeInByte = 0;
- TransferToken.ReceivePackage.ReceiveBuffer = ResponseData;
- TransferToken.ReceivePackage.ReceiveSizeInByte = *ResponseDataSize;
+ TransferToken.ReceivePackage.ReceiveBuffer = ResponseBuffer;
+ TransferToken.ReceivePackage.ReceiveSizeInByte = *ResponseDataSize + sizeof (MCTP_TRANSPORT_HEADER) + sizeof (MCTP_MESSAGE_HEADER);
TransferToken.TransmitHeader = NULL;
TransferToken.TransmitHeaderSize = 0;
TransferToken.TransmitTrailer = NULL;
@@ -461,8 +465,10 @@ CommonMctpSubmitMessage (
// Return transfer status.
//
*AdditionalTransferError = TransferToken.TransportAdditionalStatus;
- *ResponseDataSize = TransferToken.ReceivePackage.ReceiveSizeInByte;
- Status = TransferToken.TransferStatus;
+ *ResponseDataSize = TransferToken.ReceivePackage.ReceiveSizeInByte - sizeof (MCTP_TRANSPORT_HEADER) - sizeof (MCTP_MESSAGE_HEADER);
+ CopyMem (ResponseData, ResponseBuffer + sizeof (MCTP_TRANSPORT_HEADER) + sizeof (MCTP_MESSAGE_HEADER), *ResponseDataSize);
+ FreePool (ResponseBuffer);
+ Status = TransferToken.TransferStatus;
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "%a: Failed to send MCTP command over %s: %r\n", __func__, mTransportName, Status));
return Status;
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109636): https://edk2.groups.io/g/devel/message/109636
Mute This Topic: https://groups.io/mt/101994941/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH edk2-platforms 04/10] ManageabilityPkg: Check header fields in the MCTP response
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
` (2 preceding siblings ...)
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 03/10] ManageabilityPkg: Support both MCTP and IPMI in KCS tranport library Konstantin Aladyshev
@ 2023-10-16 13:18 ` Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 05/10] ManageabilityPkg: Correct typo in MCTP destination EID field Konstantin Aladyshev
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Konstantin Aladyshev @ 2023-10-16 13:18 UTC (permalink / raw)
To: devel
Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
Konstantin Aladyshev
Add checks for the MCTP header fields in the MCTP response.
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
.../MctpProtocol/Common/MctpProtocolCommon.c | 82 +++++++++++++++++++
1 file changed, 82 insertions(+)
diff --git a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
index e560c638d5..5844d54eb2 100644
--- a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
@@ -461,6 +461,88 @@ CommonMctpSubmitMessage (
&TransferToken
);
+ MctpTransportResponseHeader = (MCTP_TRANSPORT_HEADER *)ResponseBuffer;
+ if (MctpTransportResponseHeader->Bits.HeaderVersion != MCTP_KCS_HEADER_VERSION) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Response HeaderVersion (0x%02x) doesn't match MCTP_KCS_HEADER_VERSION (0x%02x)\n",
+ __func__,
+ MctpTransportResponseHeader->Bits.HeaderVersion,
+ MCTP_KCS_HEADER_VERSION
+ ));
+ FreePool (ResponseBuffer);
+ return EFI_DEVICE_ERROR;
+ }
+ if (MctpTransportResponseHeader->Bits.MessageTag != MCTP_MESSAGE_TAG) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Response MessageTag (0x%02x) doesn't match MCTP_MESSAGE_TAG (0x%02x)\n",
+ __func__,
+ MctpTransportResponseHeader->Bits.MessageTag,
+ MCTP_MESSAGE_TAG
+ ));
+ FreePool (ResponseBuffer);
+ return EFI_DEVICE_ERROR;
+ }
+ if (MctpTransportResponseHeader->Bits.TagOwner != MCTP_MESSAGE_TAG_OWNER_RESPONSE) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Response TagOwner (0x%02x) doesn't match MCTP_MESSAGE_TAG_OWNER_RESPONSE (0x%02x)\n",
+ __func__,
+ MctpTransportResponseHeader->Bits.TagOwner,
+ MCTP_MESSAGE_TAG_OWNER_RESPONSE
+ ));
+ FreePool (ResponseBuffer);
+ return EFI_DEVICE_ERROR;
+ }
+ if (MctpTransportResponseHeader->Bits.SourceEndpointId != MctpDestinationEndpointId) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Response SrcEID (0x%02x) doesn't match sent EID (0x%02x)\n",
+ __func__,
+ MctpTransportResponseHeader->Bits.SourceEndpointId,
+ MctpDestinationEndpointId
+ ));
+ FreePool (ResponseBuffer);
+ return EFI_DEVICE_ERROR;
+ }
+ if (MctpTransportResponseHeader->Bits.DestinationEndpointId != MctpSourceEndpointId) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Response DestEID (0x%02x) doesn't match local EID (0x%02x)\n",
+ __func__,
+ MctpTransportResponseHeader->Bits.DestinationEndpointId,
+ MctpSourceEndpointId
+ ));
+ FreePool (ResponseBuffer);
+ return EFI_DEVICE_ERROR;
+ }
+
+ MctpMessageResponseHeader = (MCTP_MESSAGE_HEADER *)(MctpTransportResponseHeader + 1);
+ if (MctpMessageResponseHeader->Bits.MessageType != MctpType) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Response MessageType (0x%02x) doesn't match sent MessageType (0x%02x)\n",
+ __func__,
+ MctpMessageResponseHeader->Bits.MessageType,
+ MctpType
+ ));
+ FreePool (ResponseBuffer);
+ return EFI_DEVICE_ERROR;
+ }
+
+ if (MctpMessageResponseHeader->Bits.IntegrityCheck != (UINT8)RequestDataIntegrityCheck) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Response IntegrityCheck (%d) doesn't match sent IntegrityCheck (%d)\n",
+ __func__,
+ MctpMessageResponseHeader->Bits.IntegrityCheck,
+ (UINT8)RequestDataIntegrityCheck
+ ));
+ FreePool (ResponseBuffer);
+ return EFI_DEVICE_ERROR;
+ }
+
//
// Return transfer status.
//
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109637): https://edk2.groups.io/g/devel/message/109637
Mute This Topic: https://groups.io/mt/101994942/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH edk2-platforms 05/10] ManageabilityPkg: Correct typo in MCTP destination EID field
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
` (3 preceding siblings ...)
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 04/10] ManageabilityPkg: Check header fields in the MCTP response Konstantin Aladyshev
@ 2023-10-16 13:18 ` Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 06/10] ManageabilityPkg: Update the algorithm of using MCTP endpoint ID PCD Konstantin Aladyshev
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Konstantin Aladyshev @ 2023-10-16 13:18 UTC (permalink / raw)
To: devel
Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
Konstantin Aladyshev
Correct wrong structure member used for MCTP destination EID.
Signed-off-by: Abner Chang <abner.chang@amd.com>
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
.../Universal/PldmProtocol/Common/PldmProtocolCommon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
index ce1e2cba95..1c4506d87f 100644
--- a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
@@ -119,7 +119,7 @@ SetupPldmRequestTransportPacket (
}
MctpHeader->SourceEndpointId = PcdGet8 (PcdMctpSourceEndpointId);
- MctpHeader->SourceEndpointId = PcdGet8 (PcdMctpDestinationEndpointId);
+ MctpHeader->DestinationEndpointId = PcdGet8 (PcdMctpDestinationEndpointId);
MctpHeader->MessageHeader.IntegrityCheck = FALSE;
MctpHeader->MessageHeader.MessageType = MCTP_MESSAGE_TYPE_PLDM;
*PacketHeader = (MANAGEABILITY_TRANSPORT_HEADER *)MctpHeader;
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109638): https://edk2.groups.io/g/devel/message/109638
Mute This Topic: https://groups.io/mt/101994943/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH edk2-platforms 06/10] ManageabilityPkg: Update the algorithm of using MCTP endpoint ID PCD
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
` (4 preceding siblings ...)
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 05/10] ManageabilityPkg: Correct typo in MCTP destination EID field Konstantin Aladyshev
@ 2023-10-16 13:18 ` Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 07/10] ManageabilityPkg: Correct value for the MCTP TAG_OWNER response bit Konstantin Aladyshev
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Konstantin Aladyshev @ 2023-10-16 13:18 UTC (permalink / raw)
To: devel
Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
Konstantin Aladyshev
Although MtcpSubmit function receives source and destination MCTP EID
arguments these value are not used in any way currently. Instead the
code always uses EID values from the PCDs.
To correct this issue modify function interface to receive source and
destination MCTP EIDs via pointers and use PCD values only if the
pointers are NULL.
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
Signed-off-by: Abner Chang <abner.chang@amd.com>
---
.../Include/Protocol/MctpProtocol.h | 12 ++++--
.../Dxe/ManageabilityTransportMctp.c | 4 +-
.../MctpProtocol/Common/MctpProtocolCommon.c | 4 +-
.../Universal/MctpProtocol/Dxe/MctpProtocol.c | 42 ++++++++++++++-----
4 files changed, 44 insertions(+), 18 deletions(-)
diff --git a/Features/ManageabilityPkg/Include/Protocol/MctpProtocol.h b/Features/ManageabilityPkg/Include/Protocol/MctpProtocol.h
index 85e42f157d..c96b986c44 100644
--- a/Features/ManageabilityPkg/Include/Protocol/MctpProtocol.h
+++ b/Features/ManageabilityPkg/Include/Protocol/MctpProtocol.h
@@ -28,8 +28,12 @@ typedef struct _EDKII_MCTP_PROTOCOL EDKII_MCTP_PROTOCOL;
@param[in] This EDKII_MCTP_PROTOCOL instance.
@param[in] MctpType MCTP message type.
- @param[in] MctpSourceEndpointId MCTP source endpoint ID.
- @param[in] MctpDestinationEndpointId MCTP source endpoint ID.
+ @param[in] MctpSourceEndpointId Pointer of MCTP source endpoint ID.
+ Set to NULL means use platform PCD value
+ (PcdMctpSourceEndpointId).
+ @param[in] MctpDestinationEndpointId Pointer of MCTP destination endpoint ID.
+ Set to NULL means use platform PCD value
+ (PcdMctpDestinationEndpointId).
@param[in] RequestDataIntegrityCheck Indicates whether MCTP message has
integrity check byte.
@param[in] RequestData Message Data.
@@ -58,8 +62,8 @@ EFI_STATUS
(EFIAPI *MCTP_SUBMIT_COMMAND)(
IN EDKII_MCTP_PROTOCOL *This,
IN UINT8 MctpType,
- IN UINT8 MctpSourceEndpointId,
- IN UINT8 MctpDestinationEndpointId,
+ IN UINT8 *MctpSourceEndpointId,
+ IN UINT8 *MctpDestinationEndpointId,
IN BOOLEAN RequestDataIntegrityCheck,
IN UINT8 *RequestData,
IN UINT32 RequestDataSize,
diff --git a/Features/ManageabilityPkg/Library/ManageabilityTransportMctpLib/Dxe/ManageabilityTransportMctp.c b/Features/ManageabilityPkg/Library/ManageabilityTransportMctpLib/Dxe/ManageabilityTransportMctp.c
index c520e2302d..249104c873 100644
--- a/Features/ManageabilityPkg/Library/ManageabilityTransportMctpLib/Dxe/ManageabilityTransportMctp.c
+++ b/Features/ManageabilityPkg/Library/ManageabilityTransportMctpLib/Dxe/ManageabilityTransportMctp.c
@@ -205,8 +205,8 @@ MctpTransportTransmitReceive (
Status = mMctpProtocol->Functions.Version1_0->MctpSubmitCommand (
mMctpProtocol,
TransmitHeader->MessageHeader.MessageType,
- TransmitHeader->SourceEndpointId,
- TransmitHeader->DestinationEndpointId,
+ &TransmitHeader->SourceEndpointId,
+ &TransmitHeader->DestinationEndpointId,
(BOOLEAN)TransmitHeader->MessageHeader.IntegrityCheck,
TransferToken->TransmitPackage.TransmitPayload,
TransferToken->TransmitPackage.TransmitSizeInByte,
diff --git a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
index 5844d54eb2..3128aadd15 100644
--- a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
@@ -175,8 +175,8 @@ SetupMctpRequestTransportPacket (
MctpTransportHeader = (MCTP_TRANSPORT_HEADER *)ThisPackage;
MctpTransportHeader->Bits.Reserved = 0;
MctpTransportHeader->Bits.HeaderVersion = MCTP_KCS_HEADER_VERSION;
- MctpTransportHeader->Bits.DestinationEndpointId = PcdGet8 (PcdMctpDestinationEndpointId);
- MctpTransportHeader->Bits.SourceEndpointIdId = PcdGet8 (PcdMctpSourceEndpointId);
+ MctpTransportHeader->Bits.DestinationEndpointId = MctpDestinationEndpointId;
+ MctpTransportHeader->Bits.SourceEndpointId = MctpSourceEndpointId;
MctpTransportHeader->Bits.MessageTag = MCTP_MESSAGE_TAG;
MctpTransportHeader->Bits.TagOwner = MCTP_MESSAGE_TAG_OWNER_REQUEST;
MctpTransportHeader->Bits.PacketSequence = mMctpPacketSequence & MCTP_PACKET_SEQUENCE_MASK;
diff --git a/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c b/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c
index d0f49a1abb..73445bf816 100644
--- a/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c
+++ b/Features/ManageabilityPkg/Universal/MctpProtocol/Dxe/MctpProtocol.c
@@ -29,8 +29,12 @@ UINT32 mTransportMaximumPayload;
@param[in] This EDKII_MCTP_PROTOCOL instance.
@param[in] MctpType MCTP message type.
- @param[in] MctpSourceEndpointId MCTP source endpoint ID.
- @param[in] MctpDestinationEndpointId MCTP source endpoint ID.
+ @param[in] MctpSourceEndpointId Pointer of MCTP source endpoint ID.
+ Set to NULL means use platform PCD value
+ (PcdMctpSourceEndpointId).
+ @param[in] MctpDestinationEndpointId Pointer of MCTP destination endpoint ID.
+ Set to NULL means use platform PCD value
+ (PcdMctpDestinationEndpointId).
@param[in] RequestDataIntegrityCheck Indicates whether MCTP message has
integrity check byte.
@param[in] RequestData Message Data.
@@ -59,8 +63,8 @@ EFIAPI
MctpSubmitMessage (
IN EDKII_MCTP_PROTOCOL *This,
IN UINT8 MctpType,
- IN UINT8 MctpSourceEndpointId,
- IN UINT8 MctpDestinationEndpointId,
+ IN UINT8 *MctpSourceEndpointId,
+ IN UINT8 *MctpDestinationEndpointId,
IN BOOLEAN RequestDataIntegrityCheck,
IN UINT8 *RequestData,
IN UINT32 RequestDataSize,
@@ -72,24 +76,42 @@ MctpSubmitMessage (
)
{
EFI_STATUS Status;
+ UINT8 SourceEid;
+ UINT8 DestinationEid;
if ((RequestData == NULL) && (ResponseData == NULL)) {
DEBUG ((DEBUG_ERROR, "%a: Both RequestData and ResponseData are NULL\n", __func__));
return EFI_INVALID_PARAMETER;
}
+ if (MctpSourceEndpointId == NULL) {
+ SourceEid = PcdGet8 (PcdMctpSourceEndpointId);
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: Use PcdMctpSourceEndpointId for MCTP source EID: %x\n", __func__, SourceEid));
+ } else {
+ SourceEid = *MctpSourceEndpointId;
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: MCTP source EID: %x\n", __func__, SourceEid));
+ }
+
+ if (MctpDestinationEndpointId == NULL) {
+ DestinationEid = PcdGet8 (PcdMctpDestinationEndpointId);
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: Use PcdMctpDestinationEndpointId for MCTP destination EID: %x\n", __func__, DestinationEid));
+ } else {
+ DestinationEid = *MctpDestinationEndpointId;
+ DEBUG ((DEBUG_MANAGEABILITY, "%a: MCTP destination EID: %x\n", __func__, DestinationEid));
+ }
+
//
// Check source EID and destination EID
//
- if ((MctpSourceEndpointId >= MCTP_RESERVED_ENDPOINT_START_ID) &&
- (MctpSourceEndpointId <= MCTP_RESERVED_ENDPOINT_END_ID)
+ if ((SourceEid >= MCTP_RESERVED_ENDPOINT_START_ID) &&
+ (SourceEid <= MCTP_RESERVED_ENDPOINT_END_ID)
) {
DEBUG ((DEBUG_ERROR, "%a: The value of MCTP source EID (%x) is reserved.\n", __func__, MctpSourceEndpointId));
return EFI_INVALID_PARAMETER;
}
- if ((MctpDestinationEndpointId >= MCTP_RESERVED_ENDPOINT_START_ID) &&
- (MctpDestinationEndpointId <= MCTP_RESERVED_ENDPOINT_END_ID)
+ if ((DestinationEid >= MCTP_RESERVED_ENDPOINT_START_ID) &&
+ (DestinationEid <= MCTP_RESERVED_ENDPOINT_END_ID)
) {
DEBUG ((DEBUG_ERROR, "%a: The value of MCTP destination EID (%x) is reserved.\n", __func__, MctpDestinationEndpointId));
return EFI_INVALID_PARAMETER;
@@ -98,8 +120,8 @@ MctpSubmitMessage (
Status = CommonMctpSubmitMessage (
mTransportToken,
MctpType,
- MctpSourceEndpointId,
- MctpDestinationEndpointId,
+ SourceEid,
+ DestinationEid,
RequestDataIntegrityCheck,
RequestData,
RequestDataSize,
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109639): https://edk2.groups.io/g/devel/message/109639
Mute This Topic: https://groups.io/mt/101994944/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH edk2-platforms 07/10] ManageabilityPkg: Correct value for the MCTP TAG_OWNER response bit
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
` (5 preceding siblings ...)
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 06/10] ManageabilityPkg: Update the algorithm of using MCTP endpoint ID PCD Konstantin Aladyshev
@ 2023-10-16 13:18 ` Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 08/10] ManageabilityPkg: Don't check MCTP header fields if transfer has failed Konstantin Aladyshev
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Konstantin Aladyshev @ 2023-10-16 13:18 UTC (permalink / raw)
To: devel
Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
Konstantin Aladyshev
Currently the MCTP TAG_OWNER bit is checked against 1 both in MTCP
request and response.
According to the MTCP Base specification in case of the MCTP response
the TAG_OWNER bit should be equal to 0.
Correct MCTP_MESSAGE_TAG_OWNER_RESPONSE flag value to fix the issue.
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
.../Include/Library/ManageabilityTransportMctpLib.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportMctpLib.h b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportMctpLib.h
index 462e7436e6..a8dc8a8519 100644
--- a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportMctpLib.h
+++ b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportMctpLib.h
@@ -51,8 +51,8 @@ typedef struct {
// is not defined by the specification.
#define MCTP_MESSAGE_TAG 0x1
-#define MCTP_MESSAGE_TAG_OWNER_REQUEST 0x01
-#define MCTP_MESSAGE_TAG_OWNER_RESPONSE 0x01
+#define MCTP_MESSAGE_TAG_OWNER_REQUEST 1
+#define MCTP_MESSAGE_TAG_OWNER_RESPONSE 0
#define MCTP_PACKET_SEQUENCE_MASK 0x3
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109640): https://edk2.groups.io/g/devel/message/109640
Mute This Topic: https://groups.io/mt/101994945/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH edk2-platforms 08/10] ManageabilityPkg: Don't check MCTP header fields if transfer has failed
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
` (6 preceding siblings ...)
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 07/10] ManageabilityPkg: Correct value for the MCTP TAG_OWNER response bit Konstantin Aladyshev
@ 2023-10-16 13:18 ` Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 09/10] ManageabilityPkg: Use correct constants for PLDM header checks Konstantin Aladyshev
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Konstantin Aladyshev @ 2023-10-16 13:18 UTC (permalink / raw)
To: devel
Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
Konstantin Aladyshev
If MCTP KCS communication has failed we need to abort MCTP transfer
function before checking any MCTP header data.
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
.../MctpProtocol/Common/MctpProtocolCommon.c | 16 +++++++---------
1 file changed, 7 insertions(+), 9 deletions(-)
diff --git a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
index 3128aadd15..4aae4fcba9 100644
--- a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
@@ -461,6 +461,13 @@ CommonMctpSubmitMessage (
&TransferToken
);
+ *AdditionalTransferError = TransferToken.TransportAdditionalStatus;
+ Status = TransferToken.TransferStatus;
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: Failed to send MCTP command over %s: %r\n", __func__, mTransportName, Status));
+ return Status;
+ }
+
MctpTransportResponseHeader = (MCTP_TRANSPORT_HEADER *)ResponseBuffer;
if (MctpTransportResponseHeader->Bits.HeaderVersion != MCTP_KCS_HEADER_VERSION) {
DEBUG ((
@@ -543,18 +550,9 @@ CommonMctpSubmitMessage (
return EFI_DEVICE_ERROR;
}
- //
- // Return transfer status.
- //
- *AdditionalTransferError = TransferToken.TransportAdditionalStatus;
*ResponseDataSize = TransferToken.ReceivePackage.ReceiveSizeInByte - sizeof (MCTP_TRANSPORT_HEADER) - sizeof (MCTP_MESSAGE_HEADER);
CopyMem (ResponseData, ResponseBuffer + sizeof (MCTP_TRANSPORT_HEADER) + sizeof (MCTP_MESSAGE_HEADER), *ResponseDataSize);
FreePool (ResponseBuffer);
- Status = TransferToken.TransferStatus;
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "%a: Failed to send MCTP command over %s: %r\n", __func__, mTransportName, Status));
- return Status;
- }
return Status;
}
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109641): https://edk2.groups.io/g/devel/message/109641
Mute This Topic: https://groups.io/mt/101994946/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH edk2-platforms 09/10] ManageabilityPkg: Use correct constants for PLDM header checks
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
` (7 preceding siblings ...)
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 08/10] ManageabilityPkg: Don't check MCTP header fields if transfer has failed Konstantin Aladyshev
@ 2023-10-16 13:18 ` Konstantin Aladyshev
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 10/10] ManageabilityPkg: Return error on multiple-packet MCTP responses Konstantin Aladyshev
2023-10-18 5:52 ` [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Chang, Abner via groups.io
10 siblings, 0 replies; 12+ messages in thread
From: Konstantin Aladyshev @ 2023-10-16 13:18 UTC (permalink / raw)
To: devel
Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
Konstantin Aladyshev
Currently PldmProtocol code uses magic numbers in the PLDM header
checks. Since PLDM headers have all the necessary definitions replace
magic numbers with the appropriate defines.
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
.../Universal/PldmProtocol/Common/PldmProtocolCommon.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
index 1c4506d87f..3deded541a 100644
--- a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
@@ -321,15 +321,15 @@ CommonPldmSubmitCommand (
//
// Check the integrity of response. data.
ResponseHeader = (PLDM_RESPONSE_HEADER *)FullPacketResponseData;
- if ((ResponseHeader->PldmHeader.DatagramBit != 0) ||
- (ResponseHeader->PldmHeader.RequestBit != 0) ||
+ if ((ResponseHeader->PldmHeader.DatagramBit != PLDM_MESSAGE_HEADER_IS_DATAGRAM) ||
+ (ResponseHeader->PldmHeader.RequestBit != PLDM_MESSAGE_HEADER_IS_RESPONSE) ||
(ResponseHeader->PldmHeader.InstanceId != mPldmRequestInstanceId) ||
(ResponseHeader->PldmHeader.PldmType != PldmType) ||
(ResponseHeader->PldmHeader.PldmTypeCommandCode != PldmCommand))
{
DEBUG ((DEBUG_ERROR, "PLDM integrity check of response data is failed.\n"));
- DEBUG ((DEBUG_ERROR, " Request bit = %d (Expected value: 0)\n"));
- DEBUG ((DEBUG_ERROR, " Datagram = %d (Expected value: 0)\n"));
+ DEBUG ((DEBUG_ERROR, " Request bit = %d (Expected value: %d)\n", PLDM_MESSAGE_HEADER_IS_RESPONSE));
+ DEBUG ((DEBUG_ERROR, " Datagram = %d (Expected value: %d)\n", PLDM_MESSAGE_HEADER_IS_DATAGRAM));
DEBUG ((DEBUG_ERROR, " Instance ID = %d (Expected value: %d)\n", ResponseHeader->PldmHeader.InstanceId, mPldmRequestInstanceId));
DEBUG ((DEBUG_ERROR, " Pldm Type = %d (Expected value: %d)\n", ResponseHeader->PldmHeader.PldmType, PldmType));
DEBUG ((DEBUG_ERROR, " Pldm Command = %d (Expected value: %d)\n", ResponseHeader->PldmHeader.PldmTypeCommandCode, PldmCommand));
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109642): https://edk2.groups.io/g/devel/message/109642
Mute This Topic: https://groups.io/mt/101994947/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH edk2-platforms 10/10] ManageabilityPkg: Return error on multiple-packet MCTP responses
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
` (8 preceding siblings ...)
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 09/10] ManageabilityPkg: Use correct constants for PLDM header checks Konstantin Aladyshev
@ 2023-10-16 13:18 ` Konstantin Aladyshev
2023-10-18 5:52 ` [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Chang, Abner via groups.io
10 siblings, 0 replies; 12+ messages in thread
From: Konstantin Aladyshev @ 2023-10-16 13:18 UTC (permalink / raw)
To: devel
Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
Konstantin Aladyshev
Since the current driver doesn't yet support handling of
multiple-packet MCTP responses, return EFI_UNSUPPORTED error in such
cases.
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
.../MctpProtocol/Common/MctpProtocolCommon.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
index 4aae4fcba9..7cf2d916f4 100644
--- a/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/MctpProtocol/Common/MctpProtocolCommon.c
@@ -524,6 +524,17 @@ CommonMctpSubmitMessage (
FreePool (ResponseBuffer);
return EFI_DEVICE_ERROR;
}
+ if ((MctpTransportResponseHeader->Bits.StartOfMessage != 1) ||
+ (MctpTransportResponseHeader->Bits.EndOfMessage != 1) ||
+ (MctpTransportResponseHeader->Bits.PacketSequence != 0)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Error! Multiple-packet MCTP responses are not supported by the current driver\n",
+ __func__,
+ ));
+ FreePool (ResponseBuffer);
+ return EFI_UNSUPPORTED;
+ }
MctpMessageResponseHeader = (MCTP_MESSAGE_HEADER *)(MctpTransportResponseHeader + 1);
if (MctpMessageResponseHeader->Bits.MessageType != MctpType) {
--
2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109643): https://edk2.groups.io/g/devel/message/109643
Mute This Topic: https://groups.io/mt/101994949/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support
2023-10-16 13:18 [edk2-devel] [PATCH edk2-platforms 00/10] MTCP-over-KCS support Konstantin Aladyshev
` (9 preceding siblings ...)
2023-10-16 13:18 ` [edk2-devel] [PATCH edk2-platforms 10/10] ManageabilityPkg: Return error on multiple-packet MCTP responses Konstantin Aladyshev
@ 2023-10-18 5:52 ` Chang, Abner via groups.io
10 siblings, 0 replies; 12+ messages in thread
From: Chang, Abner via groups.io @ 2023-10-18 5:52 UTC (permalink / raw)
To: Konstantin Aladyshev, devel@edk2.groups.io
Cc: isaac.w.oram@intel.com, Attar, AbdulLateef (Abdul Lateef),
nicklew@nvidia.com
[AMD Official Use Only - General]
For entire series, Reviewed-by: Abner Chang <abner.chang@amd.com>
Hi Aladyshev,
Could you please provide the corresponding branch somewhere that contains the latest version of this patch set? Then I can move forward to merge it after edk2 portion is merged.
Thanks for this contribution.
Abner
> -----Original Message-----
> From: Konstantin Aladyshev <aladyshev22@gmail.com>
> Sent: Monday, October 16, 2023 9:18 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; isaac.w.oram@intel.com; Attar,
> AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>;
> nicklew@nvidia.com; Konstantin Aladyshev <aladyshev22@gmail.com>
> Subject: [PATCH edk2-platforms 00/10] MTCP-over-KCS support
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> The Manageability KCS transport library needs to support requests both
> from MCTP and IPMI transports. Currently the code only handles IPMI
> case correctly.
> In the MCTP case the communication should be based on the MCTP-over-KCS
> specification (DSP0254). This specification defines a special KCS
> binding header and trailer structures that need to be present in every
> MCTP message.
> The header structure contains a length field, therefore response packet
> size is not needed to be known beforehand.
> The trailer structure contains a PEC checksum that can be used to check
> itegrity of the response message.
> Modify Manageability KCS transport library code to check which message
> is processed (IPMI or MCTP) and handle each case correctly based on its
> own specification.
> This patch is a result of a joint effort from the Konstantin Aladyshev
> <aladyshev22@gmail.com> and Abner Chang <abner.chang@amd.com>.
>
> Tested:
> PLDM communication between the HOST and BMC was tested with both
> components implemented via open-source software:
> - The HOST (UEFI firmware) part was based one the edk2 [1] and
> edk2-platforms [2] code,
> - The BMC part was based on the openbmc [3] distribution.
>
> The testing process and all the necessary utilities are described in
> the [4] repository.
>
> The provided changes keep IPMI over KCS stack working as reported by
> Abner Chang.
>
> [1]: https://github.com/tianocore/edk2
> [2]: https://github.com/tianocore/edk2-platforms
> [3]: https://github.com/openbmc/openbmc
> [4]: https://github.com/Kostr/PLDM
>
> Konstantin Aladyshev (10):
> ManageabilityPkg: Add definition for the MCTP KCS TRAILER structure
> ManageabilityPkg: Check MCTP EIDs for reserved values
> ManageabilityPkg: Support both MCTP and IPMI in KCS tranport library
> ManageabilityPkg: Check header fields in the MCTP response
> ManageabilityPkg: Correct typo in MCTP destination EID field
> ManageabilityPkg: Update the algorithm of using MCTP endpoint ID PCD
> ManageabilityPkg: Correct value for the MCTP TAG_OWNER response bit
> ManageabilityPkg: Don't check MCTP header fields if transfer has
> failed
> ManageabilityPkg: Use correct constants for PLDM header checks
> ManageabilityPkg: Return error on multiple-packet MCTP responses
>
> .../Library/ManageabilityTransportMctpLib.h | 9 +-
> .../Include/Protocol/MctpProtocol.h | 12 +-
> .../Common/KcsCommon.c | 284 +++++++++++++++---
> .../Dxe/ManageabilityTransportMctp.c | 4 +-
> .../MctpProtocol/Common/MctpProtocolCommon.c | 129 +++++++-
> .../Universal/MctpProtocol/Dxe/MctpProtocol.c | 51 +++-
> .../PldmProtocol/Common/PldmProtocolCommon.c | 10 +-
> 7 files changed, 430 insertions(+), 69 deletions(-)
>
> --
> 2.34.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109710): https://edk2.groups.io/g/devel/message/109710
Mute This Topic: https://groups.io/mt/101994937/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 12+ messages in thread