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.ch...@amd.com>
Sent: Tuesday, October 24, 2023 6:43 PM
To: Huang, Li-Xia <lisa.hu...@intel.com>; devel@edk2.groups.io
Cc: Isaac Oram <isaac.w.o...@intel.com>; Desimone, Nathaniel L
<nathaniel.l.desim...@intel.com>; Wu, Yidong <yidong...@intel.com>; Xu, Wei6
<wei6...@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.hu...@intel.com>
> Sent: Monday, October 23, 2023 1:48 PM
> To: Chang, Abner <abner.ch...@amd.com>; devel@edk2.groups.io
> Cc: Isaac Oram <isaac.w.o...@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desim...@intel.com>; Wu, Yidong <yidong...@intel.com>;
> Xu,
> Wei6 <wei6...@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.ch...@amd.com>
> Sent: Saturday, October 21, 2023 10:15 PM
> To: devel@edk2.groups.io; Huang, Li-Xia <lisa.hu...@intel.com>
> Cc: Isaac Oram <isaac.w.o...@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desim...@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@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.o...@intel.com>; Nate DeSimone
> > <nathaniel.l.desim...@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.o...@intel.com>
> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com>
> > Signed-off-by: Lixia Huang <lisa.hu...@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+ow...@edk2.groups.io
> > Unsubscribe: https://edk2.groups.io/g/devel/unsub
> > [abner.ch...@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
