From: "Nickle Wang" <nicklew@nvidia.com>
To: "abner.chang@amd.com" <abner.chang@amd.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: Isaac Oram <isaac.w.oram@intel.com>,
Abdul Lateef Attar <abdattar@amd.com>,
Igor Kulchytskyy <igork@ami.com>
Subject: Re: [edk2-platforms][PATCH V2 06/14] ManageabilityPkg/KCS: KCS transport interface
Date: Thu, 20 Apr 2023 06:53:41 +0000 [thread overview]
Message-ID: <MW4PR12MB70314BBDE5FC7D775D5B23CBD9639@MW4PR12MB7031.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20230418071543.1951-7-abner.chang@amd.com>
Reviewed-by: Nickle Wang <nicklew@nvidia.com>
Regards,
Nickle
> -----Original Message-----
> From: abner.chang@amd.com <abner.chang@amd.com>
> Sent: Tuesday, April 18, 2023 3:16 PM
> To: devel@edk2.groups.io
> Cc: Isaac Oram <isaac.w.oram@intel.com>; Abdul Lateef Attar
> <abdattar@amd.com>; Nickle Wang <nicklew@nvidia.com>; Igor Kulchytskyy
> <igork@ami.com>
> Subject: [edk2-platforms][PATCH V2 06/14] ManageabilityPkg/KCS: KCS
> transport interface
>
> External email: Use caution opening links or attachments
>
>
> From: Abner Chang <abner.chang@amd.com>
>
> - Return Maximum Transfer Unit for MCTP over KCS
> - Check the parameters
>
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> ---
> .../Common/ManageabilityTransportKcs.h | 2 +-
> .../Common/KcsCommon.c | 112 ++++++++++--------
> .../Dxe/ManageabilityTransportKcs.c | 24 ++--
> 3 files changed, 78 insertions(+), 60 deletions(-)
>
> diff --git
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/
> ManageabilityTransportKcs.h
> b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/
> ManageabilityTransportKcs.h
> index d6685c165e..8c6a64416a 100644
> ---
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/
> ManageabilityTransportKcs.h
> +++ b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Com
> +++ mon/ManageabilityTransportKcs.h
> @@ -71,7 +71,7 @@ typedef struct {
> EFI_STATUS
> EFIAPI
> KcsTransportSendCommand (
> - IN MANAGEABILITY_TRANSPORT_HEADER TransmitHeader,
> + IN MANAGEABILITY_TRANSPORT_HEADER TransmitHeader OPTIONAL,
> IN UINT16 TransmitHeaderSize,
> IN MANAGEABILITY_TRANSPORT_TRAILER TransmitTrailer OPTIONAL,
> IN UINT16 TransmitTrailerSize,
> diff --git
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/
> KcsCommon.c
> b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/
> KcsCommon.c
> index 14a7047447..a8c6a674c9 100644
> ---
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/
> KcsCommon.c
> +++ b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Com
> +++ mon/KcsCommon.c
> @@ -99,14 +99,14 @@ ClearOBF (
> Algorithm is based on flow chart provided in IPMI spec 2.0
> Figure 9-6, KCS Interface BMC to SMS Write Transfer Flow Chart
>
> - @param[in] TransmitHeader KCS packet header.
> - @param[in] TransmitHeaderSize KCS packet header size in byte.
> - @param[in] TransmitTrailer KCS packet trailer.
> - @param[in] TransmitTrailerSize KCS packet trailer size in byte.
> - @param[in] RequestData Command Request Data, could be NULL.
> - RequestDataSize must be zero, if RequestData
> - is NULL.
> - @param[in] RequestDataSize Size of Command Request Data.
> + @param[in] TransmitHeader KCS packet header.
> + @param[in] TransmitHeaderSize KCS packet header size in byte.
> + @param[in] TransmitTrailer KCS packet trailer.
> + @param[in] TransmitTrailerSize KCS packet trailer size in byte.
> + @param[in] RequestData Command Request Data, could be NULL.
> + RequestDataSize must be zero, if RequestData
> + is NULL.
> + @param[in] RequestDataSize Size of Command Request Data.
>
> @retval EFI_SUCCESS The command byte stream was successfully
> submit to the device and a response was @@ -414,7 +414,7
> @@ KcsTransportRead ( EFI_STATUS EFIAPI KcsTransportSendCommand (
> - IN MANAGEABILITY_TRANSPORT_HEADER TransmitHeader,
> + IN MANAGEABILITY_TRANSPORT_HEADER TransmitHeader OPTIONAL,
> IN UINT16 TransmitHeaderSize,
> IN MANAGEABILITY_TRANSPORT_TRAILER TransmitTrailer OPTIONAL,
> IN UINT16 TransmitTrailerSize,
> @@ -427,6 +427,7 @@ KcsTransportSendCommand (
> EFI_STATUS Status;
> UINT32 RspHeaderSize;
> IPMI_KCS_RESPONSE_HEADER RspHeader;
> + UINT32 ExpectedResponseDataSize;
>
> if ((RequestData != NULL) && (RequestDataSize == 0)) {
> DEBUG ((DEBUG_ERROR, "%a: Mismatched values of RequestData and
> RequestDataSize\n", __FUNCTION__)); @@ -438,65 +439,72 @@
> KcsTransportSendCommand (
> return EFI_INVALID_PARAMETER;
> }
>
> - if (TransmitHeader == NULL) {
> - DEBUG ((DEBUG_ERROR, "%a: TransmitHeader is NULL\n", __FUNCTION__));
> - return EFI_INVALID_PARAMETER;
> + // Print out the request payloads.
> + if ((TransmitHeader != NULL) && (TransmitHeaderSize != 0)) {
> + HelperManageabilityDebugPrint ((VOID *)TransmitHeader,
> + (UINT32)TransmitHeaderSize, "KCS Transmit Header:\n");
> }
>
> - //
> - // Print out the request payloads.
> - HelperManageabilityDebugPrint ((VOID *)TransmitHeader, TransmitHeaderSize,
> "KCS Transmit Header:\n");
> if (RequestData != NULL) {
> HelperManageabilityDebugPrint ((VOID *)RequestData, RequestDataSize,
> "KCS Request Data:\n");
> }
>
> - if (TransmitTrailer != NULL) {
> - HelperManageabilityDebugPrint ((VOID *)TransmitTrailer, TransmitTrailerSize,
> "KCS Transmit Trailer:\n");
> - }
> + if ((TransmitTrailer != NULL) && (TransmitTrailerSize != 0)) {
> + HelperManageabilityDebugPrint ((VOID *)TransmitTrailer,
> + (UINT32)TransmitTrailerSize, "KCS Transmit Trailer:\n"); }
> +
> + if ((TransmitHeader != NULL) || (RequestData != NULL)) {
> + Status = KcsTransportWrite (
> + TransmitHeader,
> + TransmitHeaderSize,
> + TransmitTrailer,
> + TransmitTrailerSize,
> + RequestData,
> + RequestDataSize
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "KCS Write Failed with Status(%r)", Status));
> + return Status;
> + }
>
> - Status = KcsTransportWrite (
> - TransmitHeader,
> - TransmitHeaderSize,
> - TransmitTrailer,
> - TransmitTrailerSize,
> - RequestData,
> - RequestDataSize
> - );
> - if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "IPMI KCS Write Failed with Status(%r)", Status));
> - return Status;
> - }
> + //
> + // Read the response header
> + RspHeaderSize = sizeof (IPMI_KCS_RESPONSE_HEADER);
> + Status = KcsTransportRead ((UINT8 *)&RspHeader, &RspHeaderSize);
> + 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);
> + }
>
> - //
> - // Read the response header
> - RspHeaderSize = sizeof (IPMI_KCS_RESPONSE_HEADER);
> - Status = KcsTransportRead ((UINT8 *)&RspHeader, &RspHeaderSize);
> - if (EFI_ERROR (Status)) {
> - DEBUG ((
> - DEBUG_ERROR,
> - "IPMI KCS read response header failed Status(%r), " \
> - "RspNetFunctionLun = 0x%x, " \
> - "Command = 0x%x \n",
> - Status,
> - RspHeader.NetFunc,
> - RspHeader.Command
> - ));
> - return (Status);
> + //
> + // Print out the response payloads.
> + HelperManageabilityDebugPrint ((VOID *)&RspHeader, RspHeaderSize,
> + "KCS Response Header:\n");
> }
>
> - //
> - // Print out the response payloads.
> - HelperManageabilityDebugPrint ((VOID *)&RspHeader,
> (UINT16)RspHeaderSize, "KCS Response Header:\n");
> -
> if ((ResponseData != NULL) && (ResponseDataSize != NULL) &&
> (*ResponseDataSize != 0)) {
> - Status = KcsTransportRead ((UINT8 *)ResponseData, ResponseDataSize);
> + ExpectedResponseDataSize = *ResponseDataSize;
> + Status = KcsTransportRead ((UINT8 *)ResponseData,
> ResponseDataSize);
> if (EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_ERROR, "IPMI KCS response read Failed with Status(%r)",
> Status));
> + DEBUG ((DEBUG_ERROR, "KCS response read Failed with Status(%r)",
> + Status));
> }
>
> //
> // Print out the response payloads.
> - HelperManageabilityDebugPrint ((VOID *)ResponseData, *ResponseDataSize,
> "KCS Response Data:\n");
> + 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;
> + }
> +
> + HelperManageabilityDebugPrint ((VOID *)ResponseData,
> (UINT32)*ResponseDataSize, "KCS Response Data:\n");
> + }
> } else {
> *ResponseDataSize = 0;
> }
> diff --git
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/Mana
> geabilityTransportKcs.c
> b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/Mana
> geabilityTransportKcs.c
> index c236354605..9175556a26 100644
> ---
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/Mana
> geabilityTransportKcs.c
> +++ b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe
> +++ /ManageabilityTransportKcs.c
> @@ -10,6 +10,7 @@
> #include <Uefi.h>
> #include <IndustryStandard/IpmiKcs.h>
> #include <Library/IoLib.h>
> +#include <Library/BaseMemoryLib.h>
> #include <Library/DebugLib.h>
> #include <Library/MemoryAllocationLib.h> #include
> <Library/ManageabilityTransportLib.h>
> @@ -225,13 +226,6 @@ KcsTransportTransmitReceive (
> return;
> }
>
> - // Transmit header is necessary for KCS transport, which could be
> - // NetFn, Command and etc.
> - if (TransferToken->TransmitHeader == NULL) {
> - TransferToken->TransferStatus = EFI_INVALID_PARAMETER;
> - return;
> - }
> -
> Status = KcsTransportSendCommand (
> TransferToken->TransmitHeader,
> TransferToken->TransmitHeaderSize,
> @@ -354,6 +348,22 @@ GetTransportCapability (
> }
>
> *TransportCapability = 0;
> + if (CompareGuid (
> + TransportToken->ManageabilityProtocolSpecification,
> + &gManageabilityProtocolIpmiGuid
> + ))
> + {
> + *TransportCapability |=
> +
> (MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_NOT_AVAIL
> ABLE
> + <<
> MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_BIT_POSITIO
> N);
> + } else if (CompareGuid (
> + TransportToken->ManageabilityProtocolSpecification,
> + &gManageabilityProtocolMctpGuid
> + ))
> + {
> + *TransportCapability |=
> + (MCTP_KCS_MTU_IN_POWER_OF_2 <<
> +
> MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_BIT_POSITIO
> N);
> + }
> +
> return EFI_SUCCESS;
> }
>
> --
> 2.37.1.windows.1
next prev parent reply other threads:[~2023-04-20 6:53 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-18 7:15 [edk2-platforms][PATCH V2 00/14] ManageabilityPkg part II Chang, Abner
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 01/14] ManageabilityPkg: Add more helper functions Chang, Abner
2023-04-19 5:29 ` Attar, AbdulLateef (Abdul Lateef)
2023-04-20 15:18 ` Chang, Abner
2023-04-20 6:42 ` Nickle Wang
2023-04-20 15:45 ` Chang, Abner
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 02/14] ManageabilityPkg: Support Maximum Transfer Unit Chang, Abner
2023-04-20 4:28 ` Attar, AbdulLateef (Abdul Lateef)
2023-04-20 6:08 ` [edk2-devel] " Tinh Nguyen
2023-04-21 0:51 ` Chang, Abner
2023-04-21 6:59 ` Tinh Nguyen
2023-04-21 7:09 ` Chang, Abner
[not found] ` <1757E190B31AA266.29498@groups.io>
2023-04-24 1:42 ` Chang, Abner
[not found] ` <1757CCE5059D419D.29498@groups.io>
2023-04-21 5:26 ` Chang, Abner
2023-04-20 6:44 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 03/14] ManageabilityPkg: Add HeaderSize and TrailerSize Chang, Abner
2023-04-20 6:47 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 04/14] ManageabilityPkg: Add PldmProtocolLib Chang, Abner
2023-04-20 6:50 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 05/14] ManageabilityPkg: Add PldmSmbiosTransferDxe driver Chang, Abner
2023-04-20 6:52 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 06/14] ManageabilityPkg/KCS: KCS transport interface Chang, Abner
2023-04-20 6:53 ` Nickle Wang [this message]
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 07/14] ManageabilityPkg: Add definitions of MCTP Chang, Abner
2023-04-20 6:54 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 08/14] ManageabilityPkg: Add MCTP manageability header file Chang, Abner
2023-04-20 6:56 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 09/14] ManageabilityPkg/MctpProtocol: Add MctpProtocol Chang, Abner
2023-04-20 7:00 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 10/14] ManageabilityPkg: Add MCTP transport interface Chang, Abner
2023-04-20 7:04 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 11/14] ManageabilityPkg/PldmProtocol: Add PLDM protocol Chang, Abner
2023-04-20 7:05 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 12/14] ManageabilityPkg: Add Manageability PCDs Chang, Abner
2023-04-20 5:26 ` Tinh Nguyen
2023-04-20 7:07 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 13/14] ManageabilityPkg: Relocate Manageability.dsc Chang, Abner
2023-04-20 5:28 ` Tinh Nguyen
2023-04-20 7:08 ` Nickle Wang
2023-04-18 7:15 ` [edk2-platforms][PATCH V2 14/14] ManageabilityPkg: Add Manageability FDFs Chang, Abner
2023-04-20 5:29 ` Tinh Nguyen
2023-04-20 7:08 ` Nickle Wang
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=MW4PR12MB70314BBDE5FC7D775D5B23CBD9639@MW4PR12MB7031.namprd12.prod.outlook.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