public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Konstantin Aladyshev" <aladyshev22@gmail.com>
To: devel@edk2.groups.io
Cc: abner.chang@amd.com, AbdulLateef.Attar@amd.com,
	nicklew@nvidia.com, Konstantin Aladyshev <aladyshev22@gmail.com>
Subject: [edk2-devel] [PATCH edk2-platforms v3 13/16] ManageabilityPkg/PldmProtocol: Remove PLDM command table
Date: Mon, 23 Oct 2023 16:05:08 +0300	[thread overview]
Message-ID: <20231023130511.4521-14-aladyshev22@gmail.com> (raw)
In-Reply-To: <20231023130511.4521-1-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 (#109921): https://edk2.groups.io/g/devel/message/109921
Mute This Topic: https://groups.io/mt/102134663/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



  parent reply	other threads:[~2023-10-23 13:05 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-23 13:04 [edk2-devel] [PATCH edk2-platforms v3 00/16] MTCP-over-KCS support Konstantin Aladyshev
2023-10-23 13:04 ` [edk2-devel] [PATCH edk2-platforms v3 01/16] ManageabilityPkg: Add definition for the MCTP KCS TRAILER structure Konstantin Aladyshev
2023-10-25  8:55   ` Attar, AbdulLateef (Abdul Lateef) via groups.io
2023-10-23 13:04 ` [edk2-devel] [PATCH edk2-platforms v3 02/16] ManageabilityPkg: Check MCTP EIDs for reserved values Konstantin Aladyshev
2023-10-25  8:57   ` Attar, AbdulLateef (Abdul Lateef) via groups.io
2023-10-23 13:04 ` [edk2-devel] [PATCH edk2-platforms v3 03/16] ManageabilityPkg: Support both MCTP and IPMI in KCS tranport library Konstantin Aladyshev
2023-10-23 13:04 ` [edk2-devel] [PATCH edk2-platforms v3 04/16] ManageabilityPkg: Check header fields in the MCTP response Konstantin Aladyshev
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 05/16] ManageabilityPkg: Correct typo in MCTP destination EID field Konstantin Aladyshev
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 06/16] ManageabilityPkg: Update the algorithm of using MCTP endpoint ID PCD Konstantin Aladyshev
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 07/16] ManageabilityPkg: Correct value for the MCTP TAG_OWNER response bit Konstantin Aladyshev
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 08/16] ManageabilityPkg: Don't check MCTP header fields if transfer has failed Konstantin Aladyshev
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 09/16] ManageabilityPkg: Use correct constants for PLDM header checks Konstantin Aladyshev
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 10/16] ManageabilityPkg: Return error on multiple-packet MCTP responses Konstantin Aladyshev
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 11/16] ManageabilityPkg: Add PLDM terminus PCDs Konstantin Aladyshev
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 12/16] PldmProtocolDxe: Correct TID argument usage Konstantin Aladyshev
2023-10-23 13:05 ` Konstantin Aladyshev [this message]
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 14/16] ManageabilityPkg: Return error on PLDM header check fails Konstantin Aladyshev
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 15/16] PldmSmbiosTransferDxe: Implement Set PLDM terminus ID API Konstantin Aladyshev
2023-10-23 13:05 ` [edk2-devel] [PATCH edk2-platforms v3 16/16] ManageabilityPkg: Check PLDM completion code Konstantin Aladyshev
2023-10-24  2:44 ` [edk2-devel] [PATCH edk2-platforms v3 00/16] MTCP-over-KCS support Chang, Abner via groups.io
2023-10-26  5:03   ` Chang, Abner via groups.io
2023-10-31  6:37     ` Yoshinoya
2023-10-31  7:00       ` Chang, Abner via groups.io
2023-11-01  5:19         ` Yoshinoya
2023-11-01  5:53           ` Chang, Abner via groups.io

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231023130511.4521-14-aladyshev22@gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox