Re: [edk2-devel][edk2-platforms][PATCH V1 1/1] WhitleyOpenBoardPkg/BoardAcpiLib: Simplify implementation

["Nate DeSimone" <nathaniel.l.desimone@intel.com>]

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@intel.com>

-----Original Message-----
From: Oram, Isaac W <isaac.w.oram@intel.com> 
Sent: Wednesday, November 10, 2021 6:05 PM
To: devel@edk2.groups.io
Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>
Subject: [edk2-devel][edk2-platforms][PATCH V1 1/1] WhitleyOpenBoardPkg/BoardAcpiLib: Simplify implementation

Remove DXE version as the library isn't in use.
Simplify the SMM library.  Remove functions calling functions.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
---
 Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.c     |  37 ------
 Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf   |  44 -------
 Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeMtOlympusAcpiTableLib.c |  54 --------
 Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.c    |  89 +++++++++++--
 Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf  |   1 -
 Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmSiliconAcpiEnableLib.c  | 138 --------------------
 Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc                                 |   7 +-
 7 files changed, 77 insertions(+), 293 deletions(-)

diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.c
deleted file mode 100644
index dfa0c994dc..0000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.c
+++ /dev/null
@@ -1,37 +0,0 @@
-/** @file
-  Platform Hook Library instances
-
-  @copyright
-  Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#include <Base.h>
-#include <Uefi.h>
-#include <PiDxe.h>
-#include <Library/BaseLib.h>
-#include <Library/IoLib.h>
-#include <Library/BoardAcpiTableLib.h>
-#include <Library/PcdLib.h>
-#include <Library/DebugLib.h>
-
-EFI_STATUS
-EFIAPI
-MtOlympusBoardUpdateAcpiTable (
-  IN OUT EFI_ACPI_COMMON_HEADER       *Table,
-  IN OUT EFI_ACPI_TABLE_VERSION       *Version
-  );
-
-EFI_STATUS
-EFIAPI
-BoardUpdateAcpiTable (
-  IN OUT EFI_ACPI_COMMON_HEADER       *Table,
-  IN OUT EFI_ACPI_TABLE_VERSION       *Version
-  )
-{
-  MtOlympusBoardUpdateAcpiTable (Table, Version);
-
-  return EFI_SUCCESS;
-}
-
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
deleted file mode 100644
index 3186c6c91e..0000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
+++ /dev/null
@@ -1,44 +0,0 @@
-### @file
-# Platform Hook Library instance for SandyBridge Mobile/Desktop CRB.
-#
-# @copyright
-# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> -# -# SPDX-License-Identifier: BSD-2-Clause-Patent -# -##
-
-[Defines]
-  INF_VERSION                    = 0x00010017
-  BASE_NAME                      = DxeBoardAcpiTableLib
-  FILE_GUID                      = 6562E0AE-90D8-4D41-8C97-81286B4BE7D2
-  VERSION_STRING                 = 1.0
-  MODULE_TYPE                    = BASE
-  LIBRARY_CLASS                  = BoardAcpiTableLib
-
-#
-# The following information is for reference only and not required by the build tools.
-#
-# VALID_ARCHITECTURES = IA32 X64 IPF EBC -#
-
-[LibraryClasses]
-  BaseLib
-  IoLib
-  PciLib
-  PcdLib
-[Packages]
-  MdePkg/MdePkg.dec
-  MdeModulePkg/MdeModulePkg.dec
-  MinPlatformPkg/MinPlatformPkg.dec
-  WhitleyOpenBoardPkg/PlatformPkg.dec
-  WhitleySiliconPkg/WhitleySiliconPkg.dec
-  WhitleySiliconPkg/SiliconPkg.dec
-
-[Pcd]
-  gOemSkuTokenSpaceGuid.PcdAcpiGnvsAddress
-
-[Sources]
-  DxeMtOlympusAcpiTableLib.c
-  DxeBoardAcpiTableLib.c
-
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeMtOlympusAcpiTableLib.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeMtOlympusAcpiTableLib.c
deleted file mode 100644
index 09b917083c..0000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeMtOlympusAcpiTableLib.c
+++ /dev/null
@@ -1,54 +0,0 @@
-/** @file
-  Platform Hook Library instances
-
-  @copyright
-  Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#include <Base.h>
-#include <Uefi.h>
-#include <PiDxe.h>
-#include <Library/BaseLib.h>
-#include <Library/IoLib.h>
-#include <Library/BoardAcpiTableLib.h>
-#include <Library/PcdLib.h>
-#include <Library/DebugLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Protocol/GlobalNvsArea.h>
-
-GLOBAL_REMOVE_IF_UNREFERENCED BIOS_ACPI_PARAM              *mGlobalNvsArea;
-
-VOID
-MtOlympusUpdateGlobalNvs (
-  VOID
-  )
-{
-
-  //
-  // Allocate and initialize the NVS area for SMM and ASL communication.
-  //
-  mGlobalNvsArea = (VOID *)(UINTN)PcdGet64 (PcdAcpiGnvsAddress);
-
-  //
-  // Update global NVS area for ASL and SMM init code to use
-  //
-
-
-}
-
-EFI_STATUS
-EFIAPI
-MtOlympusBoardUpdateAcpiTable (
-  IN OUT EFI_ACPI_COMMON_HEADER       *Table,
-  IN OUT EFI_ACPI_TABLE_VERSION       *Version
-  )
-{
-  if (Table->Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
-    MtOlympusUpdateGlobalNvs ();
-  }
-
-  return EFI_SUCCESS;
-}
-
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.c
index 09a6b00877..0f8a62460a 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.c
+++ b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAc
+++ piEnableLib.c
@@ -15,18 +15,9 @@
 #include <Library/BoardAcpiEnableLib.h>  #include <Library/PcdLib.h>  #include <Library/DebugLib.h>
-
-EFI_STATUS
-EFIAPI
-SiliconEnableAcpi (
-  IN BOOLEAN  EnableSci
-  );
-
-EFI_STATUS
-EFIAPI
-SiliconDisableAcpi (
-  IN BOOLEAN  DisableSci
-  );
+#include <PchAccess.h>
+#include <Protocol/DynamicSiLibrarySmmProtocol.h>
+#include <Library/SmmServicesTableLib.h>
 
 EFI_STATUS
 EFIAPI
@@ -34,7 +25,52 @@ BoardEnableAcpi (
   IN BOOLEAN  EnableSci
   )
 {
-  SiliconEnableAcpi (EnableSci);
+  UINT32                           SmiEn;
+  UINT16                           Pm1En;
+  UINT16                           Pm1Cnt;
+  UINT16                           PchPmBase;
+  EFI_STATUS                       Status;
+  DYNAMIC_SI_LIBARY_SMM_PROTOCOL   *DynamicSiLibrarySmmProtocol = NULL;
+
+  Status = gSmst->SmmLocateProtocol (&gDynamicSiLibrarySmmProtocolGuid, 
+ NULL, &DynamicSiLibrarySmmProtocol);  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  //
+  // Init Power Management I/O Base aka ACPI Base  //  PchPmBase = 
+ DynamicSiLibrarySmmProtocol->PmcGetAcpiBase ();
+
+  SmiEn = IoRead32 (PchPmBase + R_ACPI_IO_SMI_EN);
+
+  //
+  // Disable SW SMI Timer and legacy USB  //  SmiEn &= 
+ ~(B_ACPI_IO_SMI_EN_SWSMI_TMR | B_ACPI_IO_SMI_EN_LEGACY_USB | 
+ B_ACPI_IO_SMI_EN_LEGACY_USB2);
+
+  //
+  // And enable SMI on write to B_PCH_ACPI_PM1_CNT_SLP_EN when SLP_TYP 
+ is written  //  SmiEn |= B_ACPI_IO_SMI_EN_ON_SLP_EN ;
+  IoWrite32 (PchPmBase + R_ACPI_IO_SMI_EN, SmiEn);
+
+  //
+  // Disable PM sources except power button  //
+  Pm1En   = B_ACPI_IO_PM1_EN_PWRBTN;
+  IoWrite16 (PchPmBase + R_ACPI_IO_PM1_EN, Pm1En);
+
+  //
+  // Enable SCI
+  //
+  if (EnableSci) {
+    Pm1Cnt = IoRead16 (PchPmBase + R_ACPI_IO_PM1_CNT);
+    Pm1Cnt |= B_ACPI_IO_PM1_CNT_SCI_EN;
+    IoWrite16 (PchPmBase + R_ACPI_IO_PM1_CNT, Pm1Cnt);  }
+
   return EFI_SUCCESS;
 }
 
@@ -44,7 +80,32 @@ BoardDisableAcpi (
   IN BOOLEAN  DisableSci
   )
 {
-  SiliconDisableAcpi (DisableSci);
+  UINT16                           Pm1Cnt;
+  UINT16                           PchPmBase;
+  EFI_STATUS                       Status;
+  DYNAMIC_SI_LIBARY_SMM_PROTOCOL   *DynamicSiLibrarySmmProtocol = NULL;
+
+  Status = gSmst->SmmLocateProtocol (&gDynamicSiLibrarySmmProtocolGuid, 
+ NULL, &DynamicSiLibrarySmmProtocol);  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    return Status;
+  }
+
+  //
+  // Init Power Management I/O Base aka ACPI Base  //  PchPmBase = 
+ DynamicSiLibrarySmmProtocol->PmcGetAcpiBase ();
+
+
+  //
+  // Disable SCI
+  //
+  if (DisableSci) {
+    Pm1Cnt = IoRead16 (PchPmBase + R_ACPI_IO_PM1_CNT);
+    Pm1Cnt &= ~B_ACPI_IO_PM1_CNT_SCI_EN;
+    IoWrite16 (PchPmBase + R_ACPI_IO_PM1_CNT, Pm1Cnt);  }
+
   return EFI_SUCCESS;
 }
 
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
index 19d29ed40f..fcb3e23365 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
+++ b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAc
+++ piEnableLib.inf
@@ -38,7 +38,6 @@
   WhitleySiliconPkg/CpRcPkg.dec
 
 [Sources]
-  SmmSiliconAcpiEnableLib.c
   SmmBoardAcpiEnableLib.c
 
 [Protocols]
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmSiliconAcpiEnableLib.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmSiliconAcpiEnableLib.c
deleted file mode 100644
index 484311811b..0000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmSiliconAcpiEnableLib.c
+++ /dev/null
@@ -1,138 +0,0 @@
-/** @file
-  Platform Hook Library instances
-
-  @copyright
-  Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#include <Base.h>
-#include <Uefi.h>
-#include <PiDxe.h>
-#include <Library/BaseLib.h>
-#include <Library/IoLib.h>
-#include <Library/BoardAcpiEnableLib.h> -#include <Library/PcdLib.h> -#include <Library/DebugLib.h> -#include <PchAccess.h> -#include <Protocol/DynamicSiLibrarySmmProtocol.h>
-#include <Library/SmmServicesTableLib.h>
-
-/**
-  Clear Port 80h
-
-  SMI handler to enable ACPI mode
-
-  Dispatched on reads from APM port with value EFI_ACPI_ENABLE_SW_SMI
-
-  Disables the SW SMI Timer.
-  ACPI events are disabled and ACPI event status is cleared.
-  SCI mode is then enabled.
-
-  Clear SLP SMI status
-  Enable SLP SMI
-
-  Disable SW SMI Timer
-
-  Clear all ACPI event status and disable all ACPI events
-
-  Disable PM sources except power button
-  Clear status bits
-
-  Disable GPE0 sources
-  Clear status bits
-
-  Disable GPE1 sources
-  Clear status bits
-
-  Guarantee day-of-month alarm is invalid (ACPI 1.0 section 4.7.2.4)
-
-  Enable SCI
-**/
-EFI_STATUS
-EFIAPI
-SiliconEnableAcpi (
-  IN BOOLEAN  EnableSci
-  )
-{
-  UINT32                           SmiEn;
-  UINT16                           Pm1En;
-  UINT16                           Pm1Cnt;
-  UINT16                           PchPmBase;
-  EFI_STATUS                       Status;
-  DYNAMIC_SI_LIBARY_SMM_PROTOCOL   *DynamicSiLibrarySmmProtocol = NULL;
-
-  Status = gSmst->SmmLocateProtocol (&gDynamicSiLibrarySmmProtocolGuid, NULL, &DynamicSiLibrarySmmProtocol);
-  if (EFI_ERROR (Status)) {
-    ASSERT_EFI_ERROR (Status);
-    return Status;
-  }
-
-  //
-  // Init Power Management I/O Base aka ACPI Base
-  //
-  PchPmBase = DynamicSiLibrarySmmProtocol->PmcGetAcpiBase ();
-
-  SmiEn = IoRead32 (PchPmBase + R_ACPI_IO_SMI_EN);
-
-  //
-  // Disable SW SMI Timer and legacy USB
-  //
-  SmiEn &= ~(B_ACPI_IO_SMI_EN_SWSMI_TMR | B_ACPI_IO_SMI_EN_LEGACY_USB | B_ACPI_IO_SMI_EN_LEGACY_USB2);
-
-  //
-  // And enable SMI on write to B_PCH_ACPI_PM1_CNT_SLP_EN when SLP_TYP is written
-  //
-  SmiEn |= B_ACPI_IO_SMI_EN_ON_SLP_EN ;
-  IoWrite32 (PchPmBase + R_ACPI_IO_SMI_EN, SmiEn);
-
-  //
-  // Disable PM sources except power button
-  //
-  Pm1En   = B_ACPI_IO_PM1_EN_PWRBTN;
-  IoWrite16 (PchPmBase + R_ACPI_IO_PM1_EN, Pm1En);
-
-  //
-  // Enable SCI
-  //
-  Pm1Cnt = IoRead16 (PchPmBase + R_ACPI_IO_PM1_CNT);
-  Pm1Cnt |= B_ACPI_IO_PM1_CNT_SCI_EN;
-  IoWrite16 (PchPmBase + R_ACPI_IO_PM1_CNT, Pm1Cnt);
-
-  return EFI_SUCCESS;
-}
-
-EFI_STATUS
-EFIAPI
-SiliconDisableAcpi (
-  IN BOOLEAN  DisableSci
-  )
-{
-  UINT16                           Pm1Cnt;
-  UINT16                           PchPmBase;
-  EFI_STATUS                       Status;
-  DYNAMIC_SI_LIBARY_SMM_PROTOCOL   *DynamicSiLibrarySmmProtocol = NULL;
-
-  Status = gSmst->SmmLocateProtocol (&gDynamicSiLibrarySmmProtocolGuid, NULL, &DynamicSiLibrarySmmProtocol);
-  if (EFI_ERROR (Status)) {
-    ASSERT_EFI_ERROR (Status);
-    return Status;
-  }
-
-  //
-  // Init Power Management I/O Base aka ACPI Base
-  //
-  PchPmBase = DynamicSiLibrarySmmProtocol->PmcGetAcpiBase ();
-
-  Pm1Cnt = IoRead16 (PchPmBase + R_ACPI_IO_PM1_CNT);
-
-  //
-  // Disable SCI
-  //
-  Pm1Cnt &= ~B_ACPI_IO_PM1_CNT_SCI_EN;
-
-  IoWrite16 (PchPmBase + R_ACPI_IO_PM1_CNT, Pm1Cnt);
-
-  return EFI_SUCCESS;
-}
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
index 63cb4dd559..ae3646df7a 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
+++ b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
@@ -627,10 +627,10 @@
 
 [LibraryClasses.Common.DXE_SMM_DRIVER]
   SpiFlashCommonLib|$(RP_PKG)/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf
-
   TestPointCheckLib|MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.inf
   TestPointLib|MinPlatformPkg/Test/Library/TestPointLib/SmmTestPointLib.inf
   MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
+  
+ BoardAcpiEnableLib|$(RP_PKG)/Library/BoardAcpiLib/SmmBoardAcpiEnableLi
+ b.inf
 
 [LibraryClasses.Common.SMM_CORE]
   S3BootScriptLib|MdePkg/Library/BaseS3BootScriptLibNull/BaseS3BootScriptLibNull.inf
@@ -745,10 +745,7 @@
 
   $(RP_PKG)/Features/AcpiVtd/AcpiVtd.inf
 
-  $(PLATFORM_PKG)/Acpi/AcpiSmm/AcpiSmm.inf {
-    <LibraryClasses>
-      BoardAcpiEnableLib|$(RP_PKG)/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
-  }
+  $(PLATFORM_PKG)/Acpi/AcpiSmm/AcpiSmm.inf
 
   $(PLATFORM_PKG)/PlatformInit/PlatformInitDxe/PlatformInitDxe.inf {
   <LibraryClasses>
--
2.27.0.windows.1