[edk2-devel] [PATCH v1 1/1] IpmiFeaturePkg/GenericIpmi: Support Standalone MM
Huang, Li-Xia
lisa.huang at intel.com
Thu Oct 26 05:49:55 UTC 2023
Hi Abner,
FAST_VIDEO_SUPPORT exists in GenericIPMI Dxe driver, my patch focuses on StandaloneMm support, and will not touch other drivers.
BTW, the patch format is also updated and pass all check.
Could you help to review again? Thanks.
Regards,
Lisa
-----Original Message-----
From: Chang, Abner <Abner.Chang at amd.com>
Sent: Tuesday, October 24, 2023 6:43 PM
To: Huang, Li-Xia <lisa.huang at intel.com>; devel at edk2.groups.io
Cc: Isaac Oram <isaac.w.oram at intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Wu, Yidong <yidong.wu at intel.com>; Xu, Wei6 <wei6.xu at intel.com>
Subject: RE: [edk2-devel] [PATCH v1 1/1] IpmiFeaturePkg/GenericIpmi: Support Standalone MM
[AMD Official Use Only - General]
BTW Lisa,
As Issac told me the implementation which Intel upstream to IpmiFeaturePkg was from a pretty old intel code base. Also, something like FAST_VIDEO_SUPPORT, BmcSlaveAddress and GetBmcStatus seems not used. I think those code should be removed as well.
Abner
> -----Original Message-----
> From: Huang, Li-Xia <lisa.huang at intel.com>
> Sent: Monday, October 23, 2023 1:48 PM
> To: Chang, Abner <Abner.Chang at amd.com>; devel at edk2.groups.io
> Cc: Isaac Oram <isaac.w.oram at intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone at intel.com>; Wu, Yidong <yidong.wu at intel.com>;
> Xu,
> Wei6 <wei6.xu at intel.com>
> Subject: RE: [edk2-devel] [PATCH v1 1/1] IpmiFeaturePkg/GenericIpmi:
> Support Standalone MM
>
> [AMD Official Use Only - General]
>
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
>
>
> Hi Abner,
>
> Thanks for your feedback.
> Since the IPMI implementation in these two packages are diverging now,
> and my patch has some dependency with common code in IpmiFeaturePkg ,
> it will be better to resend this patch after the diverging is done.
>
> Our target schedule is '23 WW51', could you let me know what's the
> schedule for these two packages'(IpmiFeaturePkg and ManageabilityPkg) diverging?
> Thanks.
>
> Regards,
> Lisa
>
> -----Original Message-----
> From: Chang, Abner <Abner.Chang at amd.com>
> Sent: Saturday, October 21, 2023 10:15 PM
> To: devel at edk2.groups.io; Huang, Li-Xia <lisa.huang at intel.com>
> Cc: Isaac Oram <isaac.w.oram at intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone at intel.com>
> Subject: RE: [edk2-devel] [PATCH v1 1/1] IpmiFeaturePkg/GenericIpmi:
> Support Standalone MM
>
> [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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > Huang, Li-Xia via groups.io
> > Sent: Friday, October 20, 2023 2:46 PM
> > To: devel at edk2.groups.io
> > Cc: Isaac Oram <isaac.w.oram at intel.com>; Nate DeSimone
> > <nathaniel.l.desimone at 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 at intel.com>
> > Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
> > Signed-off-by: Lixia Huang <lisa.huang at 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 at edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [abner.chang at amd.com]
> > -=-=-=-=-=-=
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110073): https://edk2.groups.io/g/devel/message/110073
Mute This Topic: https://groups.io/mt/102076516/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list