From: "Chang, Abner via groups.io" <abner.chang=amd.com@groups.io>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"lisa.huang@intel.com" <lisa.huang@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>,
Nate DeSimone <nathaniel.l.desimone@intel.com>
Subject: Re: [edk2-devel] [PATCH v1 1/1] IpmiFeaturePkg/GenericIpmi: Support Standalone MM
Date: Sat, 21 Oct 2023 14:15:26 +0000 [thread overview]
Message-ID: <MN2PR12MB396681EB6F3EE01F466A1D67EADAA@MN2PR12MB3966.namprd12.prod.outlook.com> (raw)
In-Reply-To: <20231020064616.6637-1-lisa.huang@intel.com>
[AMD Official Use Only - General]
Hi Lisa,
Issac was no longer the maintainer of IpmiFeaturePkg as he was retired. Nate had sent the maintainers update for review.
As the conversation I had with Issac, we all agreed IpmiFeaturePkg should be deprecated as the IPMI related drivers are now located under ManageabilityPkg in edk2-platforms. The IPMI implementation in these two packages are diverging now, which is not good.
Could you please send this patch against ManageabilityPkg? I can help to review it, so does Nate as he also proposed himself as the maintainers of ManageabilityPkg.
Thanks
Abner
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Huang,
> Li-Xia via groups.io
> Sent: Friday, October 20, 2023 2:46 PM
> To: devel@edk2.groups.io
> Cc: Isaac Oram <isaac.w.oram@intel.com>; Nate DeSimone
> <nathaniel.l.desimone@intel.com>
> Subject: [edk2-devel] [PATCH v1 1/1] IpmiFeaturePkg/GenericIpmi: Support
> Standalone MM
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Add Standalone Mm Generic Impi driver. And add type 'PcdsFixedAtBuild'
> for PcdIpmiSmmIoBaseAddress to access in StandaloneMm driver
>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Signed-off-by: Lixia Huang <lisa.huang@intel.com>
> ---
>
> Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Standa
> loneMm/StandaloneMmGenericIpmi.c | 148 ++++++++++++++++++++
>
> Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Standa
> loneMm/StandaloneMmGenericIpmi.inf | 52 +++++++
>
> Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
> | 2 +
> 3 files changed, 202 insertions(+)
>
> diff --git
> a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Stan
> daloneMm/StandaloneMmGenericIpmi.c
> b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Stan
> daloneMm/StandaloneMmGenericIpmi.c
> new file mode 100644
> index 000000000000..52d8d2abdd0d
> --- /dev/null
> +++
> b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Stan
> daloneMm/StandaloneMmGenericIpmi.c
> @@ -0,0 +1,148 @@
> +/** @file
>
> + Generic StandaloneMm IPMI stack driver
>
> +
>
> + @copyright
>
> + Copyright 2023 Intel Corporation. <BR>
>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +**/
>
> +
>
> +//
>
> +// Statements that include other files
>
> +//
>
> +#include <IndustryStandard/Ipmi.h>
>
> +#include <Uefi.h>
>
> +#include <Library/BaseLib.h>
>
> +#include <Library/SmmLib.h>
>
> +#include <Library/DebugLib.h>
>
> +#include <Library/BaseMemoryLib.h>
>
> +#include <Library/MmServicesTableLib.h>
>
> +#include <Library/MemoryAllocationLib.h>
>
> +#include <Library/UefiBootServicesTableLib.h>
>
> +#include <Library/IpmiBaseLib.h>
>
> +#include <SmStatusCodes.h>
>
> +#include "IpmiHooks.h"
>
> +#include "IpmiBmcCommon.h"
>
> +#include "IpmiBmc.h"
>
> +#include <Library/TimerLib.h>
>
> +
>
> +IPMI_BMC_INSTANCE_DATA *mIpmiInstance;
>
> +EFI_HANDLE mHandle;
>
> +
>
> +EFI_STATUS
>
> +SmmInitializeIpmiKcsPhysicalLayer (
>
> + VOID
>
> + )
>
> +/*++
>
> +
>
> +Routine Description:
>
> + Setup and initialize the BMC for the SMM phase. In order to verify the BMC
> is functioning
>
> + as expected, the BMC Selftest is performed. The results are then checked
> and any errors are
>
> + reported to the error manager. Errors are collected throughout this routine
> and reported
>
> + just prior to installing the driver. If there are more errors than
> MAX_SOFT_COUNT, then they
>
> + will be ignored.
>
> +
>
> +Arguments:
>
> + ImageHandle - Handle of this driver image
>
> + SystemTable - Table containing standard EFI services
>
> +
>
> +Returns:
>
> + EFI_SUCCESS - Successful driver initialization
>
> +
>
> +--*/
>
> +{
>
> + EFI_STATUS Status;
>
> +
>
> + DEBUG ((DEBUG_INFO,"SmmInitializeIpmiKcsPhysicalLayer entry \n"));
>
> +
>
> + Status = gMmst->MmAllocatePool (
>
> + EfiRuntimeServicesData,
>
> + sizeof (IPMI_BMC_INSTANCE_DATA),
>
> + (VOID **)&mIpmiInstance);
>
> +
>
> + if (EFI_ERROR (Status)) {
>
> + DEBUG ((DEBUG_ERROR, "mIpmiInstance mem alloc failed - 0x%x\n",
> Status));
>
> + return Status;
>
> + } else {
>
> +
>
> + //
>
> + // Initialize the KCS transaction timeout. Assume delay unit is 1000 us.
>
> + //
>
> + mIpmiInstance->KcsTimeoutPeriod = (BMC_KCS_TIMEOUT * 1000*1000) /
> KCS_DELAY_UNIT;
>
> +
>
> + //
>
> + // Initialize IPMI IO Base, we still use SMS IO base to get device ID and
> Seltest result since SMM IF may have different cmds supported
>
> + //
>
> + mIpmiInstance->IpmiIoBase = FixedPcdGet16
> (PcdIpmiSmmIoBaseAddress);
>
> + mIpmiInstance->Signature = SM_IPMI_BMC_SIGNATURE;
>
> + mIpmiInstance->SlaveAddress = BMC_SLAVE_ADDRESS;
>
> + mIpmiInstance->BmcStatus = BMC_NOTREADY;
>
> + mIpmiInstance->IpmiTransport.IpmiSubmitCommand =
> IpmiSendCommand;
>
> + mIpmiInstance->IpmiTransport.GetBmcStatus = IpmiGetBmcStatus;
>
> +
>
> + mHandle = NULL;
>
> + Status = gMmst->MmInstallProtocolInterface (
>
> + &mHandle,
>
> + &gSmmIpmiTransportProtocolGuid,
>
> + EFI_NATIVE_INTERFACE,
>
> + &mIpmiInstance->IpmiTransport
>
> + );
>
> + ASSERT_EFI_ERROR (Status);
>
> +
>
> + DEBUG ((DEBUG_INFO,"SmmInitializeIpmiKcsPhysicalLayer exit \n"));
>
> +
>
> + return EFI_SUCCESS;
>
> + }
>
> +}
>
> +
>
> +/**
>
> + The module Entry Point of driver.
>
> +
>
> + @param ImageHandle The firmware allocated handle for the EFI image.
>
> + @param SystemTable A pointer to the MM System Table.
>
> +
>
> + @retval EFI_SUCCESS The entry point is executed successfully.
>
> + @retval Other Some error occurs when executing this entry point.
>
> +
>
> +**/
>
> +EFI_STATUS
>
> +InitializeGenericIpmiStandaloneMm (
>
> + IN EFI_HANDLE ImageHandle,
>
> + IN EFI_MM_SYSTEM_TABLE *SystemTable
>
> + )
>
> +{
>
> + SmmInitializeIpmiKcsPhysicalLayer ();
>
> + return EFI_SUCCESS;
>
> +}
>
> +
>
> +/**
>
> + Unloads an image.
>
> +
>
> + @param[in] ImageHandle Handle that identifies the image to be
> unloaded.
>
> +
>
> + @retval EFI_SUCCESS The image has been unloaded.
>
> + @retval EFI_INVALID_PARAMETER ImageHandle is not a valid image handle.
>
> +
>
> +**/
>
> +EFI_STATUS
>
> +EFIAPI
>
> +GenericIpmiStandaloneMmUnload (
>
> + IN EFI_HANDLE ImageHandle
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> +
>
> + Status = EFI_SUCCESS;
>
> + if (mIpmiInstance != NULL) {
>
> + if (mHandle != NULL) {
>
> + Status = gMmst->MmUninstallProtocolInterface (
>
> + mHandle,
>
> + &gSmmIpmiTransportProtocolGuid,
>
> + &mIpmiInstance->IpmiTransport
>
> + );
>
> + ASSERT_EFI_ERROR (Status);
>
> + }
>
> + gMmst->MmFreePool (mIpmiInstance);
>
> + }
>
> +
>
> + return Status;
>
> +}
> \ No newline at end of file
> diff --git
> a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Stan
> daloneMm/StandaloneMmGenericIpmi.inf
> b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Stan
> daloneMm/StandaloneMmGenericIpmi.inf
> new file mode 100644
> index 000000000000..336e28368ed6
> --- /dev/null
> +++
> b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Stan
> daloneMm/StandaloneMmGenericIpmi.inf
> @@ -0,0 +1,52 @@
> +## @file
>
> +# StandaloneMm Generic IPMI SMM Driver.
>
> +#
>
> +# @copyright
>
> +# Copyright 2023 Intel Corporation. <BR>
>
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
>
> +##
>
> +
>
> +[Defines]
>
> + INF_VERSION = 0x00010017
>
> + BASE_NAME = StandaloneMmGenericIpmi
>
> + FILE_GUID = 6899c3ea-0920-414f-9555-ad2f00f81060
>
> + MODULE_TYPE = MM_STANDALONE
>
> + VERSION_STRING = 1.0
>
> + PI_SPECIFICATION_VERSION = 0x00010032
>
> + ENTRY_POINT = InitializeGenericIpmiStandaloneMm
>
> + UNLOAD_IMAGE = GenericIpmiStandaloneMmUnload
>
> +
>
> +[Sources]
>
> + ../Common/IpmiHooks.h
>
> + ../Common/IpmiHooks.c
>
> + ../Common/IpmiBmcCommon.h
>
> + ../Common/KcsBmc.c
>
> + ../Common/KcsBmc.h
>
> + ../Common/IpmiBmc.c
>
> + ../Common/IpmiBmc.h
>
> + StandaloneMmGenericIpmi.c
>
> +
>
> +[Packages]
>
> + MdePkg/MdePkg.dec
>
> + IpmiFeaturePkg/IpmiFeaturePkg.dec
>
> +
>
> +[LibraryClasses]
>
> + MemoryAllocationLib
>
> + BaseLib
>
> + MmServicesTableLib
>
> + DebugLib
>
> + StandaloneMmDriverEntryPoint
>
> + IoLib
>
> + ReportStatusCodeLib
>
> + TimerLib
>
> +
>
> +[Protocols]
>
> + gSmmIpmiTransportProtocolGuid # PROTOCOL
> ALWAYS_PRODUCED
>
> +
>
> +[Guids]
>
> +
>
> +[Pcd]
>
> + gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiSmmIoBaseAddress
>
> +
>
> +[Depex]
>
> + gIpmiTransportProtocolGuid
>
> diff --git
> a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.d
> ec
> b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.d
> ec
> index 5df71300cbd1..73dca30caae9 100644
> ---
> a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.d
> ec
> +++
> b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.d
> ec
> @@ -129,4 +129,6 @@
>
> gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiIoBaseAddress|0xCA2|UINT16|0x
> D0000003
>
>
> gIpmiFeaturePkgTokenSpaceGuid.PcdSioMailboxBaseAddress|0x600|UINT32
> |0xD0000004
>
>
> gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiBmcReadyDelayTimer|120|UINT8|
> 0xD0000005
>
> +
>
> +[PcdsFixedAtBuild, PcdsDynamic, PcdsDynamicEx]
>
>
> gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiSmmIoBaseAddress|0xCA2|UINT1
> 6|0xD0000006
>
> --
> 2.26.2.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#109823):
> https://edk2.groups.io/g/devel/message/109823
> Mute This Topic: https://groups.io/mt/102076516/7039027
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [abner.chang@amd.com]
> -=-=-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#109874): https://edk2.groups.io/g/devel/message/109874
Mute This Topic: https://groups.io/mt/102076516/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
next prev parent reply other threads:[~2023-10-21 14:15 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-20 6:46 [edk2-devel] [PATCH v1 1/1] IpmiFeaturePkg/GenericIpmi: Support Standalone MM Huang, Li-Xia
2023-10-21 14:15 ` Chang, Abner via groups.io [this message]
2023-10-23 5:48 ` Huang, Li-Xia
2023-10-24 5:07 ` Chang, Abner via groups.io
2023-10-24 10:43 ` Chang, Abner via groups.io
2023-10-26 5:49 ` Huang, Li-Xia
2023-10-26 6:17 ` Chang, Abner via groups.io
2023-10-24 5:23 ` Chang, Abner via groups.io
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MN2PR12MB396681EB6F3EE01F466A1D67EADAA@MN2PR12MB3966.namprd12.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox