[edk2-devel] [PATCH edk2-platforms v2 13/15] ManageabilityPkg/PldmProtocol: Remove PLDM command table

["Konstantin Aladyshev" <aladyshev22@gmail.com>]

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]
-=-=-=-=-=-=-=-=-=-=-=-