@Chaganty, Rangasai V Is it fair to say that the build time address for ACPI BAR is the cross silicon design? I think it is and therefore we should move the PCD to IntelSiliconPkg. I agree with Benjamin that we don’t really want features depending on specific silicon packages. We want API definitions in IntelSiliconPkg. Regards, Isaac From: devel@edk2.groups.io On Behalf Of Benjamin Doron Sent: Monday, August 29, 2022 5:58 PM To: Oram, Isaac W Cc: devel@edk2.groups.io; Desimone, Nathaniel L ; Sinha, Ankit ; Ni, Ray ; Chaganty, Rangasai V Subject: Re: [edk2-devel][edk2-platforms][PATCH v1 2/5] Silicon/Intel: Port SmmControl protocol to PPI for S3 Right, but Kabylake has a different implementation that retrieves it from HW registers - PchAcpiBaseGet(). This is probably optional, there is a PCD, but it's in a different package scope. I don't know how to handle the Packages in the INF to keep this silicon package agnostic. For that matter, it might not really be a CFL plus shim, because Tigerlake, etc are different package DECs too. Best regards, Benjamin On Mon, 29 Aug 2022 at 19:17, Oram, Isaac W > wrote: I think that the shim lib might be overkill. PmcGetAcpiBase just resolves to PcdGet16 (PcdAcpiBaseAddress); I think that you should be able to use that PCD for any Intel chipset/silicon for the foreseeable future. I would prefer to see contents of sections in INF files indented, but it is a nit. Regards, Isaac -----Original Message----- From: devel@edk2.groups.io > On Behalf Of Benjamin Doron Sent: Monday, August 29, 2022 1:36 PM To: devel@edk2.groups.io Cc: Desimone, Nathaniel L >; Sinha, Ankit >; Ni, Ray >; Chaganty, Rangasai V >; Oram, Isaac W > Subject: [edk2-devel][edk2-platforms][PATCH v1 2/5] Silicon/Intel: Port SmmControl protocol to PPI for S3 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. As the registers are common across Intel platforms in the tree, while a helper function definition is not, implement a new library as a compatibility shim. Tested, working on Kabylake. Further testing required after the refactor for compatibility. Cc: Nate DeSimone > Cc: Ankit Sinha > Cc: Ray Ni > Cc: Rangasai V Chaganty > Cc: Isaac Oram > Signed-off-by: Benjamin Doron > --- .../BaseIntelCompatShimLibCfl.c | 24 ++ .../BaseIntelCompatShimLibCfl.inf | 24 ++ .../PeiSmmControlLib/PeiSmmControlLib.c | 309 ++++++++++++++++++ .../PeiSmmControlLib/PeiSmmControlLib.inf | 36 ++ .../Include/Library/IntelCompatShimLib.h | 20 ++ .../Include/Library/SmmControlLib.h | 26 ++ .../Intel/IntelSiliconPkg/IntelSiliconPkg.dec | 4 + .../BaseIntelCompatShimLibKbl.c | 29 ++ .../BaseIntelCompatShimLibKbl.inf | 24 ++ 9 files changed, 496 insertions(+) create mode 100644 Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseIntelCompatShimLibCfl/BaseIntelCompatShimLibCfl.c create mode 100644 Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseIntelCompatShimLibCfl/BaseIntelCompatShimLibCfl.inf 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/IntelCompatShimLib.h create mode 100644 Silicon/Intel/IntelSiliconPkg/Include/Library/SmmControlLib.h create mode 100644 Silicon/Intel/KabylakeSiliconPkg/Library/BaseIntelCompatShimLibKbl/BaseIntelCompatShimLibKbl.c create mode 100644 Silicon/Intel/KabylakeSiliconPkg/Library/BaseIntelCompatShimLibKbl/BaseIntelCompatShimLibKbl.inf diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseIntelCompatShimLibCfl/BaseIntelCompatShimLibCfl.c b/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseIntelCompatShimLibCfl/BaseIntelCompatShimLibCfl.c new file mode 100644 index 000000000000..5353126267e6 --- /dev/null +++ b/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseIntelCompatShimLibC +++ fl/BaseIntelCompatShimLibCfl.c @@ -0,0 +1,24 @@ +/** @file + A Coffeelake+ compatibility shim + + Copyright (c) 2022, Baruch Binyamin Doron
+ SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include +#include + +/** + Get PCH ACPI base address. + + @retval Address Address of PWRM base address. +**/ +UINT16 +EFIAPI +CompatShimGetAcpiBase ( + VOID + ) +{ + return PmcGetAcpiBase (); +} diff --git a/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseIntelCompatShimLibCfl/BaseIntelCompatShimLibCfl.inf b/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseIntelCompatShimLibCfl/BaseIntelCompatShimLibCfl.inf new file mode 100644 index 000000000000..48b071ed05ae --- /dev/null +++ b/Silicon/Intel/CoffeelakeSiliconPkg/Library/BaseIntelCompatShimLibC +++ fl/BaseIntelCompatShimLibCfl.inf @@ -0,0 +1,24 @@ +## @file +# Library description file for the Coffeelake+ compatibility shim # # +Copyright (c) 2022, Baruch Binyamin Doron
# +SPDX-License-Identifier: BSD-2-Clause-Patent # ## + +[Defines] +INF_VERSION = 0x00010017 +BASE_NAME = BaseIntelCompatShimLibCfl +FILE_GUID = 3D0BB32E-D328-4615-ADFC-782CECC68D53 +VERSION_STRING = 1.0 +MODULE_TYPE = BASE +LIBRARY_CLASS = IntelCompatShimLib + +[LibraryClasses] +PmcLib + +[Packages] +CoffeelakeSiliconPkg/SiPkg.dec + +[Sources] +BaseIntelCompatShimLibCfl.c 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..80c2c1be90b1 --- /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.
+ SPDX-License-Identifier: BSD-2-Clause-Patent++**/+#include +#include +#include +#include +#include +#include ++#include +#include ++#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 = CompatShimGetAcpiBase ();++ ///+ /// 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 = CompatShimGetAcpiBase ();++ ///+ /// 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+ - Introduction \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..92f2879d82ab --- /dev/null +++ b/Silicon/Intel/IntelSiliconPkg/Feature/SmmControl/Library/PeiSmmCon +++ trolLib/PeiSmmControlLib.inf @@ -0,0 +1,36 @@ +## @file+# Library description file for the SmmControl PPI+#+# +Copyright (c) 2019, Intel Corporation. All rights reserved.
+# +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]+IntelCompatShimLib+IoLib+DebugLib+Memo +ryAllocationLib+PeiServicesLib+++[Packages]+MdePkg/MdePkg.dec+IntelSili +conPkg/IntelSiliconPkg.dec+++[Sources]+PeiSmmControlLib.c+++[Ppis]+gEfi +PeiMmControlPpiGuid ## PRODUCESdiff --git +a/Silicon/Intel/IntelSiliconPkg/Include/Library/IntelCompatShimLib.h +b/Silicon/Intel/IntelSiliconPkg/Include/Library/IntelCompatShimLib.h new file mode 100644 index 000000000000..f8621d92e41a --- /dev/null +++ b/Silicon/Intel/IntelSiliconPkg/Include/Library/IntelCompatShimLib.h @@ -0,0 +1,20 @@ +/** @file + Library description file for compatibility shim + + Copyright (c) 2022, Baruch Binyamin Doron
+ SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include + +/** + Get PCH ACPI base address. + + @retval Address Address of PWRM base address. +**/ +UINT16 +EFIAPI +CompatShimGetAcpiBase ( + VOID + ); diff --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.
+ 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+ - Introduction \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 c36d130a0197..fc27b394d267 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.hdiff --git a/Silicon/Intel/KabylakeSiliconPkg/Library/BaseIntelCompatShimLibKbl/BaseIntelCompatShimLibKbl.c b/Silicon/Intel/KabylakeSiliconPkg/Library/BaseIntelCompatShimLibKbl/BaseIntelCompatShimLibKbl.c new file mode 100644 index 000000000000..573f67555aa3 --- /dev/null +++ b/Silicon/Intel/KabylakeSiliconPkg/Library/BaseIntelCompatShimLibKbl +++ /BaseIntelCompatShimLibKbl.c @@ -0,0 +1,29 @@ +/** @file + A Kabylake compatibility shim + + Copyright (c) 2022, Baruch Binyamin Doron
+ SPDX-License-Identifier: BSD-2-Clause-Patent + +**/ + +#include +#include + +/** + Get PCH ACPI base address. + + @retval Address Address of PWRM base address. +**/ +UINT16 +EFIAPI +CompatShimGetAcpiBase ( + VOID + ) +{ + UINT16 Address; + + Address = 0; + PchAcpiBaseGet (&Address); + + return Address; +} diff --git a/Silicon/Intel/KabylakeSiliconPkg/Library/BaseIntelCompatShimLibKbl/BaseIntelCompatShimLibKbl.inf b/Silicon/Intel/KabylakeSiliconPkg/Library/BaseIntelCompatShimLibKbl/BaseIntelCompatShimLibKbl.inf new file mode 100644 index 000000000000..af3e5a4726e6 --- /dev/null +++ b/Silicon/Intel/KabylakeSiliconPkg/Library/BaseIntelCompatShimLibKbl +++ /BaseIntelCompatShimLibKbl.inf @@ -0,0 +1,24 @@ +## @file +# Library description file for the Kabylake compatibility shim # # +Copyright (c) 2022, Baruch Binyamin Doron
# +SPDX-License-Identifier: BSD-2-Clause-Patent # ## + +[Defines] +INF_VERSION = 0x00010017 +BASE_NAME = BaseIntelCompatShimLibKbl +FILE_GUID = B4A2193E-CF3E-46E6-8617-49E48143B5AB +VERSION_STRING = 1.0 +MODULE_TYPE = BASE +LIBRARY_CLASS = IntelCompatShimLib + +[LibraryClasses] +PchCycleDecodingLib + +[Packages] +KabylakeSiliconPkg/SiPkg.dec + +[Sources] +BaseIntelCompatShimLibKbl.c -- 2.37.2 -=-=-=-=-=-= Groups.io Links: You receive all messages sent to this group. View/Reply Online (#92921): https://edk2.groups.io/g/devel/message/92921 Mute This Topic: https://groups.io/mt/93335519/1492418 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [isaac.w.oram@intel.com] -=-=-=-=-=-=