public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support
@ 2023-10-20 12:52 Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 01/15] ManageabilityPkg: Add definition for the MCTP KCS TRAILER structure Konstantin Aladyshev
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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.
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

Changes v1 -> v2:
 - Add new patches with corrections for the PLDM protocol. The
  resulting communication via EDKII_PLDM_PROTOCOL was successfully
  tested.
  

Abner Chang (4):
  ManageabilityPkg: Add PLDM terminus PCDs
  PldmProtocolDxe: Correct TID argument usage
  ManageabilityPkg/PldmProtocol: Remove PLDM command table
  PldmSmbiosTransferDxe: Implement Set PLDM terminus ID API

Konstantin Aladyshev (11):
  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
  ManageabilityPkg: Return error on PLDM header check fails

 .../Include/Library/BasePldmProtocolLib.h     |  16 +
 .../Library/ManageabilityTransportMctpLib.h   |   9 +-
 .../Include/Protocol/MctpProtocol.h           |  12 +-
 .../Include/Protocol/PldmProtocol.h           |  18 +-
 .../Protocol/PldmSmbiosTransferProtocol.h     |  26 ++
 .../Common/KcsCommon.c                        | 284 +++++++++++++++---
 .../Dxe/ManageabilityTransportMctp.c          |   4 +-
 .../PldmProtocolLibrary/Dxe/PldmProtocolLib.c |  49 ++-
 .../Dxe/PldmProtocolLib.inf                   |   6 +-
 .../ManageabilityPkg/ManageabilityPkg.dec     |   6 +
 .../MctpProtocol/Common/MctpProtocolCommon.c  | 129 +++++++-
 .../Universal/MctpProtocol/Dxe/MctpProtocol.c |  51 +++-
 .../PldmProtocol/Common/PldmProtocolCommon.c  | 145 +++------
 .../PldmProtocol/Common/PldmProtocolCommon.h  |  25 +-
 .../Universal/PldmProtocol/Dxe/PldmProtocol.c |  69 ++++-
 .../PldmProtocol/Dxe/PldmProtocolDxe.inf      |   4 -
 .../PldmSmbiosTransferDxe.c                   |  28 ++
 17 files changed, 688 insertions(+), 193 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109850): https://edk2.groups.io/g/devel/message/109850
Mute This Topic: https://groups.io/mt/102080227/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 01/15] ManageabilityPkg: Add definition for the MCTP KCS TRAILER structure
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 02/15] ManageabilityPkg: Check MCTP EIDs for reserved values Konstantin Aladyshev
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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 (#109851): https://edk2.groups.io/g/devel/message/109851
Mute This Topic: https://groups.io/mt/102080228/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 02/15] ManageabilityPkg: Check MCTP EIDs for reserved values
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 01/15] ManageabilityPkg: Add definition for the MCTP KCS TRAILER structure Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 03/15] ManageabilityPkg: Support both MCTP and IPMI in KCS tranport library Konstantin Aladyshev
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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 (#109852): https://edk2.groups.io/g/devel/message/109852
Mute This Topic: https://groups.io/mt/102080229/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 03/15] ManageabilityPkg: Support both MCTP and IPMI in KCS tranport library
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 01/15] ManageabilityPkg: Add definition for the MCTP KCS TRAILER structure Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 02/15] ManageabilityPkg: Check MCTP EIDs for reserved values Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 04/15] ManageabilityPkg: Check header fields in the MCTP response Konstantin Aladyshev
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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 (#109853): https://edk2.groups.io/g/devel/message/109853
Mute This Topic: https://groups.io/mt/102080230/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 04/15] ManageabilityPkg: Check header fields in the MCTP response
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (2 preceding siblings ...)
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 03/15] ManageabilityPkg: Support both MCTP and IPMI in KCS tranport library Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 05/15] ManageabilityPkg: Correct typo in MCTP destination EID field Konstantin Aladyshev
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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 (#109854): https://edk2.groups.io/g/devel/message/109854
Mute This Topic: https://groups.io/mt/102080231/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 05/15] ManageabilityPkg: Correct typo in MCTP destination EID field
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (3 preceding siblings ...)
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 04/15] ManageabilityPkg: Check header fields in the MCTP response Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 06/15] ManageabilityPkg: Update the algorithm of using MCTP endpoint ID PCD Konstantin Aladyshev
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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 (#109855): https://edk2.groups.io/g/devel/message/109855
Mute This Topic: https://groups.io/mt/102080232/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 06/15] ManageabilityPkg: Update the algorithm of using MCTP endpoint ID PCD
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (4 preceding siblings ...)
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 05/15] ManageabilityPkg: Correct typo in MCTP destination EID field Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 07/15] ManageabilityPkg: Correct value for the MCTP TAG_OWNER response bit Konstantin Aladyshev
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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 (#109856): https://edk2.groups.io/g/devel/message/109856
Mute This Topic: https://groups.io/mt/102080233/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 07/15] ManageabilityPkg: Correct value for the MCTP TAG_OWNER response bit
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (5 preceding siblings ...)
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 06/15] ManageabilityPkg: Update the algorithm of using MCTP endpoint ID PCD Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 08/15] ManageabilityPkg: Don't check MCTP header fields if transfer has failed Konstantin Aladyshev
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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 (#109857): https://edk2.groups.io/g/devel/message/109857
Mute This Topic: https://groups.io/mt/102080235/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 08/15] ManageabilityPkg: Don't check MCTP header fields if transfer has failed
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (6 preceding siblings ...)
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 07/15] ManageabilityPkg: Correct value for the MCTP TAG_OWNER response bit Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 09/15] ManageabilityPkg: Use correct constants for PLDM header checks Konstantin Aladyshev
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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 (#109858): https://edk2.groups.io/g/devel/message/109858
Mute This Topic: https://groups.io/mt/102080236/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 09/15] ManageabilityPkg: Use correct constants for PLDM header checks
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (7 preceding siblings ...)
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 08/15] ManageabilityPkg: Don't check MCTP header fields if transfer has failed Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 10/15] ManageabilityPkg: Return error on multiple-packet MCTP responses Konstantin Aladyshev
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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..4edfe05955 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, "    Datagram     = %d (Expected value: %d)\n", ResponseHeader->PldmHeader.DatagramBit, (!PLDM_MESSAGE_HEADER_IS_DATAGRAM)));
+    DEBUG ((DEBUG_ERROR, "    Request bit  = %d (Expected value: %d)\n", ResponseHeader->PldmHeader.RequestBit, PLDM_MESSAGE_HEADER_IS_RESPONSE));
     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 (#109859): https://edk2.groups.io/g/devel/message/109859
Mute This Topic: https://groups.io/mt/102080238/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 10/15] ManageabilityPkg: Return error on multiple-packet MCTP responses
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (8 preceding siblings ...)
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 09/15] ManageabilityPkg: Use correct constants for PLDM header checks Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 11/15] ManageabilityPkg: Add PLDM terminus PCDs Konstantin Aladyshev
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 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..3709ab16eb 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 (#109860): https://edk2.groups.io/g/devel/message/109860
Mute This Topic: https://groups.io/mt/102080239/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 11/15] ManageabilityPkg: Add PLDM terminus PCDs
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (9 preceding siblings ...)
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 10/15] ManageabilityPkg: Return error on multiple-packet MCTP responses Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 12/15] PldmProtocolDxe: Correct TID argument usage Konstantin Aladyshev
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 UTC (permalink / raw)
  To: devel
  Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
	Konstantin Aladyshev

From: Abner Chang <abner.chang@amd.com>

Add PLDM source and destination terminus IDs for transmiting PLDM
message.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
 Features/ManageabilityPkg/ManageabilityPkg.dec | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec b/Features/ManageabilityPkg/ManageabilityPkg.dec
index 14fe0fd2e0..eb0ee67cba 100644
--- a/Features/ManageabilityPkg/ManageabilityPkg.dec
+++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
@@ -72,6 +72,12 @@
   # @Prompt MCTP KCS (Memory mapped) I/O base address
   gManageabilityPkgTokenSpaceGuid.PcdMctpKcsBaseAddress|0xca2|UINT32|0x00000004
 
+  ## This value is the PLDM source and destination terminus ID for transmiting PLDM message.
+  # @Prompt PLDM source terminus ID
+  gManageabilityPkgTokenSpaceGuid.PcdPldmSourceTerminusId|0|UINT8|0x00000040
+  # @Prompt PLDM destination terminus ID
+  gManageabilityPkgTokenSpaceGuid.PcdPldmDestinationEndpointId|0|UINT8|0x00000041
+
   ## This is the value of SOL channels supported on platform.
   # @Prompt SOL channel number
   gManageabilityPkgTokenSpaceGuid.PcdMaxSolChannels|3|UINT8|0x00000100
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109861): https://edk2.groups.io/g/devel/message/109861
Mute This Topic: https://groups.io/mt/102080241/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 12/15] PldmProtocolDxe: Correct TID argument usage
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (10 preceding siblings ...)
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 11/15] ManageabilityPkg: Add PLDM terminus PCDs Konstantin Aladyshev
@ 2023-10-20 12:52 ` Konstantin Aladyshev
  2023-10-20 12:53 ` [edk2-devel] [PATCH edk2-platforms v2 13/15] ManageabilityPkg/PldmProtocol: Remove PLDM command table Konstantin Aladyshev
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:52 UTC (permalink / raw)
  To: devel
  Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
	Konstantin Aladyshev

From: Abner Chang <abner.chang@amd.com>

Currently the PLDM source/destination TID arguments for the PldmSubmit
function are not actually used in any way in the underlying MCTP
communication. The code just uses MCTP source/destination EID PCDs. So
we have to restructure code to actually use provided PLDM TIDs.
On the other case the PldmSubmitCommand function from the
PldmProtocolLib doesn't even accept the source/destination TID
arguments.
To address both these facts correct TID argument usage in the following
way:
- by default the TID values are taken from the built-time PCDs,
- user have an ability to provide custom TIDs either via PldmSubmit
function arguments or by calling PldmSetTerminus API.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
 .../Include/Library/BasePldmProtocolLib.h     | 16 ++++++
 .../Include/Protocol/PldmProtocol.h           | 18 +++---
 .../PldmProtocolLibrary/Dxe/PldmProtocolLib.c | 49 +++++++++++++++-
 .../Dxe/PldmProtocolLib.inf                   |  6 +-
 .../PldmProtocol/Common/PldmProtocolCommon.c  | 28 +++++++---
 .../PldmProtocol/Common/PldmProtocolCommon.h  | 22 +++++---
 .../Universal/PldmProtocol/Dxe/PldmProtocol.c | 56 ++++++++++++++++---
 .../PldmProtocol/Dxe/PldmProtocolDxe.inf      |  4 --
 8 files changed, 162 insertions(+), 37 deletions(-)

diff --git a/Features/ManageabilityPkg/Include/Library/BasePldmProtocolLib.h b/Features/ManageabilityPkg/Include/Library/BasePldmProtocolLib.h
index 5523ac3a4d..a698197263 100644
--- a/Features/ManageabilityPkg/Include/Library/BasePldmProtocolLib.h
+++ b/Features/ManageabilityPkg/Include/Library/BasePldmProtocolLib.h
@@ -9,6 +9,22 @@
 #ifndef EDKII_PLDM_PROTOCOL_LIB_H_
 #define EDKII_PLDM_PROTOCOL_LIB_H_
 
+/**
+  This function sets the PLDM source termius and destination terminus
+  ID for SMBIOS PLDM transfer.
+
+  @param[in]         SourceId       PLDM source teminus ID.
+  @param[in]         DestinationId  PLDM destination teminus ID.
+
+  @retval EFI_SUCCESS            The terminus is set successfully.
+  @retval EFI_INVALID_PARAMETER  The terminus is set unsuccessfully.
+**/
+EFI_STATUS
+PldmSetTerminus (
+  IN  UINT8   SourceId,
+  IN  UINT8   DestinationId
+);
+
 /**
   This service enables submitting commands via EDKII PLDM protocol.
 
diff --git a/Features/ManageabilityPkg/Include/Protocol/PldmProtocol.h b/Features/ManageabilityPkg/Include/Protocol/PldmProtocol.h
index 651997e1ad..02efb3015a 100644
--- a/Features/ManageabilityPkg/Include/Protocol/PldmProtocol.h
+++ b/Features/ManageabilityPkg/Include/Protocol/PldmProtocol.h
@@ -26,13 +26,15 @@ typedef struct  _EDKII_PLDM_PROTOCOL EDKII_PLDM_PROTOCOL;
 /**
   This service enables submitting commands via EDKII PLDM protocol.
 
-  @param[in]         This              EDKII_PLDM_PROTOCOL instance.
-  @param[in]         PldmType          PLDM message type.
-  @param[in]         Command           PLDM Command of PLDM message type.
-  @param[in]         RequestData       Command Request Data.
-  @param[in]         RequestDataSize   Size of Command Request Data.
-  @param[out]        ResponseData      Command Response Data. The completion code is the first byte of response data.
-  @param[in, out]    ResponseDataSize  Size of Command Response Data.
+  @param[in]         This                       EDKII_PLDM_PROTOCOL instance.
+  @param[in]         PldmType                   PLDM message type.
+  @param[in]         Command                    PLDM Command of PLDM message type.
+  @param[in]         PldmTerminusSourceId       PLDM source teminus ID.
+  @param[in]         PldmTerminusDestinationId  PLDM destination teminus ID.
+  @param[in]         RequestData                Command Request Data.
+  @param[in]         RequestDataSize            Size of Command Request Data.
+  @param[out]        ResponseData               Command Response Data. The completion code is the first byte of response data.
+  @param[in, out]    ResponseDataSize           Size of Command Response Data.
 
   @retval EFI_SUCCESS            The command byte stream was successfully submit to the device and a response was successfully received.
   @retval EFI_NOT_FOUND          The command was not successfully sent to the device or a response was not successfully received from the device.
@@ -49,6 +51,8 @@ EFI_STATUS
   IN     EDKII_PLDM_PROTOCOL  *This,
   IN     UINT8                PldmType,
   IN     UINT8                Command,
+  IN     UINT8                PldmTerminusSourceId,
+  IN     UINT8                PldmTerminusDestinationId,
   IN     UINT8                *RequestData,
   IN     UINT32               RequestDataSize,
   OUT    UINT8                *ResponseData,
diff --git a/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.c b/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.c
index 267bd8fbc1..37231b0756 100644
--- a/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.c
+++ b/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.c
@@ -9,10 +9,34 @@
 #include <PiDxe.h>
 #include <Protocol/PldmProtocol.h>
 #include <Library/DebugLib.h>
+#include <Library/PcdLib.h>
 #include <Library/ManageabilityTransportHelperLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 
-EDKII_PLDM_PROTOCOL  *mEdkiiPldmProtocol = NULL;
+EDKII_PLDM_PROTOCOL  *mEdkiiPldmProtocol        = NULL;
+UINT8                mSourcePldmTerminusId      = 0;
+UINT8                mDestinationPldmTerminusId = 0;
+
+/**
+  This function sets the PLDM source termius and destination terminus
+  ID for SMBIOS PLDM transfer.
+
+  @param[in]         SourceId       PLDM source teminus ID.
+  @param[in]         DestinationId  PLDM destination teminus ID.
+
+  @retval EFI_SUCCESS            The terminus is set successfully.
+  @retval EFI_INVALID_PARAMETER  The terminus is set unsuccessfully.
+**/
+EFI_STATUS
+PldmSetTerminus (
+  IN  UINT8   SourceId,
+  IN  UINT8   DestinationId
+)
+{
+  mSourcePldmTerminusId      = SourceId;
+  mDestinationPldmTerminusId = DestinationId;
+  return EFI_SUCCESS;
+}
 
 /**
   This service enables submitting commands via EDKII PLDM protocol.
@@ -69,6 +93,8 @@ PldmSubmitCommand (
                                                        mEdkiiPldmProtocol,
                                                        PldmType,
                                                        Command,
+                                                       mSourcePldmTerminusId,
+                                                       mDestinationPldmTerminusId,
                                                        RequestData,
                                                        RequestDataSize,
                                                        ResponseData,
@@ -85,3 +111,24 @@ PldmSubmitCommand (
 
   return Status;
 }
+/**
+
+  Initialize mSourcePldmTerminusId and mDestinationPldmTerminusId.
+
+  @param ImageHandle     The image handle.
+  @param SystemTable     The system table.
+
+  @retval  EFI_SUCCESS  Protocol listener is registered successfully.
+
+**/
+EFI_STATUS
+EFIAPI
+PldmProtocolLibConstructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+
+  PldmSetTerminus (PcdGet8(PcdPldmSourceTerminusId), PcdGet8(PcdPldmDestinationEndpointId));
+  return EFI_SUCCESS;
+}
diff --git a/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.inf b/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.inf
index 1233d76726..19c84840b6 100644
--- a/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.inf
+++ b/Features/ManageabilityPkg/Library/PldmProtocolLibrary/Dxe/PldmProtocolLib.inf
@@ -16,7 +16,7 @@
   MODULE_TYPE                    = DXE_DRIVER
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = PldmProtocolLib|DXE_RUNTIME_DRIVER DXE_DRIVER DXE_CORE UEFI_DRIVER UEFI_APPLICATION
-
+  CONSTRUCTOR                    = PldmProtocolLibConstructor
 #
 #  VALID_ARCHITECTURES          = IA32 X64
 #
@@ -40,3 +40,7 @@
 [Protocols]
   gEdkiiPldmProtocolGuid          ## ALWAYS_CONSUMES
 
+[FixedPcd]
+  gManageabilityPkgTokenSpaceGuid.PcdPldmSourceTerminusId
+  gManageabilityPkgTokenSpaceGuid.PcdPldmDestinationEndpointId
+
diff --git a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
index 4edfe05955..ea3d4a22b2 100644
--- a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
@@ -64,6 +64,8 @@ GetFullPacketResponseSize (
   @param[in]         TransportToken     The transport interface.
   @param[in]         PldmType           PLDM message type.
   @param[in]         PldmCommand        PLDM command of this PLDM type.
+  @param[in]         SourceId           PLDM source teminus ID.
+  @param[in]         DestinationId      PLDM destination teminus ID.
   @param[out]        PacketHeader       The pointer to receive header of request.
   @param[out]        PacketHeaderSize   Packet header size in bytes.
   @param[in, out]    PacketBody         The request body.
@@ -88,6 +90,8 @@ SetupPldmRequestTransportPacket (
   IN   MANAGEABILITY_TRANSPORT_TOKEN    *TransportToken,
   IN   UINT8                            PldmType,
   IN   UINT8                            PldmCommand,
+  IN   UINT8                            SourceId,
+  IN   UINT8                            DestinationId,
   OUT  MANAGEABILITY_TRANSPORT_HEADER   *PacketHeader,
   OUT  UINT16                           *PacketHeaderSize,
   IN OUT UINT8                          **PacketBody,
@@ -118,8 +122,8 @@ SetupPldmRequestTransportPacket (
       return EFI_OUT_OF_RESOURCES;
     }
 
-    MctpHeader->SourceEndpointId             = PcdGet8 (PcdMctpSourceEndpointId);
-    MctpHeader->DestinationEndpointId        = PcdGet8 (PcdMctpDestinationEndpointId);
+    MctpHeader->SourceEndpointId             = SourceId;
+    MctpHeader->DestinationEndpointId        = DestinationId;
     MctpHeader->MessageHeader.IntegrityCheck = FALSE;
     MctpHeader->MessageHeader.MessageType    = MCTP_MESSAGE_TYPE_PLDM;
     *PacketHeader                            = (MANAGEABILITY_TRANSPORT_HEADER *)MctpHeader;
@@ -161,13 +165,15 @@ SetupPldmRequestTransportPacket (
 /**
   Common code to submit PLDM commands
 
-  @param[in]         TransportToken    Transport token.
-  @param[in]         PldmType          PLDM message type.
-  @param[in]         PldmCommand       PLDM command of this PLDM type.
-  @param[in]         RequestData       Command Request Data.
-  @param[in]         RequestDataSize   Size of Command Request Data.
-  @param[out]        ResponseData      Command Response Data. The completion code is the first byte of response data.
-  @param[in, out]    ResponseDataSize  Size of Command Response Data.
+  @param[in]         TransportToken             Transport token.
+  @param[in]         PldmType                   PLDM message type.
+  @param[in]         PldmCommand                PLDM command of this PLDM type.
+  @param[in]         PldmTerminusSourceId       PLDM source teminus ID.
+  @param[in]         PldmTerminusDestinationId  PLDM destination teminus ID.
+  @param[in]         RequestData                Command Request Data.
+  @param[in]         RequestDataSize            Size of Command Request Data.
+  @param[out]        ResponseData               Command Response Data. The completion code is the first byte of response data.
+  @param[in, out]    ResponseDataSize           Size of Command Response Data.
 
   @retval EFI_SUCCESS            The command byte stream was successfully submit to the device and a response was successfully received.
   @retval EFI_NOT_FOUND          The command was not successfully sent to the device or a response was not successfully received from the device.
@@ -182,6 +188,8 @@ CommonPldmSubmitCommand (
   IN     MANAGEABILITY_TRANSPORT_TOKEN  *TransportToken,
   IN     UINT8                          PldmType,
   IN     UINT8                          PldmCommand,
+  IN     UINT8                          PldmTerminusSourceId,
+  IN     UINT8                          PldmTerminusDestinationId,
   IN     UINT8                          *RequestData OPTIONAL,
   IN     UINT32                         RequestDataSize,
   OUT    UINT8                          *ResponseData OPTIONAL,
@@ -225,6 +233,8 @@ CommonPldmSubmitCommand (
                            TransportToken,
                            PldmType,
                            PldmCommand,
+                           PldmTerminusSourceId,
+                           PldmTerminusDestinationId,
                            &PldmTransportHeader,
                            &HeaderSize,
                            &ThisRequestData,
diff --git a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.h b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.h
index 231d6e802e..79431dd3b1 100644
--- a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.h
+++ b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.h
@@ -44,6 +44,8 @@ SetupPldmTransportHardwareInformation (
   @param[in]         TransportToken     The transport interface.
   @param[in]         PldmType           PLDM message type.
   @param[in]         PldmCommand        PLDM command of this PLDM type.
+  @param[in]         SourceId           PLDM source teminus ID.
+  @param[in]         DestinationId      PLDM destination teminus ID.
   @param[out]        PacketHeader       The pointer to receive header of request.
   @param[out]        PacketHeaderSize   Packet header size in bytes.
   @param[in, out]    PacketBody         The request body.
@@ -68,6 +70,8 @@ SetupPldmRequestTransportPacket (
   IN   MANAGEABILITY_TRANSPORT_TOKEN    *TransportToken,
   IN   UINT8                            PldmType,
   IN   UINT8                            PldmCommand,
+  IN   UINT8                            SourceId,
+  IN   UINT8                            DestinationId,
   OUT  MANAGEABILITY_TRANSPORT_HEADER   *PacketHeader,
   OUT  UINT16                           *PacketHeaderSize,
   IN OUT UINT8                          **PacketBody,
@@ -79,13 +83,15 @@ SetupPldmRequestTransportPacket (
 /**
   Common code to submit PLDM commands
 
-  @param[in]         TransportToken    Transport token.
-  @param[in]         PldmType          PLDM message type.
-  @param[in]         PldmCommand       PLDM command of this PLDM type.
-  @param[in]         RequestData       Command Request Data.
-  @param[in]         RequestDataSize   Size of Command Request Data.
-  @param[out]        ResponseData      Command Response Data. The completion code is the first byte of response data.
-  @param[in, out]    ResponseDataSize  Size of Command Response Data.
+  @param[in]         TransportToken             Transport token.
+  @param[in]         PldmType                   PLDM message type.
+  @param[in]         PldmCommand                PLDM command of this PLDM type.
+  @param[in]         PldmTerminusSourceId       PLDM source teminus ID.
+  @param[in]         PldmTerminusDestinationId  PLDM destination teminus ID.
+  @param[in]         RequestData                Command Request Data.
+  @param[in]         RequestDataSize            Size of Command Request Data.
+  @param[out]        ResponseData               Command Response Data. The completion code is the first byte of response data.
+  @param[in, out]    ResponseDataSize           Size of Command Response Data.
 
   @retval EFI_SUCCESS            The command byte stream was successfully submit to the device and a response was successfully received.
   @retval EFI_NOT_FOUND          The command was not successfully sent to the device or a response was not successfully received from the device.
@@ -100,6 +106,8 @@ CommonPldmSubmitCommand (
   IN     MANAGEABILITY_TRANSPORT_TOKEN  *TransportToken,
   IN     UINT8                          PldmType,
   IN     UINT8                          PldmCommand,
+  IN     UINT8                          PldmTerminusSourceId,
+  IN     UINT8                          PldmTerminusDestinationId,
   IN     UINT8                          *RequestData OPTIONAL,
   IN     UINT32                         RequestDataSize,
   OUT    UINT8                          *ResponseData OPTIONAL,
diff --git a/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocol.c b/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocol.c
index b2ca69b05f..726747416c 100644
--- a/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocol.c
+++ b/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocol.c
@@ -25,13 +25,15 @@ UINT32                         TransportMaximumPayload;
 /**
   This service enables submitting commands via EDKII PLDM protocol.
 
-  @param[in]         This              EDKII_PLDM_PROTOCOL instance.
-  @param[in]         PldmType          PLDM message type.
-  @param[in]         Command           PLDM Command of PLDM message type.
-  @param[in]         RequestData       Command Request Data.
-  @param[in]         RequestDataSize   Size of Command Request Data.
-  @param[out]        ResponseData      Command Response Data. The completion code is the first byte of response data.
-  @param[in, out]    ResponseDataSize  Size of Command Response Data.
+  @param[in]         This                       EDKII_PLDM_PROTOCOL instance.
+  @param[in]         PldmType                   PLDM message type.
+  @param[in]         Command                    PLDM Command of PLDM message type.
+  @param[in]         PldmTerminusSourceId       PLDM source teminus ID.
+  @param[in]         PldmTerminusDestinationId  PLDM destination teminus ID.
+  @param[in]         RequestData                Command Request Data.
+  @param[in]         RequestDataSize            Size of Command Request Data.
+  @param[out]        ResponseData               Command Response Data. The completion code is the first byte of response data.
+  @param[in, out]    ResponseDataSize           Size of Command Response Data.
 
   @retval EFI_SUCCESS            The command byte stream was successfully submit to the device and a response was successfully received.
   @retval EFI_NOT_FOUND          The command was not successfully sent to the device or a response was not successfully received from the device.
@@ -39,7 +41,7 @@ UINT32                         TransportMaximumPayload;
   @retval EFI_DEVICE_ERROR       PLDM transport interface Device hardware error.
   @retval EFI_TIMEOUT            The command time out.
   @retval EFI_UNSUPPORTED        The command was not successfully sent to the device.
-  @retval EFI_OUT_OF_RESOURCES   The resource allocation is out of resource or data size error.
+  @retval EFI_OUT_OF_RESOURCES   The resource allcation is out of resource or data size error.
   @retval EFI_INVALID_PARAMETER  Both RequestData and ResponseData are NULL
 **/
 EFI_STATUS
@@ -48,6 +50,8 @@ PldmSubmitCommand (
   IN     EDKII_PLDM_PROTOCOL  *This,
   IN     UINT8                PldmType,
   IN     UINT8                Command,
+  IN     UINT8                PldmTerminusSourceId,
+  IN     UINT8                PldmTerminusDestinationId,
   IN     UINT8                *RequestData,
   IN     UINT32               RequestDataSize,
   OUT    UINT8                *ResponseData,
@@ -61,10 +65,46 @@ PldmSubmitCommand (
     return EFI_INVALID_PARAMETER;
   }
 
+  if (RequestData != NULL && RequestDataSize == 0) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: RequestDataSize == 0, however RequestData is not NULL for PLDM type: 0x%x, Command: 0x%x.\n",
+      __func__,
+      PldmType,
+      Command
+      ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (ResponseData == NULL && *ResponseDataSize != 0) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: *ResponseDataSize != 0, however ResponseData is NULL for PLDM type: 0x%x, Command: 0x%x.\n",
+      __func__,
+      PldmType,
+      Command
+      ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (ResponseData != NULL && *ResponseDataSize == 0) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: *ResponseDataSize == 0, however ResponseData is not NULL for PLDM type: 0x%x, Command: 0x%x.\n",
+      __func__,
+      PldmType,
+      Command
+      ));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  DEBUG ((DEBUG_MANAGEABILITY, "%a: Source terminus ID: 0x%x, Destination terminus ID: 0x%x.\n"));
   Status = CommonPldmSubmitCommand (
              mTransportToken,
              PldmType,
              Command,
+             PldmTerminusSourceId,
+             PldmTerminusDestinationId,
              RequestData,
              RequestDataSize,
              ResponseData,
diff --git a/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocolDxe.inf b/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocolDxe.inf
index 006f77b09a..aef25f6438 100644
--- a/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocolDxe.inf
+++ b/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocolDxe.inf
@@ -42,9 +42,5 @@
 [Protocols]
   gEdkiiPldmProtocolGuid
 
-[FixedPcd]
-  gManageabilityPkgTokenSpaceGuid.PcdMctpSourceEndpointId
-  gManageabilityPkgTokenSpaceGuid.PcdMctpDestinationEndpointId
-
 [Depex]
   TRUE
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109862): https://edk2.groups.io/g/devel/message/109862
Mute This Topic: https://groups.io/mt/102080242/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 13/15] ManageabilityPkg/PldmProtocol: Remove PLDM command table
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (11 preceding siblings ...)
  2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 12/15] PldmProtocolDxe: Correct TID argument usage Konstantin Aladyshev
@ 2023-10-20 12:53 ` Konstantin Aladyshev
  2023-10-20 12:53 ` [edk2-devel] [PATCH edk2-platforms v2 14/15] ManageabilityPkg: Return error on PLDM header check fails Konstantin Aladyshev
  2023-10-20 12:53 ` [edk2-devel] [PATCH edk2-platforms v2 15/15] PldmSmbiosTransferDxe: Implement Set PLDM terminus ID API Konstantin Aladyshev
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:53 UTC (permalink / raw)
  To: devel
  Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
	Konstantin Aladyshev

From: Abner Chang <abner.chang@amd.com>

In case of the PLDM/MCTP communication response size doesn't have to be
known beforehand, the caller just need to provide the buffer big enough
to accomodate the response.
Remove PLDM command table for retrieving the response payload size and
correct the code to fix the response buffer size handling.
Also update the message for error conditions.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
 .../PldmProtocol/Common/PldmProtocolCommon.c  | 100 +++---------------
 .../PldmProtocol/Common/PldmProtocolCommon.h  |   3 +
 .../Universal/PldmProtocol/Dxe/PldmProtocol.c |  13 ++-
 3 files changed, 31 insertions(+), 85 deletions(-)

diff --git a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
index ea3d4a22b2..bc72ce07b3 100644
--- a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
@@ -21,42 +21,6 @@
 extern CHAR16  *mTransportName;
 extern UINT8   mPldmRequestInstanceId;
 
-PLDM_MESSAGE_PACKET_MAPPING  PldmMessagePacketMappingTable[] = {
-  { PLDM_TYPE_SMBIOS, PLDM_GET_SMBIOS_STRUCTURE_TABLE_METADATA_COMMAND_CODE, sizeof (PLDM_GET_SMBIOS_STRUCTURE_TABLE_METADATA_RESPONSE_FORMAT) },
-  { PLDM_TYPE_SMBIOS, PLDM_SET_SMBIOS_STRUCTURE_TABLE_METADATA_COMMAND_CODE, sizeof (PLDM_SET_SMBIOS_STRUCTURE_TABLE_METADATA_RESPONSE_FORMAT) },
-  { PLDM_TYPE_SMBIOS, PLDM_SET_SMBIOS_STRUCTURE_TABLE_COMMAND_CODE,          sizeof (PLDM_SET_SMBIOS_STRUCTURE_TABLE_REQUEST_FORMAT)           }
-};
-
-/**
-  This function returns the expected full size of PLDM response message.
-
-  @param[in]         PldmType        PLDM message type.
-  @param[in]         PldmCommand     PLDM command of this PLDM type.
-
-  @retval  Zero       No matched entry for this PldmType/PldmCommand.
-  @retval  None-zero  Size of full packet is returned.
-**/
-UINT32
-GetFullPacketResponseSize (
-  IN UINT8  PldmType,
-  IN UINT8  PldmCommand
-  )
-{
-  INT16                        Index;
-  PLDM_MESSAGE_PACKET_MAPPING  *ThisEntry;
-
-  ThisEntry = PldmMessagePacketMappingTable;
-  for (Index = 0; Index < (sizeof (PldmMessagePacketMappingTable)/ sizeof (PLDM_MESSAGE_PACKET_MAPPING)); Index++) {
-    if ((PldmType == ThisEntry->PldmType) && (PldmCommand == ThisEntry->PldmCommand)) {
-      return ThisEntry->ResponseSize;
-    }
-
-    ThisEntry++;
-  }
-
-  return 0;
-}
-
 /**
   This functions setup the final header/body/trailer packets for
   the acquired transport interface.
@@ -267,10 +231,10 @@ CommonPldmSubmitCommand (
   TransferToken.TransmitPackage.TransmitTimeoutInMillisecond = MANAGEABILITY_TRANSPORT_NO_TIMEOUT;
 
   // Set receive packet.
-  FullPacketResponseDataSize = GetFullPacketResponseSize (PldmType, PldmCommand);
-  if (FullPacketResponseDataSize == 0) {
-    DEBUG ((DEBUG_ERROR, "  No mapping entry in PldmMessagePacketMappingTable for PLDM Type:%d Command %d\n", PldmType, PldmCommand));
-    ASSERT (FALSE);
+  if (ResponseData == NULL && *ResponseDataSize == 0) {
+    FullPacketResponseDataSize = sizeof (PLDM_RESPONSE_HEADER);
+  } else {
+    FullPacketResponseDataSize = *ResponseDataSize + sizeof (PLDM_RESPONSE_HEADER);
   }
 
   FullPacketResponseData = (UINT8 *)AllocateZeroPool (FullPacketResponseDataSize);
@@ -306,6 +270,7 @@ CommonPldmSubmitCommand (
                                                     );
   //
   // Check the response size.
+  //
   if (TransferToken.ReceivePackage.ReceiveSizeInByte < sizeof (PLDM_RESPONSE_HEADER)) {
     DEBUG ((
       DEBUG_MANAGEABILITY_INFO,
@@ -315,21 +280,13 @@ CommonPldmSubmitCommand (
       TransferToken.ReceivePackage.ReceiveSizeInByte,
       FullPacketResponseDataSize
       ));
-    if (ResponseDataSize != NULL) {
-      if (*ResponseDataSize > TransferToken.ReceivePackage.ReceiveSizeInByte) {
-        *ResponseDataSize = TransferToken.ReceivePackage.ReceiveSizeInByte;
-      }
-    }
-
-    if (ResponseData != NULL) {
-      CopyMem ((VOID *)ResponseData, (VOID *)FullPacketResponseData, *ResponseDataSize);
-    }
-
+    HelperManageabilityDebugPrint ((VOID *)FullPacketResponseData, TransferToken.ReceivePackage.ReceiveSizeInByte, "Failed response payload\n");
     goto ErrorExit;
   }
 
   //
   // Check the integrity of response. data.
+  //
   ResponseHeader = (PLDM_RESPONSE_HEADER *)FullPacketResponseData;
   if ((ResponseHeader->PldmHeader.DatagramBit != (!PLDM_MESSAGE_HEADER_IS_DATAGRAM)) ||
       (ResponseHeader->PldmHeader.RequestBit != PLDM_MESSAGE_HEADER_IS_RESPONSE) ||
@@ -343,22 +300,16 @@ CommonPldmSubmitCommand (
     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));
-    if (ResponseDataSize != NULL) {
-      if (*ResponseDataSize > TransferToken.ReceivePackage.ReceiveSizeInByte) {
-        *ResponseDataSize = TransferToken.ReceivePackage.ReceiveSizeInByte;
-      }
-    }
-
-    if (ResponseData != NULL) {
-      CopyMem ((VOID *)ResponseData, (VOID *)FullPacketResponseData, *ResponseDataSize);
-    }
+    DEBUG ((DEBUG_ERROR, "    Pldm Completion Code = 0x%x\n", ResponseHeader->PldmCompletionCode));
 
+    HelperManageabilityDebugPrint ((VOID *)FullPacketResponseData, TransferToken.ReceivePackage.ReceiveSizeInByte, "Failed response payload\n");
     goto ErrorExit;
   }
 
   //
   // Check the response size
-  if (TransferToken.ReceivePackage.ReceiveSizeInByte != FullPacketResponseDataSize) {
+  //
+  if (TransferToken.ReceivePackage.ReceiveSizeInByte > FullPacketResponseDataSize) {
     DEBUG ((
       DEBUG_ERROR,
       "The response size is incorrect: Response size %d (Expected %d), Completion code %d.\n",
@@ -366,38 +317,21 @@ CommonPldmSubmitCommand (
       FullPacketResponseDataSize,
       ResponseHeader->PldmCompletionCode
       ));
-    if (ResponseDataSize != NULL) {
-      if (*ResponseDataSize > TransferToken.ReceivePackage.ReceiveSizeInByte) {
-        *ResponseDataSize = TransferToken.ReceivePackage.ReceiveSizeInByte;
-      }
-    }
-
-    if (ResponseData != NULL) {
-      CopyMem ((VOID *)ResponseData, (VOID *)FullPacketResponseData, *ResponseDataSize);
-    }
 
+    HelperManageabilityDebugPrint ((VOID *)FullPacketResponseData, TransferToken.ReceivePackage.ReceiveSizeInByte, "Failed response payload\n");
     goto ErrorExit;
   }
 
-  if (*ResponseDataSize != (TransferToken.ReceivePackage.ReceiveSizeInByte - sizeof (PLDM_RESPONSE_HEADER))) {
+  if (*ResponseDataSize < GET_PLDM_MESSAGE_PAYLOAD_SIZE(TransferToken.ReceivePackage.ReceiveSizeInByte)) {
     DEBUG ((DEBUG_ERROR, "  The size of response is not matched to RequestDataSize assigned by caller.\n"));
     DEBUG ((
       DEBUG_ERROR,
       "Caller expects %d, the response size minus PLDM_RESPONSE_HEADER size is %d, Completion Code %d.\n",
       *ResponseDataSize,
-      TransferToken.ReceivePackage.ReceiveSizeInByte - sizeof (PLDM_RESPONSE_HEADER),
+      GET_PLDM_MESSAGE_PAYLOAD_SIZE(TransferToken.ReceivePackage.ReceiveSizeInByte),
       ResponseHeader->PldmCompletionCode
       ));
-    if (ResponseDataSize != NULL) {
-      if (*ResponseDataSize > TransferToken.ReceivePackage.ReceiveSizeInByte) {
-        *ResponseDataSize = TransferToken.ReceivePackage.ReceiveSizeInByte;
-      }
-    }
-
-    if (ResponseData != NULL) {
-      CopyMem ((VOID *)ResponseData, (VOID *)FullPacketResponseData, *ResponseDataSize);
-    }
-
+    HelperManageabilityDebugPrint ((VOID *)FullPacketResponseData, GET_PLDM_MESSAGE_PAYLOAD_SIZE(TransferToken.ReceivePackage.ReceiveSizeInByte), "Failed response payload\n");
     goto ErrorExit;
   }
 
@@ -406,10 +340,10 @@ CommonPldmSubmitCommand (
 
   // Copy response data (without header) to caller's buffer.
   if ((ResponseData != NULL) && (*ResponseDataSize != 0)) {
-    *ResponseDataSize = FullPacketResponseDataSize - sizeof (PLDM_RESPONSE_HEADER);
+    *ResponseDataSize = GET_PLDM_MESSAGE_PAYLOAD_SIZE(TransferToken.ReceivePackage.ReceiveSizeInByte);
     CopyMem (
       (VOID *)ResponseData,
-      (VOID *)(FullPacketResponseData + sizeof (PLDM_RESPONSE_HEADER)),
+      GET_PLDM_MESSAGE_PAYLOAD_PTR(FullPacketResponseData),
       *ResponseDataSize
       );
   }
diff --git a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.h b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.h
index 79431dd3b1..eb273c4f46 100644
--- a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.h
+++ b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.h
@@ -12,6 +12,9 @@
 #include <IndustryStandard/Pldm.h>
 #include <Library/ManageabilityTransportLib.h>
 
+#define GET_PLDM_MESSAGE_PAYLOAD_SIZE(PayloadSize) (PayloadSize - sizeof (PLDM_RESPONSE_HEADER))
+#define GET_PLDM_MESSAGE_PAYLOAD_PTR(PayloadPtr) ((UINT8 *)PayloadPtr + sizeof (PLDM_RESPONSE_HEADER))
+
 typedef struct {
   UINT8     PldmType;
   UINT8     PldmCommand;
diff --git a/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocol.c b/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocol.c
index 726747416c..058f98e677 100644
--- a/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocol.c
+++ b/Features/ManageabilityPkg/Universal/PldmProtocol/Dxe/PldmProtocol.c
@@ -60,8 +60,17 @@ PldmSubmitCommand (
 {
   EFI_STATUS  Status;
 
-  if ((RequestData == NULL) && (ResponseData == NULL)) {
-    DEBUG ((DEBUG_ERROR, "%a: Both RequestData and ResponseData are NULL\n", __func__));
+  //
+  // Check the given input parameters.
+  //
+  if (RequestData == NULL && RequestDataSize != 0) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: RequestDataSize != 0, however RequestData is NULL for PLDM type: 0x%x, Command: 0x%x.\n",
+      __func__,
+      PldmType,
+      Command
+      ));
     return EFI_INVALID_PARAMETER;
   }
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109863): https://edk2.groups.io/g/devel/message/109863
Mute This Topic: https://groups.io/mt/102080243/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 14/15] ManageabilityPkg: Return error on PLDM header check fails
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (12 preceding siblings ...)
  2023-10-20 12:53 ` [edk2-devel] [PATCH edk2-platforms v2 13/15] ManageabilityPkg/PldmProtocol: Remove PLDM command table Konstantin Aladyshev
@ 2023-10-20 12:53 ` Konstantin Aladyshev
  2023-10-20 12:53 ` [edk2-devel] [PATCH edk2-platforms v2 15/15] PldmSmbiosTransferDxe: Implement Set PLDM terminus ID API Konstantin Aladyshev
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:53 UTC (permalink / raw)
  To: devel
  Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
	Konstantin Aladyshev

Currently PldmSubmit command returns EFI_SUCCESS even if the response
header checks have failed.
Correct the code to return errors in such cases.

Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
 .../Universal/PldmProtocol/Common/PldmProtocolCommon.c   | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
index bc72ce07b3..04f250e57c 100644
--- a/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
+++ b/Features/ManageabilityPkg/Universal/PldmProtocol/Common/PldmProtocolCommon.c
@@ -241,7 +241,7 @@ CommonPldmSubmitCommand (
   if (FullPacketResponseData == NULL) {
     DEBUG ((DEBUG_ERROR, "  Not enough memory for FullPacketResponseDataSize.\n"));
     Status = EFI_OUT_OF_RESOURCES;
-    goto ErrorExit2;
+    goto ErrorExit;
   }
 
   // Print out PLDM packet.
@@ -281,6 +281,7 @@ CommonPldmSubmitCommand (
       FullPacketResponseDataSize
       ));
     HelperManageabilityDebugPrint ((VOID *)FullPacketResponseData, TransferToken.ReceivePackage.ReceiveSizeInByte, "Failed response payload\n");
+    Status = EFI_DEVICE_ERROR;
     goto ErrorExit;
   }
 
@@ -303,6 +304,7 @@ CommonPldmSubmitCommand (
     DEBUG ((DEBUG_ERROR, "    Pldm Completion Code = 0x%x\n", ResponseHeader->PldmCompletionCode));
 
     HelperManageabilityDebugPrint ((VOID *)FullPacketResponseData, TransferToken.ReceivePackage.ReceiveSizeInByte, "Failed response payload\n");
+    Status = EFI_DEVICE_ERROR;
     goto ErrorExit;
   }
 
@@ -319,6 +321,7 @@ CommonPldmSubmitCommand (
       ));
 
     HelperManageabilityDebugPrint ((VOID *)FullPacketResponseData, TransferToken.ReceivePackage.ReceiveSizeInByte, "Failed response payload\n");
+    Status = EFI_DEVICE_ERROR;
     goto ErrorExit;
   }
 
@@ -332,6 +335,7 @@ CommonPldmSubmitCommand (
       ResponseHeader->PldmCompletionCode
       ));
     HelperManageabilityDebugPrint ((VOID *)FullPacketResponseData, GET_PLDM_MESSAGE_PAYLOAD_SIZE(TransferToken.ReceivePackage.ReceiveSizeInByte), "Failed response payload\n");
+    Status = EFI_DEVICE_ERROR;
     goto ErrorExit;
   }
 
@@ -350,13 +354,12 @@ CommonPldmSubmitCommand (
 
   // Return transfer status.
   //
-ErrorExit:
   Status = TransferToken.TransferStatus;
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a: Failed to send PLDM command over %s\n", __func__, mTransportName));
   }
 
-ErrorExit2:
+ErrorExit:
   if (PldmTransportHeader != NULL) {
     FreePool ((VOID *)PldmTransportHeader);
   }
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109864): https://edk2.groups.io/g/devel/message/109864
Mute This Topic: https://groups.io/mt/102080244/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] 16+ messages in thread

* [edk2-devel] [PATCH edk2-platforms v2 15/15] PldmSmbiosTransferDxe: Implement Set PLDM terminus ID API
  2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
                   ` (13 preceding siblings ...)
  2023-10-20 12:53 ` [edk2-devel] [PATCH edk2-platforms v2 14/15] ManageabilityPkg: Return error on PLDM header check fails Konstantin Aladyshev
@ 2023-10-20 12:53 ` Konstantin Aladyshev
  14 siblings, 0 replies; 16+ messages in thread
From: Konstantin Aladyshev @ 2023-10-20 12:53 UTC (permalink / raw)
  To: devel
  Cc: abner.chang, isaac.w.oram, AbdulLateef.Attar, nicklew,
	Konstantin Aladyshev

From: Abner Chang <abner.chang@amd.com>

Currently all PLDM functions inside the PLDM_SMBIOS_TRANSFER_PROTOCOL
use PLDM terminus PCDs for the MCTP addressing.
Add additional function to the protocol API to provide user a way to
use custom TIDs.

Signed-off-by: Abner Chang <abner.chang@amd.com>
Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
---
 .../Protocol/PldmSmbiosTransferProtocol.h     | 26 +++++++++++++++++
 .../PldmSmbiosTransferDxe.c                   | 28 +++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/Features/ManageabilityPkg/Include/Protocol/PldmSmbiosTransferProtocol.h b/Features/ManageabilityPkg/Include/Protocol/PldmSmbiosTransferProtocol.h
index 7903e12726..8b23d39682 100644
--- a/Features/ManageabilityPkg/Include/Protocol/PldmSmbiosTransferProtocol.h
+++ b/Features/ManageabilityPkg/Include/Protocol/PldmSmbiosTransferProtocol.h
@@ -23,6 +23,31 @@ typedef struct  _EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL EDKII_PLDM_SMBIOS_TRANSFER_
 #define EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL_VERSION        ((EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL_VERSION_MAJOR << 8) |\
                                                        EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL_VERSION_MINOR)
 
+/**
+  This function sets PLDM SMBIOS transfer source and destination
+  PLDM terminus ID.
+
+  @param [in]   This           EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL instance.
+  @param [in]   SourceId       PLDM source teminus ID.
+                               Set to PLDM_TERMINUS_ID_UNASSIGNED means use
+                               platform default PLDM terminus ID.
+                               (gManageabilityPkgTokenSpaceGuid.PcdPldmSourceTerminusId)
+  @param [in]   DestinationId  PLDM destination teminus ID.
+                               Set to PLDM_TERMINUS_ID_UNASSIGNED means use
+                               platform default PLDM terminus ID.
+                               (gManageabilityPkgTokenSpaceGuid.PcdPldmDestinationEndpointId)
+
+  @retval       EFI_SUCCESS            Get SMBIOS table metadata Successfully.
+  @retval       EFI_INVALID_PARAMETER  Invalid value of source or destination
+                                       PLDM terminus ID.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *PLDM_GET_SMBIOS_TRANSFER_TERMINUS_ID)(
+  IN  UINT8  SourceId,
+  IN  UINT8  DestinationId
+  );
+
 /**
   This function gets SMBIOS table metadata.
 
@@ -151,6 +176,7 @@ EFI_STATUS
 // EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL
 //
 typedef struct {
+  PLDM_GET_SMBIOS_TRANSFER_TERMINUS_ID        SetPldmSmbiosTransferTerminusId;
   PLDM_GET_SMBIOS_STRUCTURE_TABLE_METADATA    GetSmbiosStructureTableMetaData;
   PLDM_SET_SMBIOS_STRUCTURE_TABLE_METADATA    SetSmbiosStructureTableMetaData;
   PLDM_GET_SMBIOS_STRUCTURE_TABLE             GetSmbiosStructureTable;
diff --git a/Features/ManageabilityPkg/Universal/PldmSmbiosTransferDxe/PldmSmbiosTransferDxe.c b/Features/ManageabilityPkg/Universal/PldmSmbiosTransferDxe/PldmSmbiosTransferDxe.c
index fdf033f0b1..357a7d49e4 100644
--- a/Features/ManageabilityPkg/Universal/PldmSmbiosTransferDxe/PldmSmbiosTransferDxe.c
+++ b/Features/ManageabilityPkg/Universal/PldmSmbiosTransferDxe/PldmSmbiosTransferDxe.c
@@ -25,6 +25,33 @@
 
 UINT32  SetSmbiosStructureTableHandle;
 
+/**
+  This function sets PLDM SMBIOS transfer source and destination
+  PLDM terminus ID.
+
+  @param [in]   This           EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL instance.
+  @param [in]   SourceId       PLDM source teminus ID.
+                               Set to PLDM_TERMINUS_ID_UNASSIGNED means use
+                               platform default PLDM terminus ID.
+                               (gManageabilityPkgTokenSpaceGuid.PcdPldmSourceTerminusId)
+  @param [in]   DestinationId  PLDM destination teminus ID.
+                               Set to PLDM_TERMINUS_ID_UNASSIGNED means use
+                               platform default PLDM terminus ID.
+                               (gManageabilityPkgTokenSpaceGuid.PcdPldmDestinationEndpointId)
+
+  @retval       EFI_SUCCESS            Get SMBIOS table metadata Successfully.
+  @retval       EFI_INVALID_PARAMETER  Invalid value of source or destination
+                                       PLDM terminus ID.
+**/
+EFI_STATUS
+SetPldmSmbiosTransferTerminusId (
+  IN  UINT8  SourceId,
+  IN  UINT8  DestinationId
+  )
+{
+  return PldmSetTerminus(SourceId, DestinationId);
+}
+
 /**
   Get the full size of SMBIOS structure including optional strings that follow the formatted structure.
 
@@ -457,6 +484,7 @@ GetSmbiosStructureByHandle (
 }
 
 EDKII_PLDM_SMBIOS_TRANSFER_PROTOCOL_V1_0  mPldmSmbiosTransferProtocolV10 = {
+  SetPldmSmbiosTransferTerminusId,
   GetSmbiosStructureTableMetaData,
   SetSmbiosStructureTableMetaData,
   GetSmbiosStructureTable,
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109865): https://edk2.groups.io/g/devel/message/109865
Mute This Topic: https://groups.io/mt/102080245/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] 16+ messages in thread

end of thread, other threads:[~2023-10-20 12:53 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-20 12:52 [edk2-devel] [PATCH edk2-platforms v2 00/15] MTCP-over-KCS support Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 01/15] ManageabilityPkg: Add definition for the MCTP KCS TRAILER structure Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 02/15] ManageabilityPkg: Check MCTP EIDs for reserved values Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 03/15] ManageabilityPkg: Support both MCTP and IPMI in KCS tranport library Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 04/15] ManageabilityPkg: Check header fields in the MCTP response Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 05/15] ManageabilityPkg: Correct typo in MCTP destination EID field Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 06/15] ManageabilityPkg: Update the algorithm of using MCTP endpoint ID PCD Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 07/15] ManageabilityPkg: Correct value for the MCTP TAG_OWNER response bit Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 08/15] ManageabilityPkg: Don't check MCTP header fields if transfer has failed Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 09/15] ManageabilityPkg: Use correct constants for PLDM header checks Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 10/15] ManageabilityPkg: Return error on multiple-packet MCTP responses Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 11/15] ManageabilityPkg: Add PLDM terminus PCDs Konstantin Aladyshev
2023-10-20 12:52 ` [edk2-devel] [PATCH edk2-platforms v2 12/15] PldmProtocolDxe: Correct TID argument usage Konstantin Aladyshev
2023-10-20 12:53 ` [edk2-devel] [PATCH edk2-platforms v2 13/15] ManageabilityPkg/PldmProtocol: Remove PLDM command table Konstantin Aladyshev
2023-10-20 12:53 ` [edk2-devel] [PATCH edk2-platforms v2 14/15] ManageabilityPkg: Return error on PLDM header check fails Konstantin Aladyshev
2023-10-20 12:53 ` [edk2-devel] [PATCH edk2-platforms v2 15/15] PldmSmbiosTransferDxe: Implement Set PLDM terminus ID API Konstantin Aladyshev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox