Re: [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFrb: IPMI FRB Driver

["Chang, Abner" <abner.chang@amd.com>]

[AMD Official Use Only - General]

Thanks Tinh, that typo is corrected. Just I am not going to send another version of patch set for this change for review.
Abner

> -----Original Message-----
> From: Tinh Nguyen <tinhnguyen@amperemail.onmicrosoft.com>
> Sent: Monday, May 15, 2023 12:47 AM
> To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Isaac Oram <isaac.w.oram@intel.com>; Attar, AbdulLateef (Abdul Lateef)
> <AbdulLateef.Attar@amd.com>; Nickle Wang <nicklew@nvidia.com>; Tinh
> Nguyen <tinhnguyen@os.amperecomputing.com>
> Subject: Re: [edk2-platforms][PATCH 1/2] ManageabilityPkg/IpmiFrb: IPMI
> FRB Driver
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> Reviewed-by: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>
> 
> There is a minor typo that I commented on below.
> 
> 
> On 13/05/2023 19:33, abner.chang@amd.com wrote:
> > From: Abner Chang <abner.chang@amd.com>
> >
> > IpmiFrb is cloned from
> > edk2-platforms/Features/Intel/OutOfBandManagement/
> > IpmiFeaturePkg/Frb in order to consolidate
> > edk2 system manageability support in one place.
> > Uncustify is applied to C files and no functionalities
> > are changed in this patch.
> >
> > We will still keep the one under IpmiFeaturePkg/Frb
> > until the reference to this instance are removed from
> > platforms.
> >
> > Signed-off-by: Abner Chang <abner.chang@amd.com>
> > Cc: Isaac Oram <isaac.w.oram@intel.com>
> > Cc: Abdul Lateef Attar <abdattar@amd.com>
> > Cc: Nickle Wang <nicklew@nvidia.com>
> > Cc: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>
> > ---
> >   .../ManageabilityPkg/ManageabilityPkg.dec     |   5 +
> >   .../Universal/IpmiFrb/FrbDxe.inf              |  37 +++
> >   .../Universal/IpmiFrb/FrbPei.inf              |  37 +++
> >   .../Universal/IpmiFrb/FrbDxe.c                | 212 ++++++++++++++++++
> >   .../Universal/IpmiFrb/FrbPei.c                |  87 +++++++
> >   5 files changed, 378 insertions(+)
> >   create mode 100644
> Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
> >   create mode 100644
> Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
> >   create mode 100644
> Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
> >   create mode 100644
> Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.c
> >
> > diff --git a/Features/ManageabilityPkg/ManageabilityPkg.dec
> b/Features/ManageabilityPkg/ManageabilityPkg.dec
> > index 38813c5f48..b0ca01094a 100644
> > --- a/Features/ManageabilityPkg/ManageabilityPkg.dec
> > +++ b/Features/ManageabilityPkg/ManageabilityPkg.dec
> > @@ -80,3 +80,8 @@
> >
> gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxeMctpEnable|FALSE
> |BOOLEAN|0x10000005
> >
> gManageabilityPkgTokenSpaceGuid.PcdManageabilityDxePldmSmbiosTransf
> erEnable|FALSE|BOOLEAN|0x10000006
> >
> > +[PcdsDynamic, PcdsDynamicEx]
> > +
> gManageabilityPkgTokenSpaceGuid.PcdFRB2EnabledFlag|TRUE|BOOLEAN|0
> x20000001
> > +  ## This is the timeout value in milliseconds, default set to 360
> milliseconds
> > +  # @Prompt IPMI Fault Resilient Booting timeout value in milliseconds.
> > +
> gManageabilityPkgTokenSpaceGuid.PcdFRBTimeoutValue|360|UINT16|0x20
> 000002
> > diff --git a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
> b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
> > new file mode 100644
> > index 0000000000..ae57fe7697
> > --- /dev/null
> > +++ b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.inf
> > @@ -0,0 +1,37 @@
> > +### @file
> > +# Component description file for IPMI FRB.
> > +#
> > +# Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +###
> > +
> > +[defines]
> > +  INF_VERSION          = 0x00010005
> > +  BASE_NAME            = FrbDxe
> > +  FILE_GUID            = A142CEE5-99D5-4ECF-943E-D8F0DE3052AA
> > +  MODULE_TYPE          = DXE_DRIVER
> > +  VERSION_STRING       = 1.0
> > +  ENTRY_POINT          = FrbDxeEntryPoint
> > +
> > +[Sources]
> > +  FrbDxe.c
> > +
> > +[Packages]
> > +  ManageabilityPkg/ManageabilityPkg.dec
> > +  MdePkg/MdePkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> > +
> > +[LibraryClasses]
> > +  UefiLib
> > +  BaseMemoryLib
> > +  DebugLib
> > +  IpmiCommandLib
> > +  MemoryAllocationLib
> > +  PcdLib
> > +  UefiBootServicesTableLib
> > +  UefiDriverEntryPoint
> > +
> > +[Depex]
> > +  TRUE
> > diff --git a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
> b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
> > new file mode 100644
> > index 0000000000..89d633f32e
> > --- /dev/null
> > +++ b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.inf
> > @@ -0,0 +1,37 @@
> > +### @file
> > +# Component description file for IPMI FRB PEIM.
> > +#
> > +# Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
> > +#
> > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > +#
> > +###
> > +
> > +[defines]
> > +  INF_VERSION          = 0x00010005
> > +  BASE_NAME            = FrbPei
> > +  FILE_GUID            = 7CAAE1A7-0B37-4EEA-A432-2F203DA6F288
> > +  MODULE_TYPE          = PEIM
> > +  VERSION_STRING       = 1.0
> > +  ENTRY_POINT          = InitializeFrbPei
> > +
> > +[Sources]
> > +  FrbPei.c
> > +
> > +[Packages]
> > +  ManageabilityPkg/ManageabilityPkg.dec
> > +  MdePkg/MdePkg.dec
> > +
> > +[LibraryClasses]
> > +  BaseMemoryLib
> > +  DebugLib
> > +  IpmiCommandLib
> > +  PcdLib
> > +  PeimEntryPoint
> > +
> > +[Pcd]
> > +  gManageabilityPkgTokenSpaceGuid.PcdFRB2EnabledFlag
> > +  gManageabilityPkgTokenSpaceGuid.PcdFRBTimeoutValue
> > +
> > +[Depex]
> > +  TRUE
> > diff --git a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
> b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
> > new file mode 100644
> > index 0000000000..46f741eed1
> > --- /dev/null
> > +++ b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbDxe.c
> > @@ -0,0 +1,212 @@
> > +/** @file
> > +    IPMI Fault Resilient Booting Driver.
> > +
> > +Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
> > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <PiDxe.h>
> > +#include <Library/UefiBootServicesTableLib.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/UefiLib.h>
> > +#include <Library/MemoryAllocationLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/IpmiCommandLib.h>
> > +#include <IndustryStandard/Ipmi.h>
> > +
> > +/**
> > +  This routine disables the specified FRB timer.
> > +
> > +  @retval EFI_STATUS  EFI_SUCCESS      FRB timer was disabled
> > +                      EFI_ABORTED      Timer was already stopped
> > +                      EFI_UNSUPPORTED  This type of FRB timer is not supported.
> > +
> > +**/
> > +EFI_STATUS
> > +EfiDisableFrb (
> > +  VOID
> > +  )
> > +{
> > +  EFI_STATUS                        Status;
> > +  IPMI_SET_WATCHDOG_TIMER_REQUEST   SetWatchdogTimer;
> > +  UINT8                             CompletionCode;
> > +  IPMI_GET_WATCHDOG_TIMER_RESPONSE  GetWatchdogTimer;
> > +
> > +  Status = IpmiGetWatchdogTimer (&GetWatchdogTimer);
> > +  if (EFI_ERROR (Status)) {
> > +    return Status;
> > +  }
> > +
> > +  //
> > +  // Check if timer is still running, if not abort disable routine.
> > +  //
> > +  if (GetWatchdogTimer.TimerUse.Bits.TimerRunning == 0) {
> > +    return EFI_ABORTED;
> > +  }
> > +
> > +  ZeroMem (&SetWatchdogTimer, sizeof (SetWatchdogTimer));
> > +  //
> > +  // Just flip the Timer Use bit. This should release the timer.
> > +  //
> > +  SetWatchdogTimer.TimerUse.Bits.TimerRunning    = 0;
> > +  SetWatchdogTimer.TimerUse.Bits.TimerUse        =
> IPMI_WATCHDOG_TIMER_BIOS_FRB2;
> > +  SetWatchdogTimer.TimerUseExpirationFlagsClear &= ~BIT2;
> > +  SetWatchdogTimer.TimerUseExpirationFlagsClear |= BIT1 | BIT4;
> > +
> > +  Status = IpmiSetWatchdogTimer (&SetWatchdogTimer,
> &CompletionCode);
> > +  return Status;
> > +}
> > +
> > +/**
> > +  This function disables FRB2. This function gets called each time the
> > +  EFI_EVENT_SIGNAL_READY_TO_BOOT gets signaled
> > +
> > +  @param[in] Event    The handle of callback event.
> > +  @param[in] Context  This should be NULL means no context associated
> > +                      with this event.
> > +
> > +**/
> > +VOID
> > +EFIAPI
> > +DisableFRB2Handler (
> > +  IN EFI_EVENT  Event,
> > +  IN VOID       *Context
> > +  )
> > +{
> > +  DEBUG ((DEBUG_ERROR, "!!! enter DisableFRB2Handler()!!!\n"));
> > +
> > +  EfiDisableFrb ();
> > +}
> > +
> > +/**
> > +  This function checks the Watchdog timer expiration flags and
> > +  report the kind of watchdog timeout occurred to the Error
> > +  Manager.
> > +
> > +  @retval EFI_STATUS  EFI_SUCCESS  Timeout status is checked and
> cleared.
> > +                      EFI_ERROR    There was an error when check and clear
> > +                                   timeout status.
> > +
> > +**/
> > +EFI_STATUS
> > +CheckForAndReportErrors (
> > +  VOID
> > +  )
> > +{
> > +  EFI_STATUS                        Status;
> > +  IPMI_GET_WATCHDOG_TIMER_RESPONSE  GetWatchdogTimer;
> > +  IPMI_SET_WATCHDOG_TIMER_REQUEST   SetWatchdogTimer;
> > +  UINT8                             CompletionCode;
> > +
> > +  //
> > +  // Get the Watchdog timer info to find out what kind of timer expiration
> occurred.
> > +  //
> > +  Status = IpmiGetWatchdogTimer (&GetWatchdogTimer);
> > +  if (EFI_ERROR (Status)) {
> > +    return Status;
> > +  }
> > +
> > +  //
> > +  // If FRB2 Failure occurred, report it to the error manager and log a SEL.
> > +  //
> > +  if ((GetWatchdogTimer.TimerUseExpirationFlagsClear & BIT1) != 0) {
> > +    //
> > +    // Report the FRB2 time-out error
> > +    //
> > +  } else if ((GetWatchdogTimer.TimerUseExpirationFlagsClear & BIT3) != 0)
> {
> > +    //
> > +    // Report the OS Watchdog timer failure
> > +    //
> > +  }
> > +
> > +  //
> > +  // Need to clear Timer expiration flags after checking.
> > +  //
> > +  ZeroMem (&SetWatchdogTimer, sizeof (SetWatchdogTimer));
> > +  SetWatchdogTimer.TimerUse                      = GetWatchdogTimer.TimerUse;
> > +  SetWatchdogTimer.TimerActions                  =
> GetWatchdogTimer.TimerActions;
> > +  SetWatchdogTimer.PretimeoutInterval            =
> GetWatchdogTimer.PretimeoutInterval;
> > +  SetWatchdogTimer.TimerUseExpirationFlagsClear  =
> GetWatchdogTimer.TimerUseExpirationFlagsClear;
> > +  SetWatchdogTimer.InitialCountdownValue         =
> GetWatchdogTimer.InitialCountdownValue;
> > +  SetWatchdogTimer.TimerUse.Bits.TimerRunning    = 1;
> > +  SetWatchdogTimer.TimerUseExpirationFlagsClear |= BIT1 | BIT2 | BIT3;
> > +
> > +  Status = IpmiSetWatchdogTimer (&SetWatchdogTimer,
> &CompletionCode);
> > +
> > +  return Status;
> > +}
> > +
> > +/**
> > +  This routine is built only when DEBUG_MODE is enabled.  It is used
> > +  to report the status of FRB2 when the FRB2 driver is installed.
> > +
> > +  @retval EFI_STATUS  EFI_SUCCESS  All info was retrieved and reported
> > +                      EFI_ERROR    There was an error during info retrieval
> > +
> > +**/
> > +EFI_STATUS
> > +ReportFrb2Status (
> > +  VOID
> > +  )
> > +{
> > +  EFI_STATUS                        Status;
> > +  IPMI_GET_WATCHDOG_TIMER_RESPONSE  GetWatchdogTimer;
> > +
> > +  //
> > +  // Get the Watchdog timer info to find out what kind of timer expiration
> occurred.
> > +  //
> > +  Status = IpmiGetWatchdogTimer (&GetWatchdogTimer);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_INFO, "Failed to get Watchdog Timer info from
> BMC.\n"));
> > +    return Status;
> > +  }
> > +
> > +  //
> > +  // Check if timer is running, report status to DEBUG_MODE output.
> > +  //
> > +  if (GetWatchdogTimer.TimerUse.Bits.TimerRunning == 1) {
> > +    DEBUG ((DEBUG_INFO, "FRB2 Timer is running.\n"));
> > +  } else {
> > +    DEBUG ((DEBUG_INFO, "FRB2 Timer is not running.\n"));
> > +  }
> > +
> > +  return EFI_SUCCESS;
> > +}
> > +
> > +/**
> > +  The entry point of the Ipmi Fault Resilient Booting 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
> > +FrbDxeEntryPoint (
> > +  IN EFI_HANDLE        ImageHandle,
> > +  IN EFI_SYSTEM_TABLE  *SystemTable
> > +  )
> > +{
> > +  EFI_EVENT   ReadyToBootEvent;
> > +  EFI_STATUS  Status;
> > +
> > +  CheckForAndReportErrors ();
> > +  ReportFrb2Status ();
> > +
> > +  //
> > +  // Register the event to Disable FRB2 before Boot.
> > +  //
> > +  Status = EfiCreateEventReadyToBootEx (
> > +             TPL_NOTIFY,
> > +             DisableFRB2Handler,
> > +             NULL,
> > +             &ReadyToBootEvent
> > +             );
> > +
> > +  return Status;
> > +}
> > diff --git a/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.c
> b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.c
> > new file mode 100644
> > index 0000000000..431cac17bc
> > --- /dev/null
> > +++ b/Features/ManageabilityPkg/Universal/IpmiFrb/FrbPei.c
> > @@ -0,0 +1,87 @@
> > +/** @file
> > +    IPMI Fault Resilient Booting (FRB) PEIM.
> > +
> > +Copyright (c) 2018 - 2019, Intel Corporation. All rights reserved.<BR>
> > +SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +
> > +#include <PiPei.h>
> > +#include <Library/DebugLib.h>
> > +#include <Library/BaseMemoryLib.h>
> > +#include <Library/IoLib.h>
> > +#include <Library/PcdLib.h>
> > +#include <Library/IpmiCommandLib.h>
> > +
> > +#include <IndustryStandard/Ipmi.h>
> > +
> > +/**
> > +  This function sets wtchdog timer for Failt Resilient Booting
> watchdog
> > +  according to Frb2Enabled.
> > +
> > +  @param [in]  Frb2Enabled  Whether to enable FRB2 timeout
> > +
> > +**/
> > +VOID
> > +SetWatchDogTimer (
> > +  IN BOOLEAN  Frb2Enabled
> > +  )
> > +{
> > +  EFI_STATUS                        Status;
> > +  IPMI_SET_WATCHDOG_TIMER_REQUEST   FrbTimer;
> > +  IPMI_GET_WATCHDOG_TIMER_RESPONSE  GetWatchdogTimer;
> > +  UINT8                             CompletionCode;
> > +
> > +  Status = IpmiGetWatchdogTimer (&GetWatchdogTimer);
> > +  if (EFI_ERROR (Status)) {
> > +    return;
> > +  }
> > +
> > +  if (Frb2Enabled) {
> > +    ZeroMem (&FrbTimer, sizeof (FrbTimer));
> > +    // Byte 1
> > +    FrbTimer.TimerUse.Bits.TimerUse =
> IPMI_WATCHDOG_TIMER_BIOS_FRB2;
> > +    // Byte 2
> > +    FrbTimer.TimerActions.Uint8 = 0;    // NormalBoot, NoTimeOutInterrupt.
> i.e no action when BMC watchdog timeout
> > +    // Byte 3
> > +    FrbTimer.PretimeoutInterval = 0;
> > +    // Byte 4
> > +    FrbTimer.TimerUseExpirationFlagsClear |= BIT1;  // set
> Frb2ExpirationFlag
> > +
> > +    // Data Byte 5/6
> > +    FrbTimer.InitialCountdownValue = PcdGet16 (PcdFRBTimeoutValue) *
> 10;
> > +
> > +    // Set BMC watchdog timer
> > +    Status = IpmiSetWatchdogTimer (&FrbTimer, &CompletionCode);
> > +    Status = IpmiResetWatchdogTimer (&CompletionCode);
> > +  }
> > +}
> > +
> > +/**
> > +  The entry point of the Ipmi Fault Resilient Booting PEIM.
> > +
> > +  @param [in]  FileHandle  Handle of the file being invoked.
> > +  @param [in]  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
> > +InitializeFrbPei (
> > +  IN       EFI_PEI_FILE_HANDLE  FileHandle,
> > +  IN CONST EFI_PEI_SERVICES     **PeiServices
> > +  )
> > +{
> > +  BOOLEAN  Frb2Enabled;
> > +
> > +  //
> > +  // If we are booting with defaults, then make sure FRB2 is enabled.
> > +  //
> > +  Frb2Enabled = PcdGetBool (PcdFRB2EnabledFlag);
> > +
> > +  SetWatchDogTimer (Frb2Enabled);
> > +
> > +  return EFI_SUCCESS;
> > +}