[AMD Official Use Only - General] Hi Nickle, see my feedbacks below,
> -----Original Message----- > From: Nickle Wang <nick...@nvidia.com> > Sent: Monday, March 20, 2023 9:40 PM > To: Chang, Abner <abner.ch...@amd.com>; devel@edk2.groups.io > Cc: Liming Gao <gaolim...@byosoft.com.cn>; Isaac Oram > <isaac.w.o...@intel.com>; Nate DeSimone > <nathaniel.l.desim...@intel.com>; Attar, AbdulLateef (Abdul Lateef) > <abdullateef.at...@amd.com>; Igor Kulchytskyy <ig...@ami.com> > Subject: RE: [edk2-platforms][PATCH V2 6/8] ManageabilityPkg: Implement > Ipmi Protocol/Ppi > > Caution: This message originated from an External Source. Use proper > caution when opening attachments, clicking links, or responding. > > > Please find my comment inline below. > > Regards, > Nickle > > -----Original Message----- > From: abner.ch...@amd.com <abner.ch...@amd.com> > Sent: Wednesday, March 8, 2023 10:17 PM > To: devel@edk2.groups.io > Cc: Liming Gao <gaolim...@byosoft.com.cn>; Isaac Oram > <isaac.w.o...@intel.com>; Nate DeSimone > <nathaniel.l.desim...@intel.com>; Abdul Lateef Attar > <abdat...@amd.com>; Nickle Wang <nick...@nvidia.com>; Igor Kulchytskyy > <ig...@ami.com> > Subject: [edk2-platforms][PATCH V2 6/8] ManageabilityPkg: Implement Ipmi > Protocol/Ppi > > External email: Use caution opening links or attachments > > > From: Abner Chang <abner.ch...@amd.com> > > Add Ipmi Protocol/Ppi/SMM implementations. > The underlying implementation of transport interface depends on the > binded ManageabilityTransportLib. > > Signed-off-by: Abner Chang <abner.ch...@amd.com> > Cc: Liming Gao <gaolim...@byosoft.com.cn> > Cc: Isaac Oram <isaac.w.o...@intel.com> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > Cc: Abdul Lateef Attar <abdat...@amd.com> > Cc: Nickle Wang <nick...@nvidia.com> > Cc: Igor Kulchytskyy <ig...@ami.com> > --- > .../IpmiProtocol/Dxe/IpmiProtocolDxe.inf | 50 ++++ > .../Universal/IpmiProtocol/Pei/IpmiPpiPei.inf | 51 ++++ > .../IpmiProtocol/Smm/IpmiProtocolSmm.inf | 52 ++++ > .../IpmiProtocol/Common/IpmiProtocolCommon.h | 108 > ++++++++ .../IpmiProtocol/Common/IpmiProtocolCommon.c | 247 > ++++++++++++++++++ .../Universal/IpmiProtocol/Dxe/IpmiProtocol.c | 177 > +++++++++++++ > .../Universal/IpmiProtocol/Pei/IpmiPpi.c | 151 +++++++++++ > .../Universal/IpmiProtocol/Smm/IpmiProtocol.c | 147 +++++++++++ > 8 files changed, 983 insertions(+) > create mode 100644 > Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocolDxe.in > f > create mode 100644 > Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiPei.inf > create mode 100644 > Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocolSmm > .inf > create mode 100644 > Features/ManageabilityPkg/Universal/IpmiProtocol/Common/IpmiProtocolC > ommon.h > create mode 100644 > Features/ManageabilityPkg/Universal/IpmiProtocol/Common/IpmiProtocolC > ommon.c > create mode 100644 > Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c > create mode 100644 > Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c > create mode 100644 > Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c > > diff --git > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocolDxe. > inf > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocolDxe > .inf > new file mode 100644 > index 0000000000..ff5ec56c73 > --- /dev/null > +++ > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocolD > +++ xe.inf > @@ -0,0 +1,50 @@ > +## @file > +# IPMI Protocol DXE Driver. > +# > +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights > +reserved.<BR> # SPDX-License-Identifier: BSD-2-Clause-Patent ## > + > +[Defines] > + INF_VERSION = 0x0001001d > + BASE_NAME = IpmiDxe > + FILE_GUID = BC41B0C2-9D8A-42B5-A28F-02CE0D4A6C28 > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + ENTRY_POINT = DxeIpmiEntry > + UNLOAD_IMAGE = IpmiUnloadImage > + > +# > +# VALID_ARCHITECTURES = IA32 X64 ARM AARCH64 > +# > + > +[Sources] > + IpmiProtocol.c > + ../Common/IpmiProtocolCommon.c > + ../Common/IpmiProtocolCommon.h > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + ManageabilityPkg/ManageabilityPkg.dec > + > +[LibraryClasses] > + BaseMemoryLib > + DebugLib > + ManageabilityTransportHelperLib > + ManageabilityTransportLib > + UefiDriverEntryPoint > + UefiBootServicesTableLib > + > +[Protocols] > + gIpmiProtocolGuid # PROTOCOL ALWAYS_PRODUCED > + > +[Guids] > + gManageabilityProtocolIpmiGuid > + gManageabilityTransportKcsGuid > + > +[FixedPcd] > + gEfiMdePkgTokenSpaceGuid.PcdIpmiKcsBaseAddress > + > +[Depex] > + TRUE > diff --git > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiPei.inf > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiPei.inf > new file mode 100644 > index 0000000000..058bfb63cb > --- /dev/null > +++ b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpiPei.in > +++ f > @@ -0,0 +1,51 @@ > +## @file > +# IPMI Protocol PEI Driver. > +# > +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights > +reserved.<BR> # SPDX-License-Identifier: BSD-2-Clause-Patent ## > + > +[Defines] > + INF_VERSION = 0x0001001d > + BASE_NAME = IpmiPei > + FILE_GUID = 7832F989-CB72-4715-ADCA-35C0B031856C > + MODULE_TYPE = PEIM > + VERSION_STRING = 1.0 > + ENTRY_POINT = PeiIpmiEntry > + > +# > +# The following information is for reference only and not required by the > build tools. > +# > +# VALID_ARCHITECTURES = IA32 X64 ARM AARCH64 > +# > + > +[Sources] > + IpmiPpi.c > + ../Common/IpmiProtocolCommon.c > + ../Common/IpmiProtocolCommon.h > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + ManageabilityPkg/ManageabilityPkg.dec > + > +[LibraryClasses] > + BaseMemoryLib > + DebugLib > + ManageabilityTransportHelperLib > + ManageabilityTransportLib > + PeimEntryPoint > + ManageabilityTransportLib > + > +[Ppis] > + gPeiIpmiPpiGuid # PPI ALWAYS PRODUCED > + > +[Guids] > + gManageabilityProtocolIpmiGuid > + gManageabilityTransportKcsGuid > + > +[FixedPcd] > + gEfiMdePkgTokenSpaceGuid.PcdIpmiKcsBaseAddress > + > +[Depex] > + TRUE > diff --git > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocolSm > m.inf > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocolSm > m.inf > new file mode 100644 > index 0000000000..cae1462f4f > --- /dev/null > +++ > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocolS > +++ mm.inf > @@ -0,0 +1,52 @@ > +## @file > +# IPMI Protocol SMM Driver. > +# > +# Copyright (C) 2023 Advanced Micro Devices, Inc. All rights > +reserved.<BR> # SPDX-License-Identifier: BSD-2-Clause-Patent ## > + > +# > +# The following information is for reference only and not required by the > build tools. > +# > +# VALID_ARCHITECTURES = IA32 X64 ARM AARCH64 > +# > +[Defines] > + INF_VERSION = 0x0001001d > + BASE_NAME = IpmiSmm > + FILE_GUID = CDD5D1DE-E3D3-4B1F-8689-DCC661561BB4 > + MODULE_TYPE = DXE_SMM_DRIVER > + PI_SPECIFICATION_VERSION = 0x0001000A > + VERSION_STRING = 1.0 > + ENTRY_POINT = SmmIpmiEntry > + > +[Sources] > + IpmiProtocol.c > + ../Common/IpmiProtocolCommon.c > + ../Common/IpmiProtocolCommon.h > + > +[Packages] > + MdePkg/MdePkg.dec > + MdeModulePkg/MdeModulePkg.dec > + ManageabilityPkg/ManageabilityPkg.dec > + > +[LibraryClasses] > + BaseMemoryLib > + DebugLib > + ManageabilityTransportHelperLib > + ManageabilityTransportLib > + SmmServicesTableLib > + UefiDriverEntryPoint > + UefiBootServicesTableLib > + > +[Protocols] > + gSmmIpmiProtocolGuid # PROTOCOL ALWAYS_PRODUCED > + > +[Guids] > + gManageabilityProtocolIpmiGuid > + gManageabilityTransportKcsGuid > + > +[FixedPcd] > + gEfiMdePkgTokenSpaceGuid.PcdIpmiKcsBaseAddress > + > +[Depex] > + TRUE > diff --git > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Common/IpmiProtoc > olCommon.h > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Common/IpmiProtoc > olCommon.h > new file mode 100644 > index 0000000000..39684d0617 > --- /dev/null > +++ > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Common/IpmiProtoc > +++ olCommon.h > @@ -0,0 +1,108 @@ > +/** @file > + > + IPMI Manageability Protocol common header file. > + > + Copyright (C) 2023 Advanced Micro Devices, Inc. All rights > +reserved.<BR> > + SPDX-License-Identifier: BSD-2-Clause-Patent **/ > + > +#ifndef MANAGEABILITY_IPMI_COMMON_H_ > +#define MANAGEABILITY_IPMI_COMMON_H_ > + > +#include <IndustryStandard/IpmiKcs.h> > +#include <Library/ManageabilityTransportLib.h> > + > +/// > +/// IPMI KCS hardware information. > +/// > +#define IPMI_KCS_BASE_ADDRESS PcdGet16 (PcdIpmiKcsBaseAddress) > +#define IPMI_KCS_REG_DATA_IN IPMI_KCS_BASE_ADDRESS + > IPMI_KCS_DATA_IN_REGISTER_OFFSET > +#define IPMI_KCS_REG_DATA_OUT IPMI_KCS_BASE_ADDRESS + > IPMI_KCS_DATA_OUT_REGISTER_OFFSET > +#define IPMI_KCS_REG_COMMAND IPMI_KCS_BASE_ADDRESS + > IPMI_KCS_COMMAND_REGISTER_OFFSET > +#define IPMI_KCS_REG_STATUS IPMI_KCS_BASE_ADDRESS + > IPMI_KCS_STATUS_REGISTER_OFFSET > + > +/** > + This functions setup the IPMI transport hardware information > +according > + to the specification of transport token acquired from transport library. > + > + @param[in] TransportToken The transport interface. > + @param[out] HardwareInformation Pointer to receive the hardware > information. > + > + @retval EFI_SUCCESS Hardware information is returned in > HardwareInformation. > + Caller must free the memory allocated for > HardwareInformation > + once it doesn't need it. > + @retval EFI_UNSUPPORTED No hardware information for the > specification specified > + in the transport token. > +**/ > +EFI_STATUS > +SetupIpmiTransportHardwareInformation ( > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, > + OUT MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION > +*HardwareInformation > + ); > + > +/** > + This functions setup the final header/body/trailer packets for > + the acquired transport interface. > + > + @param[in] TransportToken The transport interface. > + @param[in] NetFunction IPMI function. > + @param[in] Command IPMI command. > + @param[out] PacketHeader The pointer to receive header of > request. > + @param[in, out] PacketBody The request body. > + When IN, it is the caller's request > body. > + When OUT and NULL, the request body is > not > + changed. > + Whee out and non-NULL, the request body > is > + changed to comfort the transport > interface. > + @param[in, out] PacketBodySize The request body size. > + When IN and non-zero, it is the new data > + length of request body. > + When IN and zero, the request body is > unchanged. > + @param[out] PacketTrailer The pointer to receive trailer of > request. > + > + @retval EFI_SUCCESS Request packet is returned. > + @retval EFI_UNSUPPORTED Request packet is not returned because > + the unsupported transport interface. > +**/ > +EFI_STATUS > +SetupIpmiRequestTransportPacket ( > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, > + IN UINT8 NetFunction, > + IN UINT8 Command, > + OUT MANAGEABILITY_TRANSPORT_HEADER *PacketHeader, > + IN OUT UINT8 **PacketBody, > + IN OUT UINT32 *PacketBodySize, > + OUT MANAGEABILITY_TRANSPORT_TRAILER *PacketTrailer > + ); > + > +/** > + Common code to submit IPMI commands > + > + @param[in] TransportToken TRansport token. > + @param[in] NetFunction Net function of the command. > + @param[in] Command IPMI Command. > + @param[in] RequestData Command Request Data. > + @param[in] RequestDataSize Size of Command Request Data. > + @param[out] ResponseData Command Response Data. The > completion code is the first byte of response data. > + @param[in, out] ResponseDataSize Size of Command Response Data. > + > + @retval EFI_SUCCESS The command byte stream was successfully > submit to the device and a response was successfully received. > + @retval EFI_NOT_FOUND The command was not successfully sent to > the device or a response was not successfully received from the device. > + @retval EFI_NOT_READY Ipmi Device is not ready for Ipmi command > access. > + @retval EFI_DEVICE_ERROR Ipmi Device hardware error. > + @retval EFI_TIMEOUT The command time out. > + @retval EFI_UNSUPPORTED The command was not successfully sent to > the device. > + @retval EFI_OUT_OF_RESOURCES The resource allocation is out of > resource or data size error. > +**/ > +EFI_STATUS > +CommonIpmiSubmitCommand ( > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, > + IN UINT8 NetFunction, > + IN UINT8 Command, > + IN UINT8 *RequestData, > + IN UINT32 RequestDataSize, > + OUT UINT8 *ResponseData, > + IN OUT UINT32 *ResponseDataSize > + ); > + > +#endif > diff --git > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Common/IpmiProtoc > olCommon.c > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Common/IpmiProtoc > olCommon.c > new file mode 100644 > index 0000000000..8e9dd7bf9d > --- /dev/null > +++ > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Common/IpmiProtoc > +++ olCommon.c > @@ -0,0 +1,247 @@ > +/** @file > + > + IPMI Manageability Protocol common file. > + > + Copyright (C) 2023 Advanced Micro Devices, Inc. All rights > + reserved.<BR> > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > +#include <Uefi.h> > +#include <Library/BaseMemoryLib.h> > +#include <Library/DebugLib.h> > +#include <Library/MemoryAllocationLib.h> #include > +<Library/ManageabilityTransportIpmiLib.h> > +#include <Library/ManageabilityTransportLib.h> > + > +#include "IpmiProtocolCommon.h" > + > +extern CHAR16 *mTransportName; > + > +MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION > mHardwareInformation; > + > +/** > + This functions setup the IPMI transport hardware information > +according > + to the specification of transport token acquired from transport library. > + > + @param[in] TransportToken The transport interface. > + @param[out] HardwareInformation Pointer to receive the hardware > information. > + > + @retval EFI_SUCCESS Hardware information is returned in > HardwareInformation. > + Caller must free the memory allocated for > HardwareInformation > + once it doesn't need it. > + @retval EFI_UNSUPPORTED No hardware information for the > specification specified > + in the transport token. > + #retval EFI_OUT_OF_RESOURCES Not enough memory for > MANAGEABILITY_TRANSPORT_KCS_HARDWARE_INFO. > +**/ > +EFI_STATUS > +SetupIpmiTransportHardwareInformation ( > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, > + OUT MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION > +*HardwareInformation > + ) > +{ > + MANAGEABILITY_TRANSPORT_KCS_HARDWARE_INFO *KcsHardwareInfo; > + > + KcsHardwareInfo = AllocatePool (sizeof > + (MANAGEABILITY_TRANSPORT_KCS_HARDWARE_INFO)); > + if (KcsHardwareInfo == NULL) { > + DEBUG ((DEBUG_ERROR, "%a: Not enough memory for > MANAGEABILITY_TRANSPORT_KCS_HARDWARE_INFO.\n", > __FUNCTION__)); > + return EFI_OUT_OF_RESOURCES; > + } > + > + if (CompareGuid (&gManageabilityTransportKcsGuid, TransportToken- > >Transport->ManageabilityTransportSpecification)) { > + // This is KCS transport interface. > + KcsHardwareInfo->MemoryMap = > MANAGEABILITY_TRANSPORT_KCS_IO_MAP_IO; > + KcsHardwareInfo->IoBaseAddress.IoAddress16 = > IPMI_KCS_BASE_ADDRESS; > + KcsHardwareInfo->IoDataInAddress.IoAddress16 = > IPMI_KCS_REG_DATA_IN; > + KcsHardwareInfo->IoDataOutAddress.IoAddress16 = > IPMI_KCS_REG_DATA_OUT; > + KcsHardwareInfo->IoCommandAddress.IoAddress16 = > IPMI_KCS_REG_COMMAND; > + KcsHardwareInfo->IoStatusAddress.IoAddress16 = > IPMI_KCS_REG_STATUS; > + *HardwareInformation = > + > (MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION)KcsHardwareIn > fo; > > IpmiProtocolCommon.c will be included by IpmiPpiPei driver. Please consider > the case when memory is not ready in PEI. In this case, global variable is not > writeable. This is my mistake Nickle, I also found this problem and I am fixing it now. > > > + return EFI_SUCCESS; > + } else { > + DEBUG ((DEBUG_ERROR, "%a: No implementation of setting hardware > information.", __FUNCTION__)); > + ASSERT (FALSE); > + } > + > + return EFI_UNSUPPORTED; > +} > + > +/** > + This functions setup the final header/body/trailer packets for > + the acquired transport interface. > + > + @param[in] TransportToken The transport interface. > + @param[in] NetFunction IPMI function. > + @param[in] Command IPMI command. > + @param[out] PacketHeader The pointer to receive header of > request. > + @param[in, out] PacketBody The request body. > + When IN, it is the caller's request > body. > + When OUT and NULL, the request body is > not > + changed. > + When OUT and non-NULL, the request body > is > + changed to conform the transport > interface. > + @param[in, out] PacketBodySize The request body size. > + When OUT and non-zero, it is the new > data > + length of request body. > + When OUT and zero, the request body is > unchanged. > + @param[out] PacketTrailer The pointer to receive trailer of > request. > + > + @retval EFI_SUCCESS Request packet is returned. > + @retval EFI_UNSUPPORTED Request packet is not returned because > + the unsupported transport interface. > +**/ > +EFI_STATUS > +SetupIpmiRequestTransportPacket ( > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, > + IN UINT8 NetFunction, > + IN UINT8 Command, > + OUT MANAGEABILITY_TRANSPORT_HEADER *PacketHeader, > + IN OUT UINT8 **PacketBody, > + IN OUT UINT32 *PacketBodySize, > + OUT MANAGEABILITY_TRANSPORT_TRAILER *PacketTrailer > + ) > +{ > + MANAGEABILITY_IPMI_TRANSPORT_HEADER *IpmiHeader; > > It would be good if we can check input pointer before we dereference them. I will add OPTIONAL to some parameters and check if that is NULL in the code. > > + > + if (CompareGuid (&gManageabilityTransportKcsGuid, TransportToken- > >Transport->ManageabilityTransportSpecification)) { > + // This is KCS transport interface. > + IpmiHeader = AllocateZeroPool (sizeof > (MANAGEABILITY_IPMI_TRANSPORT_HEADER)); > + if (IpmiHeader == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + IpmiHeader->Command = Command; > + IpmiHeader->Lun = 0; > + IpmiHeader->NetFn = NetFunction; > + *PacketHeader = (MANAGEABILITY_TRANSPORT_HEADER > *)IpmiHeader; > + *PacketTrailer = NULL; > + *PacketBody = NULL; > + *PacketBodySize = 0; > + } else { > + DEBUG ((DEBUG_ERROR, "%a: No implementation of building up packet.", > __FUNCTION__)); > + ASSERT (FALSE); > + } > + > + return EFI_SUCCESS; > +} > + > +/** > + Common code to submit IPMI commands > + > + @param[in] TransportToken TRansport token. > + @param[in] NetFunction Net function of the command. > + @param[in] Command IPMI Command. > + @param[in] RequestData Command Request Data. > + @param[in] RequestDataSize Size of Command Request Data. > + @param[out] ResponseData Command Response Data. The > completion code is the first byte of response data. > + @param[in, out] ResponseDataSize Size of Command Response Data. > + > + @retval EFI_SUCCESS The command byte stream was successfully > submit to the device and a response was successfully received. > + @retval EFI_NOT_FOUND The command was not successfully sent to > the device or a response was not successfully received from the device. > + @retval EFI_NOT_READY Ipmi Device is not ready for Ipmi command > access. > + @retval EFI_DEVICE_ERROR Ipmi Device hardware error. > + @retval EFI_TIMEOUT The command time out. > + @retval EFI_UNSUPPORTED The command was not successfully sent to > the device. > + @retval EFI_OUT_OF_RESOURCES The resource allocation is out of > resource or data size error. > +**/ > +EFI_STATUS > +CommonIpmiSubmitCommand ( > + IN MANAGEABILITY_TRANSPORT_TOKEN *TransportToken, > + IN UINT8 NetFunction, > + IN UINT8 Command, > + IN UINT8 *RequestData, > + IN UINT32 RequestDataSize, > + OUT UINT8 *ResponseData, > + IN OUT UINT32 *ResponseDataSize > + ) > +{ > + EFI_STATUS Status; > + UINT8 *ThisRequestData; > + UINT32 ThisRequestDataSize; > + MANAGEABILITY_TRANSFER_TOKEN TransferToken; > + MANAGEABILITY_TRANSPORT_HEADER IpmiTransportHeader; > + MANAGEABILITY_TRANSPORT_TRAILER IpmiTransportTrailer; > + MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS > TransportAdditionalStatus; > + > + if (TransportToken == NULL) { > + DEBUG ((DEBUG_ERROR, "%a: No transport toke for IPMI\n", > __FUNCTION__)); > + return EFI_UNSUPPORTED; > + } > > I think we need same check to other input pointers. If they can be NULL, we > can add "OPTIONAL" to function header. Sure, > > + > + Status = TransportToken->Transport->Function.Version1_0- > >TransportStatus ( > + TransportToken, > + > &TransportAdditionalStatus > + ); if > + (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Transport %s for IPMI has problem - > (%r)\n", __FUNCTION__, mTransportName, Status)); > + return Status; > + } > + > + ThisRequestData = RequestData; > + ThisRequestDataSize = RequestDataSize; > + IpmiTransportHeader = NULL; > + IpmiTransportTrailer = NULL; > + Status = SetupIpmiRequestTransportPacket ( > + TransportToken, > + NetFunction, > + Command, > + &IpmiTransportHeader, > + &ThisRequestData, > + &ThisRequestDataSize, > + &IpmiTransportTrailer > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Fail to build packets - (%r)\n", > __FUNCTION__, Status)); > + return Status; > + } > + > + ZeroMem (&TransferToken, sizeof (MANAGEABILITY_TRANSFER_TOKEN)); > + TransferToken.TransmitHeader = IpmiTransportHeader; > + TransferToken.TransmitTrailer = IpmiTransportTrailer; > + > + // Transmit packet. > + if ((ThisRequestData == NULL) || (ThisRequestDataSize == 0)) { > + > + // Transmit parameter were not changed by > SetupIpmiRequestTransportPacket(). > + TransferToken.TransmitPackage.TransmitPayload = RequestData; > + TransferToken.TransmitPackage.TransmitSizeInByte = RequestDataSize; > + } else { > + TransferToken.TransmitPackage.TransmitPayload = ThisRequestData; > + TransferToken.TransmitPackage.TransmitSizeInByte = > + ThisRequestDataSize; } > + > + TransferToken.TransmitPackage.TransmitTimeoutInMillisecond = > + MANAGEABILITY_TRANSPORT_NO_TIMEOUT; > + > + // Receive packet. > + TransferToken.ReceivePackage.ReceiveBuffer = ResponseData; > + TransferToken.ReceivePackage.ReceiveSizeInByte = > *ResponseDataSize; > + TransferToken.ReceivePackage.TransmitTimeoutInMillisecond = > + MANAGEABILITY_TRANSPORT_NO_TIMEOUT; > + TransportToken->Transport->Function.Version1_0- > >TransportTransmitReceive ( > + TransportToken, > + &TransferToken > + ); > + > + if (IpmiTransportHeader != NULL) { > + FreePool ((VOID *)IpmiTransportHeader); } > + > + if (IpmiTransportTrailer != NULL) { > + FreePool ((VOID *)IpmiTransportTrailer); } > + > + if (ThisRequestData != NULL) { > + FreePool ((VOID *)ThisRequestData); } > + > + // Return transfer status. > + // > + Status = TransferToken.TransferStatus; > + TransportAdditionalStatus = TransferToken.TransportAdditionalStatus; > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to send IPMI command over %s\n", > __FUNCTION__, mTransportName)); > + return Status; > + } > + > + *ResponseDataSize = TransferToken.ReceivePackage.ReceiveSizeInByte; > + return Status; > +} > diff --git > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol.c > new file mode 100644 > index 0000000000..6d3720581c > --- /dev/null > +++ b/Features/ManageabilityPkg/Universal/IpmiProtocol/Dxe/IpmiProtocol. > +++ c > @@ -0,0 +1,177 @@ > +/** @file > + This file provides IPMI Protocol implementation. > + > + Copyright (C) 2023 Advanced Micro Devices, Inc. All rights > +reserved.<BR> > + SPDX-License-Identifier: BSD-2-Clause-Patent **/ > + > +#include <PiDxe.h> > +#include <Library/DebugLib.h> > +#include <Library/BaseMemoryLib.h> > +#include <Library/MemoryAllocationLib.h> #include > +<Library/ManageabilityTransportLib.h> > +#include <Library/ManageabilityTransportIpmiLib.h> > +#include <Library/ManageabilityTransportHelperLib.h> > +#include <Library/UefiBootServicesTableLib.h> > +#include <Protocol/IpmiProtocol.h> > + > +#include "IpmiProtocolCommon.h" > + > +MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL; > +CHAR16 *mTransportName; > + > +extern MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION > +mHardwareInformation; > + > +/** > + This service enables submitting commands via Ipmi. > + > + @param[in] This This point for IPMI_PROTOCOL > structure. > + @param[in] NetFunction Net function of the command. > + @param[in] Command IPMI Command. > + @param[in] RequestData Command Request Data. > + @param[in] RequestDataSize Size of Command Request Data. > + @param[out] ResponseData Command Response Data. The > completion code is the first byte of response data. > + @param[in, out] ResponseDataSize Size of Command Response Data. > + > + @retval EFI_SUCCESS The command byte stream was successfully > submit to the device and a response was successfully received. > + @retval EFI_NOT_FOUND The command was not successfully sent to > the device or a response was not successfully received from the device. > + @retval EFI_NOT_READY Ipmi Device is not ready for Ipmi command > access. > + @retval EFI_DEVICE_ERROR Ipmi Device hardware error. > + @retval EFI_TIMEOUT The command time out. > + @retval EFI_UNSUPPORTED The command was not successfully sent to > the device. > + @retval EFI_OUT_OF_RESOURCES The resource allocation is out of > resource or data size error. > +**/ > +EFI_STATUS > +EFIAPI > +DxeIpmiSubmitCommand ( > + IN IPMI_PROTOCOL *This, > + IN UINT8 NetFunction, > + IN UINT8 Command, > + IN UINT8 *RequestData, > + IN UINT32 RequestDataSize, > + OUT UINT8 *ResponseData, > + IN OUT UINT32 *ResponseDataSize > + ) > +{ > + EFI_STATUS Status; > + > + Status = CommonIpmiSubmitCommand ( > + mTransportToken, > + NetFunction, > + Command, > + RequestData, > + RequestDataSize, > + ResponseData, > + ResponseDataSize > + ); > + return Status; > +} > + > +static IPMI_PROTOCOL mIpmiProtocol = { > + DxeIpmiSubmitCommand > +}; > + > +/** > + The entry point of the Ipmi DXE driver. > + > + @param[in] ImageHandle - Handle of this driver image @param[in] > + SystemTable - Table containing standard EFI services > + > + @retval EFI_SUCCESS - IPMI Protocol is installed successfully. > + @retval Otherwise - Other errors. > +**/ > +EFI_STATUS > +EFIAPI > +DxeIpmiEntry ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + EFI_HANDLE Handle; > + MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability; > + MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS > TransportAdditionalStatus; > + > + GetTransportCapability (&TransportCapability); > + > + Status = HelperAcquireManageabilityTransport ( > + &gManageabilityProtocolIpmiGuid, > + &mTransportToken > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to acquire transport interface for > IPMI protocol - %r\n", __FUNCTION__, Status)); > + return Status; > + } > + > + mTransportName = HelperManageabilitySpecName > + (mTransportToken->Transport->ManageabilityTransportSpecification); > + DEBUG ((DEBUG_ERROR, "%a: IPMI protocol over %s.\n", __FUNCTION__, > + mTransportName)); > + > + // > + // Setup hardware information according to the transport interface. > + Status = SetupIpmiTransportHardwareInformation ( > + mTransportToken, > + &mHardwareInformation > + ); > + if (EFI_ERROR (Status)) { > + if (Status == EFI_UNSUPPORTED) { > + DEBUG ((DEBUG_ERROR, "%a: No hardware information of %s transport > interface.\n", __FUNCTION__, mTransportName)); > + } > + > + return Status; > + } > + > + // > + // Initial transport interface with the hardware information assigned. > + Status = HelperInitManageabilityTransport ( > + mTransportToken, > + mHardwareInformation, > + &TransportAdditionalStatus > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Handle = NULL; > + Status = gBS->InstallProtocolInterface ( > + &Handle, > + &gIpmiProtocolGuid, > + EFI_NATIVE_INTERFACE, > + (VOID **)&mIpmiProtocol > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to install IPMI protocol - %r\n", > + __FUNCTION__, Status)); } > + > + return Status; > +} > + > +/** > + This is the unload handler for IPMI protocol module. > + > + Release the MANAGEABILITY_TRANSPORT_TOKEN acquired at entry point. > + > + @param[in] ImageHandle The drivers' driver image. > + > + @retval EFI_SUCCESS The image is unloaded. > + @retval Others Failed to unload the image. > + > +**/ > +EFI_STATUS > +EFIAPI > +IpmiUnloadImage ( > + IN EFI_HANDLE ImageHandle > + ) > +{ > + EFI_STATUS Status; > + > + Status = EFI_SUCCESS; > + if (mTransportToken != NULL) { > + Status = ReleaseTransportSession (mTransportToken); } > + > + if (mHardwareInformation.Pointer != NULL) { > + FreePool (mHardwareInformation.Pointer); } > + > + return Status; > +} > diff --git a/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c > new file mode 100644 > index 0000000000..f31dbc9f4d > --- /dev/null > +++ b/Features/ManageabilityPkg/Universal/IpmiProtocol/Pei/IpmiPpi.c > @@ -0,0 +1,151 @@ > +/** @file > + This file provides IPMI PPI implementation. > + > + Copyright (C) 2023 Advanced Micro Devices, Inc. All rights > + reserved.<BR> > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include <PiPei.h> > +#include <Library/DebugLib.h> > +#include <Library/BaseMemoryLib.h> > +#include <Library/ManageabilityTransportLib.h> > +#include <Library/ManageabilityTransportIpmiLib.h> > +#include <Library/ManageabilityTransportHelperLib.h> > +#include <Library/PeiServicesLib.h> > + > +#include <Ppi/IpmiPpi.h> > + > +#include "IpmiProtocolCommon.h" > + > +MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL; > +CHAR16 *mTransportName; > + > +extern MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION > +mHardwareInformation; > + > +/** > + This service enables submitting commands via Ipmi. > + > + @param[in] This This point for PEI_IPMI_PPI structure. > + @param[in] NetFunction Net function of the command. > + @param[in] Command IPMI Command. > + @param[in] RequestData Command Request Data. > + @param[in] RequestDataSize Size of Command Request Data. > + @param[out] ResponseData Command Response Data. The > completion code is the first byte of response data. > + @param[in, out] ResponseDataSize Size of Command Response Data. > + > + @retval EFI_SUCCESS The command byte stream was successfully > submit to the device and a response was successfully received. > + @retval EFI_NOT_FOUND The command was not successfully sent to > the device or a response was not successfully received from the device. > + @retval EFI_NOT_READY Ipmi Device is not ready for Ipmi command > access. > + @retval EFI_DEVICE_ERROR Ipmi Device hardware error. > + @retval EFI_TIMEOUT The command time out. > + @retval EFI_UNSUPPORTED The command was not successfully sent to > the device. > + @retval EFI_OUT_OF_RESOURCES The resource allocation is out of > resource or data size error. > +**/ > +EFI_STATUS > +EFIAPI > +PeiIpmiSubmitCommand ( > + IN PEI_IPMI_PPI *This, > + IN UINT8 NetFunction, > + IN UINT8 Command, > + IN UINT8 *RequestData, > + IN UINT32 RequestDataSize, > + OUT UINT8 *ResponseData, > + IN OUT UINT32 *ResponseDataSize > + ) > +{ > + EFI_STATUS Status; > + > + Status = CommonIpmiSubmitCommand ( > + mTransportToken, > + NetFunction, > + Command, > + RequestData, > + RequestDataSize, > + ResponseData, > + ResponseDataSize > + ); > + return Status; > +} > + > +static PEI_IPMI_PPI mPeiIpmiPpi = { > + PeiIpmiSubmitCommand > +}; > + > +static EFI_PEI_PPI_DESCRIPTOR mIpmiPpiList[] = { > + { > + (EFI_PEI_PPI_DESCRIPTOR_PPI | > EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST), > + &gPeiIpmiPpiGuid, > + &mPeiIpmiPpi > + } > +}; > + > +/** > + The entry point of the Ipmi PPI PEIM. > + > + @param FileHandle Handle of the file being invoked. > + @param PeiServices Describes the list of possible PEI Services. > + > + @retval EFI_SUCCESS Indicates that Ipmi initialization completed > successfully. > + @retval Others Indicates that Ipmi initialization could not complete > successfully. > +**/ > +EFI_STATUS > +EFIAPI > +PeiIpmiEntry ( > + IN EFI_PEI_FILE_HANDLE FileHandle, > + IN CONST EFI_PEI_SERVICES **PeiServices > + ) > +{ > + EFI_STATUS Status; > + MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability; > + MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS > TransportAdditionalStatus; > + > + GetTransportCapability (&TransportCapability); > + > + Status = HelperAcquireManageabilityTransport ( > + &gManageabilityProtocolIpmiGuid, > + &mTransportToken > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to acquire transport interface for > IPMI protocol - %r\n", __FUNCTION__, Status)); > + return Status; > + } > + > + mTransportName = HelperManageabilitySpecName > + (mTransportToken->Transport->ManageabilityTransportSpecification); > + DEBUG ((DEBUG_ERROR, "%a: IPMI protocol over %s.\n", __FUNCTION__, > + mTransportName)); > > This is DEBUG_INFO, right? I think it is not an error message. Yes, and I will also fix the same issues on DXE/SMM file. > > + > + // > + // Setup hardware information according to the transport interface. > + Status = SetupIpmiTransportHardwareInformation ( > + mTransportToken, > + &mHardwareInformation > + ); > + if (EFI_ERROR (Status)) { > + if (Status == EFI_UNSUPPORTED) { > + DEBUG ((DEBUG_ERROR, "%a: No hardware information of %s transport > interface.\n", __FUNCTION__, mTransportName)); > + } > + > + return Status; > + } > + > + // > + // Initial transport interface with the hardware information assigned. > + Status = HelperInitManageabilityTransport ( > + mTransportToken, > + mHardwareInformation, > + &TransportAdditionalStatus > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + // > + // Install IPMI PPI. > + // > + Status = PeiServicesInstallPpi (&mIpmiPpiList[0]); if (EFI_ERROR > + (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to install IPMI PPI - %r\n", > + __FUNCTION__, Status)); } > + > + return Status; > +} > diff --git > a/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol.c > new file mode 100644 > index 0000000000..227f6f7114 > --- /dev/null > +++ > b/Features/ManageabilityPkg/Universal/IpmiProtocol/Smm/IpmiProtocol. > +++ c > @@ -0,0 +1,147 @@ > +/** @file > + This file provides IPMI SMM Protocol implementation. > + > + Copyright (C) 2023 Advanced Micro Devices, Inc. All rights > +reserved.<BR> > + SPDX-License-Identifier: BSD-2-Clause-Patent **/ > + > +#include <PiDxe.h> > +#include <Library/DebugLib.h> > +#include <Library/BaseMemoryLib.h> > +#include <Library/ManageabilityTransportLib.h> > +#include <Library/ManageabilityTransportIpmiLib.h> > +#include <Library/ManageabilityTransportHelperLib.h> > +#include <Library/SmmServicesTableLib.h> #include > +<Library/UefiBootServicesTableLib.h> > + > +#include <Protocol/IpmiProtocol.h> > + > +#include "IpmiProtocolCommon.h" > + > +MANAGEABILITY_TRANSPORT_TOKEN *mTransportToken = NULL; > +CHAR16 *mTransportName; > + > +extern MANAGEABILITY_TRANSPORT_HARDWARE_INFORMATION > +mHardwareInformation; > + > +/** > + This service enables submitting commands via Ipmi. > + > + @param[in] This This point for IPMI_PROTOCOL > structure. > + @param[in] NetFunction Net function of the command. > + @param[in] Command IPMI Command. > + @param[in] RequestData Command Request Data. > + @param[in] RequestDataSize Size of Command Request Data. > + @param[out] ResponseData Command Response Data. The > completion code is the first byte of response data. > + @param[in, out] ResponseDataSize Size of Command Response Data. > + > + @retval EFI_SUCCESS The command byte stream was successfully > submit to the device and a response was successfully received. > + @retval EFI_NOT_FOUND The command was not successfully sent to > the device or a response was not successfully received from the device. > + @retval EFI_NOT_READY Ipmi Device is not ready for Ipmi command > access. > + @retval EFI_DEVICE_ERROR Ipmi Device hardware error. > + @retval EFI_TIMEOUT The command time out. > + @retval EFI_UNSUPPORTED The command was not successfully sent to > the device. > + @retval EFI_OUT_OF_RESOURCES The resource allocation is out of > resource or data size error. > +**/ > +EFI_STATUS > +EFIAPI > +SmmIpmiSubmitCommand ( > + IN IPMI_PROTOCOL *This, > + IN UINT8 NetFunction, > + IN UINT8 Command, > + IN UINT8 *RequestData, > + IN UINT32 RequestDataSize, > + OUT UINT8 *ResponseData, > + IN OUT UINT32 *ResponseDataSize > + ) > +{ > + EFI_STATUS Status; > + > + Status = CommonIpmiSubmitCommand ( > + mTransportToken, > + NetFunction, > + Command, > + RequestData, > + RequestDataSize, > + ResponseData, > + ResponseDataSize > + ); > + return Status; > +} > + > +static IPMI_PROTOCOL mIpmiProtocol = { > + SmmIpmiSubmitCommand > +}; > + > +/** > + The entry point of the Ipmi DXE driver. > + > + @param[in] ImageHandle - Handle of this driver image @param[in] > + SystemTable - Table containing standard EFI services > + > + @retval EFI_SUCCESS - IPMI Protocol is installed successfully. > + @retval Otherwise - Other errors. > +**/ > +EFI_STATUS > +EFIAPI > +SmmIpmiEntry ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_STATUS Status; > + EFI_HANDLE Handle; > + MANAGEABILITY_TRANSPORT_CAPABILITY TransportCapability; > + MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS > TransportAdditionalStatus; > + > + GetTransportCapability (&TransportCapability); > + > + Status = HelperAcquireManageabilityTransport ( > + &gManageabilityProtocolIpmiGuid, > + &mTransportToken > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to acquire transport interface for > IPMI protocol - %r\n", __FUNCTION__, Status)); > + return Status; > + } > + > + mTransportName = HelperManageabilitySpecName > + (mTransportToken->Transport->ManageabilityTransportSpecification); > + DEBUG ((DEBUG_ERROR, "%a: IPMI protocol over %s.\n", __FUNCTION__, > + mTransportName)); > > Please use DEBUG_INFO. > > + > + // > + // Setup hardware information according to the transport interface. > + Status = SetupIpmiTransportHardwareInformation ( > + mTransportToken, > + &mHardwareInformation > + ); > + if (EFI_ERROR (Status)) { > + if (Status == EFI_UNSUPPORTED) { > + DEBUG ((DEBUG_ERROR, "%a: No hardware information of %s transport > interface.\n", __FUNCTION__, mTransportName)); > + } > + > + return Status; > + } > + > + // > + // Initial transport interface with the hardware information assigned. > + Status = HelperInitManageabilityTransport ( > + mTransportToken, > + mHardwareInformation, > + &TransportAdditionalStatus > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + Handle = NULL; > + Status = gSmst->SmmInstallProtocolInterface ( > + &Handle, > + &gSmmIpmiProtocolGuid, > + EFI_NATIVE_INTERFACE, > + (VOID **)&mIpmiProtocol > + ); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "%a: Failed to install IPMI SMM protocol - > + %r\n", __FUNCTION__, Status)); } > + > + return Status; > +} > -- > 2.37.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#101464): https://edk2.groups.io/g/devel/message/101464 Mute This Topic: https://groups.io/mt/97473058/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-