[AMD Official Use Only - General]
> -----Original Message-----
> From: Tinh Nguyen <tinhngu...@amperemail.onmicrosoft.com>
> Sent: Thursday, April 20, 2023 2:08 PM
> To: devel@edk2.groups.io; Chang, Abner <abner.ch...@amd.com>
> Cc: Isaac Oram <isaac.w.o...@intel.com>; Attar, AbdulLateef (Abdul Lateef)
> <abdullateef.at...@amd.com>; Nickle Wang <nick...@nvidia.com>; Igor
> Kulchytskyy <ig...@ami.com>
> Subject: Re: [edk2-devel] [edk2-platforms][PATCH V2 02/14]
> ManageabilityPkg: Support Maximum Transfer Unit
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Hi Abner,
>
> I have some inline comments below
>
> On 18/04/2023 14:15, Chang, Abner via groups.io wrote:
> > From: Abner Chang <abner.ch...@amd.com>
> >
> > Update GetTransportCapability to support Maximum Transfer Unit (MTU)
> > of transport interface.
> >
> > Signed-off-by: Abner Chang <abner.ch...@amd.com>
> > Cc: Isaac Oram <isaac.w.o...@intel.com>
> > Cc: Abdul Lateef Attar <abdat...@amd.com>
> > Cc: Nickle Wang <nick...@nvidia.com>
> > Cc: Igor Kulchytskyy <ig...@ami.com>
> > ---
> > .../Library/ManageabilityTransportLib.h | 33 ++++++++---
> > .../Common/ManageabilityTransportKcs.h | 2 +
> > .../IpmiProtocol/Pei/IpmiPpiInternal.h | 8 ++-
> > .../BaseManageabilityTransportNull.c | 18 ++++--
> > .../Dxe/ManageabilityTransportKcs.c | 57 +++++++++++--------
> > .../Universal/IpmiProtocol/Dxe/IpmiProtocol.c | 24 ++++++--
> > .../Universal/IpmiProtocol/Pei/IpmiPpi.c | 51 ++++++++++-------
> > .../Universal/IpmiProtocol/Smm/IpmiProtocol.c | 24 ++++++--
> > 8 files changed, 145 insertions(+), 72 deletions(-)
> >
> > diff --git
> > a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLib.
> > h
> > b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLib.
> > h
> > index c022b4ac5c..d86d0d87d5 100644
> > ---
> > a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLib.
> > h
> > +++ b/Features/ManageabilityPkg/Include/Library/ManageabilityTransport
> > +++ Lib.h
> > @@ -14,6 +14,9 @@
> > #define MANAGEABILITY_TRANSPORT_TOKEN_VERSION
> ((MANAGEABILITY_TRANSPORT_TOKEN_VERSION_MAJOR << 8) |\
> >
> > MANAGEABILITY_TRANSPORT_TOKEN_VERSION_MINOR)
> >
> > +#define
> MANAGEABILITY_TRANSPORT_PAYLOAD_SIZE_FROM_CAPABILITY(a) (1 <<
> ((a &
> MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_MASK)
> >>\
> > +
> >
> +MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_BIT_PO
> SITION))
> > +
> > typedef struct _MANAGEABILITY_TRANSPORT_FUNCTION_V1_0
> MANAGEABILITY_TRANSPORT_FUNCTION_V1_0;
> > typedef struct _MANAGEABILITY_TRANSPORT
> MANAGEABILITY_TRANSPORT;
> > typedef struct _MANAGEABILITY_TRANSPORT_TOKEN
> MANAGEABILITY_TRANSPORT_TOKEN;
> > @@ -68,8 +71,17 @@ typedef UINT32
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS;
> > /// Additional transport interface features.
> > ///
> > typedef UINT32 MANAGEABILITY_TRANSPORT_CAPABILITY;
> > +/// Bit 0
> > #define
> MANAGEABILITY_TRANSPORT_CAPABILITY_MULTIPLE_TRANSFER_TOKENS
> 0x00000001
> > -#define
> MANAGEABILITY_TRANSPORT_CAPABILITY_ASYNCHRONOUS_TRANSFER
> 0x00000002
> > +/// Bit 1
> > +#define
> MANAGEABILITY_TRANSPORT_CAPABILITY_ASYNCHRONOUS_TRANSFER
> 0x00000002
> > +/// Bit 2 - Reserved
> > +/// Bit 7:3 - Transport interface maximum payload size, which is (2 ^
> > bit[7:3]
> - 1)
> > +/// bit[7:3] means no maximum payload.
>
> I am confused with your definition here.
>
> Why does it have to be a power of 2?
Usually the maximum/minimum is in power of 2 and use power of 2 has less bits
occupied from MANAGEABILITY_TRANSPORT_CAPABILITY.
>
> And we should separate request payload size and response payload size
>
> Can you clarify more about that?
Do we really need the maximum size for response? Response is initiated by
target endpoint and suppose the payload header should have some fields that
indicate the return payload is only part of response. The size of payload
returned is actually maximum transfer size that target endpoint can handle.
Do you know any case that receiver has no idea about if the payload sent back
from target endpoint is a partial of response or not? We should have MTU
response if it is required for the transport interface.
>
> Another question, only PEI_IPMI_PPI_INTERNAL contains MaxPayloadSize,
PPI has MaxPayloadSize in structure is because we can't define a global
variable for PEI module as module is executed in place.
> how do IPMI/MCTP/PLDM protocol provide Maxpayloadsize
For DXE drivers, the Maxpayloadsize is defined as global variable.
>
> to caller?
>
> > +#define
> MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_MASK
> 0x000000f8
> > +#define
> MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_BIT_POS
> ITION 3
> > +#define
> >
> +MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_NOT_A
> VAILABLE
> > +0x00 /// Bit 8:31 - Reserved
> >
> > ///
> > /// Definitions of Manageability transport interface functions.
> > @@ -187,15 +199,20 @@ AcquireTransportSession (
> > );
> >
> > /**
> > - This function returns the transport capabilities.
> > -
> > - @param [out] TransportFeature Pointer to receive transport
> capabilities.
> > - See the definitions of
> > - MANAGEABILITY_TRANSPORT_CAPABILITY.
> > -
> > + This function returns the transport capabilities according to the
> > + manageability protocol.
> > +
> > + @param [in] TransportToken Transport token acquired from
> manageability
> > + transport library.
> > + @param [out] TransportFeature Pointer to receive transport
> capabilities.
> > + See the definitions of
> > +
> > MANAGEABILITY_TRANSPORT_CAPABILITY.
> > + @retval EFI_SUCCESS TransportCapability is returned
> successfully.
> > + @retval EFI_INVALID_PARAMETER TransportToken is not a valid
> token.
> > **/
> > -VOID
> > +EFI_STATUS
> > GetTransportCapability (
> > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken,
> > OUT MANAGEABILITY_TRANSPORT_CAPABILITY *TransportCapability
> > );
> >
> > diff --git
> >
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Commo
> > n/ManageabilityTransportKcs.h
> >
> b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Comm
> o
> > n/ManageabilityTransportKcs.h
> > index f1758ffd8f..2cdf60ba7e 100644
> > ---
> >
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Commo
> > n/ManageabilityTransportKcs.h
> > +++ b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/C
> > +++ ommon/ManageabilityTransportKcs.h
> > @@ -32,6 +32,8 @@ typedef struct {
> > #define IPMI_KCS_GET_STATE(s) (s >> 6)
> > #define IPMI_KCS_SET_STATE(s) (s << 6)
> >
> > +#define MCTP_KCS_MTU_IN_POWER_OF_2 8
> > +
> > /// 5 sec, according to IPMI spec
> > #define IPMI_KCS_TIMEOUT_5_SEC 5000*1000
> > #define IPMI_KCS_TIMEOUT_1MS 1000
> > diff --git
> > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiInternal
> > .h
> > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiInternal
> > .h
> > index bbe0c8c5cb..4b6bdc97a9 100644
> > ---
> > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiInternal
> > .h
> > +++ b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiInte
> > +++ rnal.h
> > @@ -17,9 +17,11 @@
> > #define MANAGEABILITY_IPMI_PPI_INTERNAL_FROM_LINK(a) CR (a,
> > PEI_IPMI_PPI_INTERNAL, PeiIpmiPpi,
> > MANAGEABILITY_IPMI_PPI_INTERNAL_SIGNATURE)
> >
> > typedef struct {
> > - UINT32 Signature;
> > - MANAGEABILITY_TRANSPORT_TOKEN *TransportToken;
> > - PEI_IPMI_PPI PeiIpmiPpi;
> > + UINT32 Signature;
> > + MANAGEABILITY_TRANSPORT_TOKEN *TransportToken;
> > + MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability;
> > + UINT32 TransportMaximumPayload;
> > + PEI_IPMI_PPI PeiIpmiPpi;
> > } PEI_IPMI_PPI_INTERNAL;
> >
> > #endif // MANAGEABILITY_IPMI_PPI_INTERNAL_H_
> > diff --git
> > a/Features/ManageabilityPkg/Library/BaseManageabilityTransportNullLib/
> > BaseManageabilityTransportNull.c
> > b/Features/ManageabilityPkg/Library/BaseManageabilityTransportNullLib/
> > BaseManageabilityTransportNull.c
> > index 49fc8c0f71..3aa68578a6 100644
> > ---
> > a/Features/ManageabilityPkg/Library/BaseManageabilityTransportNullLib/
> > BaseManageabilityTransportNull.c
> > +++
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportNull
> > +++ Lib/BaseManageabilityTransportNull.c
> > @@ -31,19 +31,25 @@ AcquireTransportSession (
> > }
> >
> > /**
> > - This function returns the transport capabilities.
> > -
> > - @param [out] TransportFeature Pointer to receive transport
> capabilities.
> > - See the definitions of
> > - MANAGEABILITY_TRANSPORT_CAPABILITY.
> > + This function returns the transport capabilities according to the
> > + manageability protocol.
> >
> > + @param [in] TransportToken Transport token acquired from
> manageability
> > + transport library.
> > + @param [out] TransportFeature Pointer to receive transport
> capabilities.
> > + See the definitions of
> > +
> > MANAGEABILITY_TRANSPORT_CAPABILITY.
> > + @retval EFI_SUCCESS TransportCapability is returned
> successfully.
> > + @retval EFI_INVALID_PARAMETER TransportToken is not a valid
> token.
> > **/
> > -VOID
> > +EFI_STATUS
> > GetTransportCapability (
> > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken,
> > OUT MANAGEABILITY_TRANSPORT_CAPABILITY *TransportCapability
> > )
> > {
> > *TransportCapability = 0;
> > + return EFI_SUCCESS;
> > }
> >
> > /**
> > diff --git
> >
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/M
> > anageabilityTransportKcs.c
> >
> b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/M
> > anageabilityTransportKcs.c
> > index ab416e5449..7d85378fc1 100644
> > ---
> >
> a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/M
> > anageabilityTransportKcs.c
> > +++ b/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/D
> > +++ xe/ManageabilityTransportKcs.c
> > @@ -62,7 +62,7 @@ KcsTransportInit (
> > }
> >
> > if (HardwareInfo.Kcs == NULL) {
> > - DEBUG ((DEBUG_INFO, "%a: Hardware information is not provided, use
> dfault settings.\n", __FUNCTION__));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Hardware information is
> > + not provided, use dfault settings.\n", __FUNCTION__));
> > mKcsHardwareInfo.MemoryMap =
> MANAGEABILITY_TRANSPORT_KCS_IO_MAP_IO;
> > mKcsHardwareInfo.IoBaseAddress.IoAddress16 = PcdGet16
> (PcdIpmiKcsIoBaseAddress);
> > mKcsHardwareInfo.IoDataInAddress.IoAddress16 =
> > mKcsHardwareInfo.IoBaseAddress.IoAddress16 +
> IPMI_KCS_DATA_IN_REGISTER_OFFSET; @@ -81,21 +81,21 @@
> KcsTransportInit (
> > // Get protocol specification name.
> > ManageabilityProtocolName = HelperManageabilitySpecName
> > (TransportToken->ManageabilityProtocolSpecification);
> >
> > - DEBUG ((DEBUG_INFO, "%a: KCS transport hardware for %s is:\n",
> > __FUNCTION__, ManageabilityProtocolName));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: KCS transport hardware
> for
> > + %s is:\n", __FUNCTION__, ManageabilityProtocolName));
> > if (mKcsHardwareInfo.MemoryMap) {
> > - DEBUG ((DEBUG_INFO, "Memory Map I/O\n", __FUNCTION__));
> > - DEBUG ((DEBUG_INFO, "Base Memory Address : 0x%08x\n",
> mKcsHardwareInfo.IoBaseAddress.IoAddress32));
> > - DEBUG ((DEBUG_INFO, "Data in Address : 0x%08x\n",
> mKcsHardwareInfo.IoDataInAddress.IoAddress32));
> > - DEBUG ((DEBUG_INFO, "Data out Address : 0x%08x\n",
> mKcsHardwareInfo.IoDataOutAddress.IoAddress32));
> > - DEBUG ((DEBUG_INFO, "Command Address : 0x%08x\n",
> mKcsHardwareInfo.IoCommandAddress.IoAddress32));
> > - DEBUG ((DEBUG_INFO, "Status Address : 0x%08x\n",
> mKcsHardwareInfo.IoStatusAddress.IoAddress32));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Memory Map I/O\n",
> __FUNCTION__));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Base Memory Address :
> 0x%08x\n", mKcsHardwareInfo.IoBaseAddress.IoAddress32));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Data in Address :
> 0x%08x\n", mKcsHardwareInfo.IoDataInAddress.IoAddress32));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Data out Address :
> 0x%08x\n", mKcsHardwareInfo.IoDataOutAddress.IoAddress32));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Command Address :
> 0x%08x\n", mKcsHardwareInfo.IoCommandAddress.IoAddress32));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Status Address :
> 0x%08x\n", mKcsHardwareInfo.IoStatusAddress.IoAddress32));
> > } else {
> > - DEBUG ((DEBUG_INFO, "I/O Map I/O\n"));
> > - DEBUG ((DEBUG_INFO, "Base I/O port : 0x%04x\n",
> mKcsHardwareInfo.IoBaseAddress.IoAddress16));
> > - DEBUG ((DEBUG_INFO, "Data in I/O port : 0x%04x\n",
> mKcsHardwareInfo.IoDataInAddress.IoAddress16));
> > - DEBUG ((DEBUG_INFO, "Data out I/O port: 0x%04x\n",
> mKcsHardwareInfo.IoDataOutAddress.IoAddress16));
> > - DEBUG ((DEBUG_INFO, "Command I/O port : 0x%04x\n",
> mKcsHardwareInfo.IoCommandAddress.IoAddress16));
> > - DEBUG ((DEBUG_INFO, "Status I/O port : 0x%04x\n",
> mKcsHardwareInfo.IoStatusAddress.IoAddress16));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "I/O Map I/O\n"));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Base I/O port : 0x%04x\n",
> mKcsHardwareInfo.IoBaseAddress.IoAddress16));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Data in I/O port : 0x%04x\n",
> mKcsHardwareInfo.IoDataInAddress.IoAddress16));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Data out I/O port:
> 0x%04x\n", mKcsHardwareInfo.IoDataOutAddress.IoAddress16));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Command I/O port :
> 0x%04x\n", mKcsHardwareInfo.IoCommandAddress.IoAddress16));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Status I/O port : 0x%04x\n",
> > + mKcsHardwareInfo.IoStatusAddress.IoAddress16));
> > }
> if those code is just for debugging, you should put them into
> DEBUG_CODE_BEGIN() and DEBUG_CODE_END()
DEBUG_MANAGEABILITY is got approval and we can use that to enable the print
error level for Manageability stuff. Use DEBUG_CODE_BEGIN requires user to
enable DEBUG_PROPERTY_DEBUG_CODE_ENABLED additionally, it saves ROM size though.
How do you think? Which way is convenient to users and also have a good code
structure?
Thanks
Abner
> >
> > return EFI_SUCCESS;
> > @@ -221,7 +221,7 @@ KcsTransportTransmitReceive (
> > EFI_STATUS Status;
> > MANAGEABILITY_IPMI_TRANSPORT_HEADER *TransmitHeader;
> >
> > - if (TransportToken == NULL || TransferToken == NULL) {
> > + if ((TransportToken == NULL) || (TransferToken == NULL)) {
> > DEBUG ((DEBUG_ERROR, "%a: Invalid transport token or transfer
> token.\n", __FUNCTION__));
> > return;
> > }
> > @@ -298,6 +298,7 @@ AcquireTransportSession (
> > DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory for
> MANAGEABILITY_TRANSPORT_KCS\n", __FUNCTION__));
> > return EFI_OUT_OF_RESOURCES;
> > }
> > +
> > KcsTransportToken->Token.Transport = AllocateZeroPool (sizeof
> (MANAGEABILITY_TRANSPORT));
> > if (KcsTransportToken->Token.Transport == NULL) {
> > FreePool (KcsTransportToken);
> > @@ -329,21 +330,29 @@ AcquireTransportSession (
> > }
> >
> > /**
> > - This function returns the transport capabilities.
> > -
> > - @param [out] TransportFeature Pointer to receive transport
> capabilities.
> > - See the definitions of
> > - MANAGEABILITY_TRANSPORT_CAPABILITY.
> > -
> > + This function returns the transport capabilities according to the
> > + manageability protocol.
> > +
> > + @param [in] TransportToken Transport token acquired from
> manageability
> > + transport library.
> > + @param [out] TransportFeature Pointer to receive transport
> capabilities.
> > + See the definitions of
> > +
> > MANAGEABILITY_TRANSPORT_CAPABILITY.
> > + @retval EFI_SUCCESS TransportCapability is returned
> successfully.
> > + @retval EFI_INVALID_PARAMETER TransportToken is not a valid
> token.
> > **/
> > -VOID
> > +EFI_STATUS
> > GetTransportCapability (
> > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken,
> > OUT MANAGEABILITY_TRANSPORT_CAPABILITY *TransportCapability
> > )
> > {
> > - if (TransportCapability != NULL) {
> > - *TransportCapability = 0;
> > + if ((TransportToken == NULL) || (TransportCapability == NULL)) {
> > + return EFI_INVALID_PARAMETER;
> > }
> > +
> > + *TransportCapability = 0;
> > + return EFI_SUCCESS;
> > }
> >
> > /**
> > diff --git
> > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c
> > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c
> > index 05175ee448..51d5c7f0ba 100644
> > ---
> > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c
> > +++
> b/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtoco
> > +++ l.c
> > @@ -17,9 +17,9 @@
> >
> > #include "IpmiProtocolCommon.h"
> >
> > -MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL;
> > -CHAR16 *mTransportName;
> > -
> > +MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL;
> > +CHAR16 *mTransportName;
> > +UINT32 TransportMaximumPayload;
> > MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION
> mHardwareInformation;
> >
> > /**
> > @@ -92,8 +92,6 @@ DxeIpmiEntry (
> > MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability;
> > MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS
> > TransportAdditionalStatus;
> >
> > - GetTransportCapability (&TransportCapability);
> > -
> > Status = HelperAcquireManageabilityTransport (
> > &gManageabilityProtocolIpmiGuid,
> > &mTransportToken
> > @@ -103,8 +101,22 @@ DxeIpmiEntry (
> > return Status;
> > }
> >
> > + Status = GetTransportCapability (mTransportToken,
> > + &TransportCapability); if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "%a: Failed to GetTransportCapability().\n",
> __FUNCTION__));
> > + return Status;
> > + }
> > +
> > + TransportMaximumPayload =
> > + MANAGEABILITY_TRANSPORT_PAYLOAD_SIZE_FROM_CAPABILITY
> (TransportCapability); if (TransportMaximumPayload == (1 <<
> MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_NOT_AV
> AILABLE)) {
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface
> > + maximum payload is undefined.\n", __FUNCTION__)); } else {
> > + TransportMaximumPayload -= 1;
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface for
> > + IPMI protocol has maximum payload %x.\n", __FUNCTION__,
> > + TransportMaximumPayload)); }
> > +
> > mTransportName = HelperManageabilitySpecName
> > (mTransportToken->Transport->ManageabilityTransportSpecification);
> > - DEBUG ((DEBUG_INFO, "%a: IPMI protocol over %s.\n", __FUNCTION__,
> > mTransportName));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: IPMI protocol over
> %s.\n",
> > + __FUNCTION__, mTransportName));
> >
> > //
> > // Setup hardware information according to the transport interface.
> > diff --git
> > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c
> > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c
> > index f839cd7387..8bf1e794f0 100644
> > --- a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c
> > +++ b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c
> > @@ -51,19 +51,19 @@ PeiIpmiSubmitCommand (
> > IN OUT UINT32 *ResponseDataSize
> > )
> > {
> > - EFI_STATUS Status;
> > - PEI_IPMI_PPI_INTERNAL *PeiIpmiPpiinternal;
> > -
> > - PeiIpmiPpiinternal =
> > MANAGEABILITY_IPMI_PPI_INTERNAL_FROM_LINK(This);
> > - Status = CommonIpmiSubmitCommand (
> > - PeiIpmiPpiinternal->TransportToken,
> > - NetFunction,
> > - Command,
> > - RequestData,
> > - RequestDataSize,
> > - ResponseData,
> > - ResponseDataSize
> > - );
> > + EFI_STATUS Status;
> > + PEI_IPMI_PPI_INTERNAL *PeiIpmiPpiinternal;
> > +
> > + PeiIpmiPpiinternal = MANAGEABILITY_IPMI_PPI_INTERNAL_FROM_LINK
> (This);
> > + Status = CommonIpmiSubmitCommand (
> > + PeiIpmiPpiinternal->TransportToken,
> > + NetFunction,
> > + Command,
> > + RequestData,
> > + RequestDataSize,
> > + ResponseData,
> > + ResponseDataSize
> > + );
> > return Status;
> > }
> >
> > @@ -87,29 +87,28 @@ PeiIpmiEntry (
> > CHAR16 *TransportName;
> > PEI_IPMI_PPI_INTERNAL *PeiIpmiPpiinternal;
> > EFI_PEI_PPI_DESCRIPTOR *PpiDescriptor;
> > - MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability;
> > MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS
> TransportAdditionalStatus;
> > MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION
> HardwareInformation;
> >
> > - PeiIpmiPpiinternal = (PEI_IPMI_PPI_INTERNAL *)AllocateZeroPool
> > (sizeof(PEI_IPMI_PPI_INTERNAL));
> > + PeiIpmiPpiinternal = (PEI_IPMI_PPI_INTERNAL *)AllocateZeroPool
> > + (sizeof (PEI_IPMI_PPI_INTERNAL));
> > if (PeiIpmiPpiinternal == NULL) {
> > DEBUG ((DEBUG_ERROR, "%a: Not enough memory for
> PEI_IPMI_PPI_INTERNAL.\n", __FUNCTION__));
> > return EFI_OUT_OF_RESOURCES;
> > }
> > - PpiDescriptor = (EFI_PEI_PPI_DESCRIPTOR *)AllocateZeroPool
> > (sizeof(EFI_PEI_PPI_DESCRIPTOR));
> > +
> > + PpiDescriptor = (EFI_PEI_PPI_DESCRIPTOR *)AllocateZeroPool (sizeof
> > + (EFI_PEI_PPI_DESCRIPTOR));
> > if (PpiDescriptor == NULL) {
> > DEBUG ((DEBUG_ERROR, "%a: Not enough memory for
> EFI_PEI_PPI_DESCRIPTOR.\n", __FUNCTION__));
> > return EFI_OUT_OF_RESOURCES;
> > }
> >
> > - PeiIpmiPpiinternal->Signature =
> > MANAGEABILITY_IPMI_PPI_INTERNAL_SIGNATURE;
> > + PeiIpmiPpiinternal->Signature =
> MANAGEABILITY_IPMI_PPI_INTERNAL_SIGNATURE;
> > PeiIpmiPpiinternal->PeiIpmiPpi.IpmiSubmitCommand =
> > PeiIpmiSubmitCommand;
> >
> > PpiDescriptor->Flags = EFI_PEI_PPI_DESCRIPTOR_PPI |
> EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST;
> > PpiDescriptor->Guid = &gPeiIpmiPpiGuid;
> > PpiDescriptor->Ppi = &PeiIpmiPpiinternal->PeiIpmiPpi;
> >
> > - GetTransportCapability (&TransportCapability);
> > Status = HelperAcquireManageabilityTransport (
> > &gManageabilityProtocolIpmiGuid,
> > &PeiIpmiPpiinternal->TransportToken
> > @@ -119,8 +118,22 @@ PeiIpmiEntry (
> > return Status;
> > }
> >
> > + Status = GetTransportCapability
> > + (PeiIpmiPpiinternal->TransportToken,
> > + &PeiIpmiPpiinternal->TransportCapability);
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "%a: Failed to GetTransportCapability().\n",
> __FUNCTION__));
> > + return Status;
> > + }
> > +
> > + PeiIpmiPpiinternal->TransportMaximumPayload =
> > + MANAGEABILITY_TRANSPORT_PAYLOAD_SIZE_FROM_CAPABILITY
> > + (PeiIpmiPpiinternal->TransportCapability);
> > + if (PeiIpmiPpiinternal->TransportMaximumPayload == (1 <<
> MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_NOT_AV
> AILABLE)) {
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface
> > + maximum payload is undefined.\n", __FUNCTION__)); } else {
> > + PeiIpmiPpiinternal->TransportMaximumPayload -= 1;
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface for
> > + IPMI protocol has maximum payload 0x%x.\n", __FUNCTION__,
> > + PeiIpmiPpiinternal->TransportMaximumPayload));
> > + }
> > +
> > TransportName = HelperManageabilitySpecName
> > (PeiIpmiPpiinternal->TransportToken->Transport->ManageabilityTransport
> > Specification);
> > - DEBUG ((DEBUG_INFO, "%a: IPMI protocol over %s.\n", __FUNCTION__,
> > TransportName));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: IPMI protocol over
> %s.\n",
> > + __FUNCTION__, TransportName));
> >
> > //
> > // Setup hardware information according to the transport interface.
> > diff --git
> > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c
> >
> b/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c
> > index 87a5436bdf..e4cd166b7a 100644
> > ---
> > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c
> > +++
> b/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtoco
> > +++ l.c
> > @@ -18,9 +18,9 @@
> >
> > #include "IpmiProtocolCommon.h"
> >
> > -MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL;
> > -CHAR16 *mTransportName;
> > -
> > +MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL;
> > +CHAR16 *mTransportName;
> > +UINT32 TransportMaximumPayload;
> > MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION
> mHardwareInformation;
> >
> > /**
> > @@ -93,8 +93,6 @@ SmmIpmiEntry (
> > MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability;
> > MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS
> > TransportAdditionalStatus;
> >
> > - GetTransportCapability (&TransportCapability);
> > -
> > Status = HelperAcquireManageabilityTransport (
> > &gManageabilityProtocolIpmiGuid,
> > &mTransportToken
> > @@ -104,8 +102,22 @@ SmmIpmiEntry (
> > return Status;
> > }
> >
> > + Status = GetTransportCapability (mTransportToken,
> > + &TransportCapability); if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_ERROR, "%a: Failed to GetTransportCapability().\n",
> __FUNCTION__));
> > + return Status;
> > + }
> > +
> > + TransportMaximumPayload =
> > + MANAGEABILITY_TRANSPORT_PAYLOAD_SIZE_FROM_CAPABILITY
> (TransportCapability); if (TransportMaximumPayload == (1 <<
> MANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_NOT_AV
> AILABLE)) {
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface
> > + maximum payload is undefined.\n", __FUNCTION__)); } else {
> > + TransportMaximumPayload -= 1;
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: Transport interface for
> > + IPMI protocol has maximum payload 0x%x.\n", __FUNCTION__,
> > + TransportMaximumPayload)); }
> > +
> > mTransportName = HelperManageabilitySpecName
> > (mTransportToken->Transport->ManageabilityTransportSpecification);
> > - DEBUG ((DEBUG_INFO, "%a: IPMI protocol over %s.\n", __FUNCTION__,
> > mTransportName));
> > + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: IPMI protocol over
> %s.\n",
> > + __FUNCTION__, mTransportName));
> >
> > //
> > // Setup hardware information according to the transport interface.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103329): https://edk2.groups.io/g/devel/message/103329
Mute This Topic: https://groups.io/mt/98339115/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
