Re: [edk2-devel][edk2-platforms][PATCH v2 3/6] IntelSiliconPkg/Feature/SmmControl: Implement PPI with chipset support

["Isaac Oram" <isaac.w.oram@intel.com>]

Pushed as b013adb40e..1b02220269

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Isaac Oram
Sent: Wednesday, September 7, 2022 4:51 PM
To: Benjamin Doron <benjamin.doron00@gmail.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>; Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
Subject: Re: [edk2-devel][edk2-platforms][PATCH v2 3/6] IntelSiliconPkg/Feature/SmmControl: Implement PPI with chipset support

Reviewed-by: Isaac Oram <isaac.w.oram@intel.com>

-----Original Message-----
From: Benjamin Doron <benjamin.doron00@gmail.com> 
Sent: Tuesday, September 6, 2022 10:02 AM
To: devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Sinha, Ankit <ankit.sinha@intel.com>; Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>
Subject: [edk2-devel][edk2-platforms][PATCH v2 3/6] IntelSiliconPkg/Feature/SmmControl: Implement PPI with chipset support

S3 resume may require communication with SMM, for which we need the SmmControl PPI. Therefore, port the DXE drivers to a library, like there is for SMM Access.

Tested, working on Kabylake. Further testing required after the refactor for compatibility.

Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ankit Sinha <ankit.sinha@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../PeiSmmControlLib/PeiSmmControlLib.c       | 309 ++++++++++++++++++
 .../PeiSmmControlLib/PeiSmmControlLib.inf     |  34 ++
 .../Include/Library/SmmControlLib.h           |  26 ++
 .../Intel/IntelSiliconPkg/IntelSiliconPkg.dec |   4 +
 4 files changed, 373 insertions(+)
 create mode 100644 Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmControlLib/PeiSmmControlLib.c
 create mode 100644 Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmControlLib/PeiSmmControlLib.inf
 create mode 100644 Silicon/Intel/IntelSiliconPkg/Include/Library/SmmControlLib.h

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmControlLib/PeiSmmControlLib.c b/Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmControlLib/PeiSmmControlLib.c
new file mode 100644
index 000000000000..cc6c7f8fe672
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmCon
+++ trolLib/PeiSmmControlLib.c
@@ -0,0 +1,309 @@
+/** @file+  This is to publish the SMM Control Ppi instance.++  Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>+  SPDX-License-Identifier: BSD-2-Clause-Patent++**/+#include <Uefi/UefiBaseType.h>+#include <Library/DebugLib.h>+#include <Library/IoLib.h>+#include <Library/MemoryAllocationLib.h>+#include <Library/PcdLib.h>+#include <Library/PeiServicesLib.h>++#include <Ppi/MmControl.h>+#include <IndustryStandard/Pci30.h>++#define SMM_CONTROL_PRIVATE_DATA_SIGNATURE  SIGNATURE_32 ('i', '4', 's', 'c')++typedef struct {+  UINTN                           Signature;+  EFI_HANDLE                      Handle;+  EFI_PEI_MM_CONTROL_PPI          SmmControl;+} SMM_CONTROL_PRIVATE_DATA;++#define SMM_CONTROL_PRIVATE_DATA_FROM_THIS(a) \+        CR (a, \+          SMM_CONTROL_PRIVATE_DATA, \+          SmmControl, \+          SMM_CONTROL_DEV_SIGNATURE \+      )++//+// Common registers:+//+//+// APM Registers+//+#define R_PCH_APM_CNT                             0xB2+//+// ACPI and legacy I/O register offsets from ACPIBASE+//+#define R_PCH_ACPI_PM1_STS                        0x00+#define B_PCH_ACPI_PM1_STS_PRBTNOR                BIT11++#define R_PCH_SMI_EN                              0x30++#define R_PCH_SMI_STS                             0x34+#define B_PCH_SMI_STS_APM                         BIT5+#define B_PCH_SMI_EN_APMC                         BIT5+#define B_PCH_SMI_EN_EOS                          BIT1+#define B_PCH_SMI_EN_GBL_SMI                      BIT0++/**+  Trigger the software SMI++  @param[in] Data                 The value to be set on the software SMI data port++  @retval EFI_SUCCESS             Function completes successfully+**/+EFI_STATUS+EFIAPI+SmmTrigger (+  UINT8   Data+  )+{+  UINT16  ABase;+  UINT32  OutputData;+  UINT32  OutputPort;++  ABase = FixedPcdGet16 (PcdAcpiBaseAddress);++  ///+  /// Enable the APMC SMI+  ///+  OutputPort  = ABase + R_PCH_SMI_EN;+  OutputData  = IoRead32 ((UINTN) OutputPort);+  OutputData |= (B_PCH_SMI_EN_APMC | B_PCH_SMI_EN_GBL_SMI);+  DEBUG (+    (DEBUG_EVENT,+     "The SMI Control Port at address %x will be written to %x.\n",+     OutputPort,+     OutputData)+    );+  IoWrite32 (+    (UINTN) OutputPort,+    (UINT32) (OutputData)+    );++  OutputPort  = R_PCH_APM_CNT;+  OutputData  = Data;++  ///+  /// Generate the APMC SMI+  ///+  IoWrite8 (+    (UINTN) OutputPort,+    (UINT8) (OutputData)+    );++  return EFI_SUCCESS;+}++/**+  Clear the SMI status+++  @retval EFI_SUCCESS             The function completes successfully+  @retval EFI_DEVICE_ERROR        Something error occurred+**/+EFI_STATUS+EFIAPI+SmmClear (+  VOID+  )+{+  UINT16  ABase;+  UINT32  OutputData;+  UINT32  OutputPort;++  ABase = FixedPcdGet16 (PcdAcpiBaseAddress);++  ///+  /// Clear the Power Button Override Status Bit, it gates EOS from being set.+  ///+  OutputPort  = ABase + R_PCH_ACPI_PM1_STS;+  OutputData  = B_PCH_ACPI_PM1_STS_PRBTNOR;+  DEBUG (+    (DEBUG_EVENT,+     "The PM1 Status Port at address %x will be written to %x.\n",+     OutputPort,+     OutputData)+    );+  IoWrite16 (+    (UINTN) OutputPort,+    (UINT16) (OutputData)+    );++  ///+  /// Clear the APM SMI Status Bit+  ///+  OutputPort  = ABase + R_PCH_SMI_STS;+  OutputData  = B_PCH_SMI_STS_APM;+  DEBUG (+    (DEBUG_EVENT,+     "The SMI Status Port at address %x will be written to %x.\n",+     OutputPort,+     OutputData)+    );+  IoWrite32 (+    (UINTN) OutputPort,+    (UINT32) (OutputData)+    );++  ///+  /// Set the EOS Bit+  ///+  OutputPort  = ABase + R_PCH_SMI_EN;+  OutputData  = IoRead32 ((UINTN) OutputPort);+  OutputData |= B_PCH_SMI_EN_EOS;+  DEBUG (+    (DEBUG_EVENT,+     "The SMI Control Port at address %x will be written to %x.\n",+     OutputPort,+     OutputData)+    );+  IoWrite32 (+    (UINTN) OutputPort,+    (UINT32) (OutputData)+    );++  ///+  /// There is no need to read EOS back and check if it is set.+  /// This can lead to a reading of zero if an SMI occurs right after the SMI_EN port read+  /// but before the data is returned to the CPU.+  /// SMM Dispatcher should make sure that EOS is set after all SMI sources are processed.+  ///+  return EFI_SUCCESS;+}++/**+  This routine generates an SMI++  @param[in] This                       The EFI SMM Control protocol instance+  @param[in, out] ArgumentBuffer        The buffer of argument+  @param[in, out] ArgumentBufferSize    The size of the argument buffer+  @param[in] Periodic                   Periodic or not+  @param[in] ActivationInterval         Interval of periodic SMI++  @retval EFI Status                    Describing the result of the operation+  @retval EFI_INVALID_PARAMETER         Some parameter value passed is not supported+**/+EFI_STATUS+EFIAPI+Activate (+  IN EFI_PEI_SERVICES        **PeiServices,+  IN EFI_PEI_MM_CONTROL_PPI  * This,+  IN OUT INT8                *ArgumentBuffer OPTIONAL,+  IN OUT UINTN               *ArgumentBufferSize OPTIONAL,+  IN BOOLEAN                 Periodic OPTIONAL,+  IN UINTN                   ActivationInterval OPTIONAL+  )+{+  EFI_STATUS  Status;+  UINT8       Data;++  if (Periodic) {+    DEBUG ((DEBUG_WARN, "Invalid parameter\n"));+    return EFI_INVALID_PARAMETER;+  }++  // NOTE: Copied from Quark. Matches the usage in PiSmmCommunicationPei+  if (ArgumentBuffer == NULL) {+    Data = 0xFF;+  } else {+    if (ArgumentBufferSize == NULL || *ArgumentBufferSize != 1) {+      return EFI_INVALID_PARAMETER;+    }++    Data = *ArgumentBuffer;+  }+  ///+  /// Clear any pending the APM SMI+  ///+  Status = SmmClear ();+  if (EFI_ERROR (Status)) {+    return Status;+  }++  return SmmTrigger (Data);+}++/**+  This routine clears an SMI++  @param[in] This                 The EFI SMM Control protocol instance+  @param[in] Periodic             Periodic or not++  @retval EFI Status              Describing the result of the operation+  @retval EFI_INVALID_PARAMETER   Some parameter value passed is not supported+**/+EFI_STATUS+EFIAPI+Deactivate (+  IN EFI_PEI_SERVICES        **PeiServices,+  IN EFI_PEI_MM_CONTROL_PPI  * This,+  IN BOOLEAN                 Periodic OPTIONAL+  )+{+  if (Periodic) {+    return EFI_INVALID_PARAMETER;+  }++  return SmmClear ();+}++/**+  This function is to install an SMM Control PPI+  - <b>Introduction</b> \n+    An API to install an instance of EFI_PEI_MM_CONTROL_PPI. This PPI provides a standard+    way for other modules to trigger software SMIs.++    @retval EFI_SUCCESS           - Ppi successfully started and installed.+    @retval EFI_NOT_FOUND         - Ppi can't be found.+    @retval EFI_OUT_OF_RESOURCES  - Ppi does not have enough resources to initialize the driver.+**/+EFI_STATUS+EFIAPI+PeiInstallSmmControlPpi (+  VOID+  )+{+  EFI_STATUS                      Status;+  EFI_PEI_PPI_DESCRIPTOR          *PpiList;+  SMM_CONTROL_PRIVATE_DATA        *SmmControlPrivate;++  //+  // Initialize private data+  //+  SmmControlPrivate  = AllocateZeroPool (sizeof (*SmmControlPrivate));+  ASSERT (SmmControlPrivate != NULL);+  if (SmmControlPrivate == NULL) {+    return EFI_OUT_OF_RESOURCES;+  }+  PpiList           = AllocateZeroPool (sizeof (*PpiList));+  ASSERT (PpiList != NULL);+  if (PpiList == NULL) {+    return EFI_OUT_OF_RESOURCES;+  }++  SmmControlPrivate->Signature = SMM_CONTROL_PRIVATE_DATA_SIGNATURE;+  SmmControlPrivate->Handle    = NULL;++  SmmControlPrivate->SmmControl.Trigger  = Activate;+  SmmControlPrivate->SmmControl.Clear    = Deactivate;++  //+  // Install PPI+  //+  PpiList->Flags  = (EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST);+  PpiList->Guid   = &gEfiPeiMmControlPpiGuid;+  PpiList->Ppi    = &SmmControlPrivate->SmmControl;++  Status          = PeiServicesInstallPpi (PpiList);+  ASSERT_EFI_ERROR (Status);++  // Unlike driver, do not disable SMIs as S3 resume continues+  return EFI_SUCCESS;+}diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmControlLib/PeiSmmControlLib.inf b/Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmControlLib/PeiSmmControlLib.inf
new file mode 100644
index 000000000000..91c761366446
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmCon
+++ trolLib/PeiSmmControlLib.inf
@@ -0,0 +1,34 @@
+## @file+# Library description file for the SmmControl PPI+#+# Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>+# SPDX-License-Identifier: BSD-2-Clause-Patent+#+##++[Defines]+  INF_VERSION                    = 0x00010017+  BASE_NAME                      = PeiSmmControlLib+  FILE_GUID                      = F45D521A-C0DF-4283-A3CA-65AD01B479E7+  VERSION_STRING                 = 1.0+  MODULE_TYPE                    = PEIM+  LIBRARY_CLASS                  = SmmControlLib++[LibraryClasses]+  IoLib+  DebugLib+  MemoryAllocationLib+  PeiServicesLib++[Packages]+  MdePkg/MdePkg.dec+  IntelSiliconPkg/IntelSiliconPkg.dec++[Sources]+  PeiSmmControlLib.c++[Pcd]+  gIntelSiliconPkgTokenSpaceGuid.PcdAcpiBaseAddress  ## CONSUMES++[Ppis]+  gEfiPeiMmControlPpiGuid                            ## PRODUCESdiff --git a/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmControlLib.h b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmControlLib.h
new file mode 100644
index 000000000000..b532dd13f373
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/SmmControlLib.h
@@ -0,0 +1,26 @@
+/** @file+  This is to publish the SMM Control Ppi instance.++  Copyright (c) 2019 - 2020, Intel Corporation. All rights reserved.<BR>+  SPDX-License-Identifier: BSD-2-Clause-Patent++**/+#ifndef _SMM_CONTROL_LIB_H_+#define _SMM_CONTROL_LIB_H_++/**+  This function is to install an SMM Control PPI+  - <b>Introduction</b> \n+    An API to install an instance of EFI_PEI_MM_CONTROL_PPI. This PPI provides a standard+    way for other modules to trigger software SMIs.++    @retval EFI_SUCCESS           - Ppi successfully started and installed.+    @retval EFI_NOT_FOUND         - Ppi can't be found.+    @retval EFI_OUT_OF_RESOURCES  - Ppi does not have enough resources to initialize the driver.+**/+EFI_STATUS+EFIAPI+PeiInstallSmmControlPpi (+  VOID+  );+#endifdiff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
index deefdc55b5d6..440c7d0255ce 100644
--- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
+++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
@@ -35,6 +35,10 @@
   #   SmmAccessLib|Include/Library/SmmAccessLib.h +  ## @libraryclass Provides services to trigger SMI+  #+  SmmControlLib|Include/Library/SmmControlLib.h+   ## @libraryclass Provides services to access config block   #   ConfigBlockLib|Include/Library/ConfigBlockLib.h-- 
2.37.2