On 4/21/2023 7:51 AM, Chang, Abner wrote:
[EXTERNAL EMAIL NOTICE: This email originated from an external sender. Please be mindful of safe email handling and proprietary information protection practices.] [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) +#defineMANAGEABILITY_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_0MANAGEABILITY_TRANSPORT_FUNCTION_V1_0;typedef struct _MANAGEABILITY_TRANSPORTMANAGEABILITY_TRANSPORT;typedef struct _MANAGEABILITY_TRANSPORT_TOKENMANAGEABILITY_TRANSPORT_TOKEN;@@ -68,8 +71,17 @@ typedef UINT32MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS;/// Additional transport interface features. /// typedef UINT32 MANAGEABILITY_TRANSPORT_CAPABILITY; +/// Bit 0 #defineMANAGEABILITY_TRANSPORT_CAPABILITY_MULTIPLE_TRANSFER_TOKENS 0x00000001-#defineMANAGEABILITY_TRANSPORT_CAPABILITY_ASYNCHRONOUS_TRANSFER 0x00000002+/// Bit 1 +#defineMANAGEABILITY_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.
yes, that is usually, but specification does not require it. I concern that someone will implement another size of payload
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.
Agree, I also just use it for validation.
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 MaxpayloadsizeFor DXE drivers, the Maxpayloadsize is defined as global variable.
I think we should take note somewhere
I do not believe ROM size is an issue of the system which can support those features, so let's keep themto caller?+#defineMANAGEABILITY_TRANSPORT_CAPABILITY_MAXIMUM_PAYLOAD_MASK 0x000000f8+#defineMANAGEABILITY_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 transportcapabilities.- See the definitions of - MANAGEABILITY_TRANSPORT_CAPABILITY. - + This function returns the transport capabilities according to the + manageability protocol. + + @param [in] TransportToken Transport token acquired frommanageability+ transport library. + @param [out] TransportFeature Pointer to receive transportcapabilities.+ See the definitions of + MANAGEABILITY_TRANSPORT_CAPABILITY. + @retval EFI_SUCCESS TransportCapability is returnedsuccessfully.+ @retval EFI_INVALID_PARAMETER TransportToken is not a validtoken.**/ -VOID +EFI_STATUS GetTransportCapability ( + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, OUT MANAGEABILITY_TRANSPORT_CAPABILITY *TransportCapability ); diff --gita/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/ManageabilityTransportKcs.hb/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Comm on/ManageabilityTransportKcs.h index f1758ffd8f..2cdf60ba7e 100644 ---a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Common/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 transportcapabilities.- See the definitions of - MANAGEABILITY_TRANSPORT_CAPABILITY. + This function returns the transport capabilities according to the + manageability protocol. + @param [in] TransportToken Transport token acquired frommanageability+ transport library. + @param [out] TransportFeature Pointer to receive transportcapabilities.+ See the definitions of + MANAGEABILITY_TRANSPORT_CAPABILITY. + @retval EFI_SUCCESS TransportCapability is returnedsuccessfully.+ @retval EFI_INVALID_PARAMETER TransportToken is not a validtoken.**/ -VOID +EFI_STATUS GetTransportCapability ( + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, OUT MANAGEABILITY_TRANSPORT_CAPABILITY *TransportCapability ) { *TransportCapability = 0; + return EFI_SUCCESS; } /** diff --gita/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/ManageabilityTransportKcs.cb/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/ManageabilityTransportKcs.c index ab416e5449..7d85378fc1 100644 ---a/Features/ManageabilityPkg/Library/ManageabilityTransportKcsLib/Dxe/ManageabilityTransportKcs.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, usedfault 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 hardwarefor+ %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 Abnerreturn 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 transfertoken.\n", __FUNCTION__));return; } @@ -298,6 +298,7 @@ AcquireTransportSession ( DEBUG ((DEBUG_ERROR, "%a: Fail to allocate memory forMANAGEABILITY_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 transportcapabilities.- See the definitions of - MANAGEABILITY_TRANSPORT_CAPABILITY. - + This function returns the transport capabilities according to the + manageability protocol. + + @param [in] TransportToken Transport token acquired frommanageability+ transport library. + @param [out] TransportFeature Pointer to receive transportcapabilities.+ See the definitions of + MANAGEABILITY_TRANSPORT_CAPABILITY. + @retval EFI_SUCCESS TransportCapability is returnedsuccessfully.+ @retval EFI_INVALID_PARAMETER TransportToken is not a validtoken.**/ -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_INFORMATIONmHardwareInformation;/** @@ -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_STATUSTransportAdditionalStatus;MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATIONHardwareInformation;- 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 forPEI_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 forEFI_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.cb/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.cindex 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_INFORMATIONmHardwareInformation;/** @@ -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 (#103377): https://edk2.groups.io/g/devel/message/103377 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] -=-=-=-=-=-=-=-=-=-=-=-
