[AMD Official Use Only - General] Hi Tinh, V3 patch just sent, I will wait for your feedback on my response below.
Thanks Abner > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Chang, > Abner via groups.io > Sent: Friday, April 21, 2023 8:51 AM > To: Tinh Nguyen <tinhngu...@amperemail.onmicrosoft.com>; > devel@edk2.groups.io > 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. > > > [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/ManageabilityTranspo > > > +++ rt > > > +++ 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/IpmiPpiIntern > > > al > > > .h > > > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiIntern > > > al > > > .h > > > index bbe0c8c5cb..4b6bdc97a9 100644 > > > --- > > > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiIntern > > > al > > > .h > > > +++ b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiIn > > > +++ te > > > +++ 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/BaseManageabilityTransportNullLi > > > b/ > > > BaseManageabilityTransportNull.c > > > b/Features/ManageabilityPkg/Library/BaseManageabilityTransportNullLi > > > b/ > > > BaseManageabilityTransportNull.c > > > index 49fc8c0f71..3aa68578a6 100644 > > > --- > > > a/Features/ManageabilityPkg/Library/BaseManageabilityTransportNullLi > > > b/ > > > 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->ManageabilityTranspo > > > rt > > > 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 (#103369): https://edk2.groups.io/g/devel/message/103369 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] -=-=-=-=-=-=-=-=-=-=-=-