Laszlo, Thanks for the details. Your implementation looks good.
I will discuss this further with Jiewen to see if it makes sense to provide additional implementations of this PPI that could be shared by multiple platforms. Thanks, Mike >-----Original Message----- >From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of >Laszlo Ersek >Sent: Thursday, November 05, 2015 3:15 AM >To: Kinney, Michael D; edk2-de...@ml01.01.org >Cc: Justen, Jordan L >Subject: Re: [edk2] [PATCH v4 07/41] OvmfPkg: add PEIM for providing TSEG- >as-SMRAM during PEI > >On 11/05/15 02:32, Kinney, Michael D wrote: >> Laszlo, >> >> For the EFI_PEI_COMMUNICATION_PPI, is there a reason you are not using >> UefiCpuPkg\PiSmmCommunication\PiSmmCommunicationPei.inf to produce >that PPI? > >Yes. > >When I wrote this patch originally on April 27th and the days after, >there was no EFI_PEI_COMMUNICATION_PPI implementation in edk2; neither >were plans known to add one. > >In addition, we had discussed the behavior of SmmLockBoxPeiLib at >length, in an off-list thread that you had been CC'd on. Please search >your archives for the message with the following metadata, and the >sub-thread rooted in it: > >Message-ID: <553f96cc.9050...@redhat.com> >Date: Tue, 28 Apr 2015 16:18:52 +0200 >From: Laszlo Ersek <ler...@redhat.com> >To: Yao, Jiewen <jiewen....@intel.com> >CC: Justen, Jordan L <jordan.l.jus...@intel.com>, > Zimmer, Vincent <vincent.zim...@intel.com>, > Kinney, Michael D <michael.d.kin...@intel.com>, > Paolo Bonzini <pbonz...@redhat.com>, > Gerd Hoffmann <kra...@redhat.com>, > Michael S. Tsirkin <m...@redhat.com> >Subject: Re: open-sourcing Intel's "IA32FamilyCpuPkg/PiSmmCpuDxeSmm" > >In that message, I asked: > >> In the whitepaper entitled "A Tour Beyond BIOS: Implementing S3 Resume >> with EDKII", page 25 says, under the heading "PEI Instance": >> >> The PEI instance of SmmLockboxLib has two ways to communicate with >> the LockBox in SMRAM. >> >> 1) It uses the SMM_COMMUNICATION PPI to communicate the SmmLockbox >> service provider, similar as DXE instance. >> 2) When the PEI instance is used before SMM ready, the >> SMM_COMMUNICATION PPI will return EFI_NOT_STARTED. In this case, >> PEI SmmLockBoxLib needs to search the SMRAM region directly to >> find LockBox content. >> >> The question is why a platform would choose option (1), ever. >> >> Namely, option (1) depends on additional drivers: >> - SMM_COMMUNICATION_PPI that actually works >> - (which might further depend on PEI_SMM_CONTROL_PPI) >> - a PEIM driver that implements the privileged (SMM) half of lockbox >> >> Whereas, option (2) is much simpler for the platform. It just needs to >> provide a minimal SMM_COMMUNICATION_PPI where it always returns >> EFI_NOT_STARTED. Then the lockbox library will do everything (and >> depend only on PEI_SMM_ACCESS_PPI). >> >> So, the question is: why would a platform ever pick (1), given that it >> is much more work to implement, for (as far as I can see) no benefit? > >And Jiewen replied (the same day), > >> Ah, I see. The only reason is that: A PEIM might need to restore >> lockbox data *before* SMM rebase happen. Then it has to use #2. >> >> A PEIM might need to restore lockbox data *after* SMM rebase happen. >> Then it has to use #1. (Scanning SMRAM does not work at that moment, >> which is enforced by SMRR) > >I couldn't see any grounds from Jiewen's answer to abandon option (2). >Option (2) actually worked, plus it was actually the *only* possibility >to make SmmLockboxLib work in OVMF (see above). > >I would like to stick with this approach. > >Thanks >Laszlo > >> >> Thanks, >> >> Mike >> >>> -----Original Message----- >>> From: Laszlo Ersek [mailto:ler...@redhat.com] >>> Sent: Tuesday, November 03, 2015 1:01 PM >>> To: edk2-de...@ml01.01.org >>> Cc: Kinney, Michael D; Justen, Jordan L >>> Subject: [PATCH v4 07/41] OvmfPkg: add PEIM for providing TSEG-as- >SMRAM >>> during PEI >>> >>> "MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf" is the >library >>> instance that implements the LockBoxLib library class with SMRAM access >>> for the PEI phase. >>> >>> Introduce a PEIM that produces the EFI_PEI_SMM_COMMUNICATION_PPI >and >>> PEI_SMM_ACCESS_PPI interfaces, enabling SmmLockBoxPeiLib to work. >>> >>> Said library instance can parse and access LockBox data itself (without >>> additional LockBox drivers) if the >>> EFI_PEI_SMM_COMMUNICATION_PPI.Communicate() function returns >>> EFI_NOT_STARTED to it. However it requires that >>> EFI_PEI_SMM_COMMUNICATION_PPI exist at least. Also, >>> PEI_SMM_ACCESS_PPI >>> must exist and work. >>> >>> The load / installation order of S3Resume2Pei and SmmAccessPei is >>> indifferent. SmmAccessPei produces the GUIDed HOB during its installation >>> (which happens during PEI), but S3Resume2Pei accesses the HOB only >when >>> the DXE IPL calls its S3RestoreConfig2 PPI member, as last act of PEI. >>> >>> MCH_SMRAM_D_LCK and MCH_ESMRAMC_T_EN are masked out the way >>> they are, in >>> SmmAccessPeiEntryPoint() and SmramAccessOpen() respectively, in order >to >>> prevent VS20xx from warning about the (otherwise fully intentional) >>> truncation in the UINT8 casts. (Warnings reported by Michael Kinney.) >>> >>> Cc: Michael Kinney <michael.d.kin...@intel.com> >>> Cc: Jordan Justen <jordan.l.jus...@intel.com> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Laszlo Ersek <ler...@redhat.com> >>> --- >>> >>> Notes: >>> v3: >>> - update bit-neg expressions to silence VS20xx warnings [Mike] >>> >>> OvmfPkg/OvmfPkgIa32.dsc | 6 + >>> OvmfPkg/OvmfPkgIa32X64.dsc | 6 + >>> OvmfPkg/OvmfPkgX64.dsc | 6 + >>> OvmfPkg/OvmfPkgIa32.fdf | 3 + >>> OvmfPkg/OvmfPkgIa32X64.fdf | 3 + >>> OvmfPkg/OvmfPkgX64.fdf | 3 + >>> OvmfPkg/SmmAccess/SmmAccessPei.inf | 70 +++ >>> OvmfPkg/SmmAccess/SmramInternal.h | 89 ++++ >>> OvmfPkg/SmmAccess/SmmAccessPei.c | 446 ++++++++++++++++++++ >>> OvmfPkg/SmmAccess/SmramInternal.c | 188 +++++++++ >>> 10 files changed, 820 insertions(+) >>> >>> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >>> index c6850ff..0b729ca 100644 >>> --- a/OvmfPkg/OvmfPkgIa32.dsc >>> +++ b/OvmfPkg/OvmfPkgIa32.dsc >>> @@ -445,6 +445,12 @@ [Components] >>> <LibraryClasses> >>> PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf >>> } >>> +!if $(SMM_REQUIRE) == TRUE >>> + OvmfPkg/SmmAccess/SmmAccessPei.inf { >>> + <LibraryClasses> >>> + PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf >>> + } >>> +!endif >>> >>> # >>> # DXE Phase modules >>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >>> index dd65bf9..7f672d9 100644 >>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >>> @@ -451,6 +451,12 @@ [Components.IA32] >>> <LibraryClasses> >>> PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf >>> } >>> +!if $(SMM_REQUIRE) == TRUE >>> + OvmfPkg/SmmAccess/SmmAccessPei.inf { >>> + <LibraryClasses> >>> + PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf >>> + } >>> +!endif >>> >>> [Components.X64] >>> # >>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >>> index 0de3c85..986c019 100644 >>> --- a/OvmfPkg/OvmfPkgX64.dsc >>> +++ b/OvmfPkg/OvmfPkgX64.dsc >>> @@ -450,6 +450,12 @@ [Components] >>> <LibraryClasses> >>> PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf >>> } >>> +!if $(SMM_REQUIRE) == TRUE >>> + OvmfPkg/SmmAccess/SmmAccessPei.inf { >>> + <LibraryClasses> >>> + PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf >>> + } >>> +!endif >>> >>> # >>> # DXE Phase modules >>> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf >>> index 44e4a92..650dab1 100644 >>> --- a/OvmfPkg/OvmfPkgIa32.fdf >>> +++ b/OvmfPkg/OvmfPkgIa32.fdf >>> @@ -171,6 +171,9 @@ [FV.PEIFV] >>> INF OvmfPkg/PlatformPei/PlatformPei.inf >>> INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf >>> INF UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf >>> +!if $(SMM_REQUIRE) == TRUE >>> +INF OvmfPkg/SmmAccess/SmmAccessPei.inf >>> +!endif >>> >>> >>> >################################################################# >>> ############### >>> >>> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf >>> index 67bfbe7..5830401 100644 >>> --- a/OvmfPkg/OvmfPkgIa32X64.fdf >>> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf >>> @@ -171,6 +171,9 @@ [FV.PEIFV] >>> INF OvmfPkg/PlatformPei/PlatformPei.inf >>> INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf >>> INF UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf >>> +!if $(SMM_REQUIRE) == TRUE >>> +INF OvmfPkg/SmmAccess/SmmAccessPei.inf >>> +!endif >>> >>> >>> >################################################################# >>> ############### >>> >>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf >>> index 6624789..9dd6171 100644 >>> --- a/OvmfPkg/OvmfPkgX64.fdf >>> +++ b/OvmfPkg/OvmfPkgX64.fdf >>> @@ -171,6 +171,9 @@ [FV.PEIFV] >>> INF OvmfPkg/PlatformPei/PlatformPei.inf >>> INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf >>> INF UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf >>> +!if $(SMM_REQUIRE) == TRUE >>> +INF OvmfPkg/SmmAccess/SmmAccessPei.inf >>> +!endif >>> >>> >>> >################################################################# >>> ############### >>> >>> diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.inf >>> b/OvmfPkg/SmmAccess/SmmAccessPei.inf >>> new file mode 100644 >>> index 0000000..94eb6c9 >>> --- /dev/null >>> +++ b/OvmfPkg/SmmAccess/SmmAccessPei.inf >>> @@ -0,0 +1,70 @@ >>> +## @file >>> +# A PEIM with the following responsibilities: >>> +# >>> +# - provide SMRAM access by producing PEI_SMM_ACCESS_PPI, >>> +# - provide a simple EFI_PEI_SMM_COMMUNICATION_PPI (always >returning >>> +# EFI_NOT_STARTED), >>> +# - verify & configure the Q35 TSEG in the entry point, >>> +# - set aside the SMM_S3_RESUME_STATE object at the bottom of TSEG, >and >>> expose >>> +# it via the gEfiAcpiVariableGuid GUIDed HOB. >>> +# >>> +# Copyright (C) 2013, 2015, Red Hat, Inc. >>> +# >>> +# This program and the accompanying materials are licensed and made >>> available >>> +# under the terms and conditions of the BSD License which accompanies >this >>> +# distribution. The full text of the license may be found at >>> +# http://opensource.org/licenses/bsd-license.php >>> +# >>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>> BASIS, WITHOUT >>> +# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >>> IMPLIED. >>> +# >>> +## >>> + >>> +[Defines] >>> + INF_VERSION = 0x00010005 >>> + BASE_NAME = SmmAccessPei >>> + FILE_GUID = 6C0E75B4-B0B9-44D1-8210-3377D7B4E066 >>> + MODULE_TYPE = PEIM >>> + VERSION_STRING = 1.0 >>> + ENTRY_POINT = SmmAccessPeiEntryPoint >>> + >>> +# >>> +# The following information is for reference only and not required by the >>> build tools. >>> +# >>> +# VALID_ARCHITECTURES = IA32 X64 >>> +# >>> + >>> +[Sources] >>> + SmmAccessPei.c >>> + SmramInternal.c >>> + >>> +[Packages] >>> + MdeModulePkg/MdeModulePkg.dec >>> + MdePkg/MdePkg.dec >>> + OvmfPkg/OvmfPkg.dec >>> + >>> +[Guids] >>> + gEfiAcpiVariableGuid >>> + >>> +[LibraryClasses] >>> + BaseMemoryLib >>> + DebugLib >>> + HobLib >>> + IoLib >>> + PcdLib >>> + PciLib >>> + PeiServicesLib >>> + PeimEntryPoint >>> + >>> +[FeaturePcd] >>> + gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire >>> + >>> +[FixedPcd] >>> + gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes >>> + >>> +[Ppis] >>> + gEfiPeiSmmCommunicationPpiGuid # PPI ALWAYS_PRODUCED >>> + gPeiSmmAccessPpiGuid # PPI ALWAYS_PRODUCED >>> + >>> +[Depex] >>> + TRUE >>> diff --git a/OvmfPkg/SmmAccess/SmramInternal.h >>> b/OvmfPkg/SmmAccess/SmramInternal.h >>> new file mode 100644 >>> index 0000000..4e9ac05 >>> --- /dev/null >>> +++ b/OvmfPkg/SmmAccess/SmramInternal.h >>> @@ -0,0 +1,89 @@ >>> +/** @file >>> + >>> + Functions and types shared by the SMM accessor PEI and DXE modules. >>> + >>> + Copyright (C) 2015, Red Hat, Inc. >>> + >>> + This program and the accompanying materials are licensed and made >>> available >>> + under the terms and conditions of the BSD License which accompanies >this >>> + distribution. The full text of the license may be found at >>> + http://opensource.org/licenses/bsd-license.php >>> + >>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>> BASIS, WITHOUT >>> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >>> IMPLIED. >>> + >>> +**/ >>> + >>> +#include <Pi/PiMultiPhase.h> >>> + >>> +// >>> +// We'll have two SMRAM ranges. >>> +// >>> +// The first is a tiny one that hosts an SMM_S3_RESUME_STATE object, to >be >>> +// filled in by the CPU SMM driver during normal boot, for the PEI >instance >>> of >>> +// the LockBox library (which will rely on the object during S3 resume). >>> +// >>> +// The other SMRAM range is the main one, for the SMM core and the >SMM >>> drivers. >>> +// >>> +typedef enum { >>> + DescIdxSmmS3ResumeState = 0, >>> + DescIdxMain = 1, >>> + DescIdxCount = 2 >>> +} DESCRIPTOR_INDEX; >>> + >>> +/** >>> + Read the MCH_SMRAM and ESMRAMC registers, and update the >LockState >>> and >>> + OpenState fields in the PEI_SMM_ACCESS_PPI / >>> EFI_SMM_ACCESS2_PROTOCOL object, >>> + from the D_LCK and T_EN bits. >>> + >>> + PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member >>> functions can rely on >>> + the LockState and OpenState fields being up-to-date on entry, and they >>> need >>> + to restore the same invariant on exit, if they touch the bits in >>> question. >>> + >>> + @param[out] LockState Reflects the D_LCK bit on output; TRUE iff >SMRAM >>> is >>> + locked. >>> + @param[out] OpenState Reflects the inverse of the T_EN bit on output; >>> TRUE >>> + iff SMRAM is open. >>> +**/ >>> +VOID >>> +GetStates ( >>> + OUT BOOLEAN *LockState, >>> + OUT BOOLEAN *OpenState >>> + ); >>> + >>> +// >>> +// The functions below follow the PEI_SMM_ACCESS_PPI and >>> +// EFI_SMM_ACCESS2_PROTOCOL member declarations. The PeiServices >and >>> This >>> +// pointers are removed (TSEG doesn't depend on them), and so is the >>> +// DescriptorIndex parameter (TSEG doesn't support range-wise locking). >>> +// >>> +// The LockState and OpenState members that are common to both >>> +// PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken >and >>> updated in >>> +// isolation from the rest of the (non-shared) members. >>> +// >>> + >>> +EFI_STATUS >>> +SmramAccessOpen ( >>> + OUT BOOLEAN *LockState, >>> + OUT BOOLEAN *OpenState >>> + ); >>> + >>> +EFI_STATUS >>> +SmramAccessClose ( >>> + OUT BOOLEAN *LockState, >>> + OUT BOOLEAN *OpenState >>> + ); >>> + >>> +EFI_STATUS >>> +SmramAccessLock ( >>> + OUT BOOLEAN *LockState, >>> + IN OUT BOOLEAN *OpenState >>> + ); >>> + >>> +EFI_STATUS >>> +SmramAccessGetCapabilities ( >>> + IN BOOLEAN LockState, >>> + IN BOOLEAN OpenState, >>> + IN OUT UINTN *SmramMapSize, >>> + IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap >>> + ); >>> diff --git a/OvmfPkg/SmmAccess/SmmAccessPei.c >>> b/OvmfPkg/SmmAccess/SmmAccessPei.c >>> new file mode 100644 >>> index 0000000..8de4e4f >>> --- /dev/null >>> +++ b/OvmfPkg/SmmAccess/SmmAccessPei.c >>> @@ -0,0 +1,446 @@ >>> +/** @file >>> + >>> + A PEIM with the following responsibilities: >>> + >>> + - verify & configure the Q35 TSEG in the entry point, >>> + - provide SMRAM access by producing PEI_SMM_ACCESS_PPI, >>> + - provide a simple EFI_PEI_SMM_COMMUNICATION_PPI (always >returning >>> + EFI_NOT_STARTED), >>> + - set aside the SMM_S3_RESUME_STATE object at the bottom of TSEG, >and >>> expose >>> + it via the gEfiAcpiVariableGuid GUID HOB. >>> + >>> + This PEIM runs from RAM, so we can write to variables with static >storage >>> + duration. >>> + >>> + Copyright (C) 2013, 2015, Red Hat, Inc.<BR> >>> + Copyright (c) 2010, Intel Corporation. All rights reserved.<BR> >>> + >>> + This program and the accompanying materials are licensed and made >>> available >>> + under the terms and conditions of the BSD License which accompanies >this >>> + distribution. The full text of the license may be found at >>> + http://opensource.org/licenses/bsd-license.php >>> + >>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>> BASIS, WITHOUT >>> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >>> IMPLIED. >>> + >>> +**/ >>> + >>> +#include <Guid/AcpiS3Context.h> >>> +#include <Library/BaseMemoryLib.h> >>> +#include <Library/DebugLib.h> >>> +#include <Library/HobLib.h> >>> +#include <Library/IoLib.h> >>> +#include <Library/PcdLib.h> >>> +#include <Library/PciLib.h> >>> +#include <Library/PeiServicesLib.h> >>> +#include <Ppi/SmmAccess.h> >>> +#include <Ppi/SmmCommunication.h> >>> + >>> +#include <OvmfPlatforms.h> >>> + >>> +#include "SmramInternal.h" >>> + >>> +// >>> +// EFI_PEI_SMM_COMMUNICATION_PPI implementation. >>> +// >>> + >>> +/** >>> + Communicates with a registered handler. >>> + >>> + This function provides a service to send and receive messages from a >>> + registered UEFI service. >>> + >>> + @param[in] This The EFI_PEI_SMM_COMMUNICATION_PPI >>> instance. >>> + @param[in] CommBuffer A pointer to the buffer to convey into >>> SMRAM. >>> + @param[in] CommSize The size of the data buffer being passed >>> in.On >>> + exit, the size of data being returned. >>> Zero if >>> + the handler does not wish to reply with >>> any >>> + data. >>> + >>> + @retval EFI_SUCCESS The message was successfully posted. >>> + @retval EFI_INVALID_PARAMETER The CommBuffer was NULL. >>> +**/ >>> +STATIC >>> +EFI_STATUS >>> +EFIAPI >>> +SmmCommunicate ( >>> + IN CONST EFI_PEI_SMM_COMMUNICATION_PPI *This, >>> + IN OUT VOID *CommBuffer, >>> + IN OUT UINTN *CommSize >>> + ) >>> +{ >>> + // >>> + // This return status is not documented, but it causes >>> + // "MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c" to >look >>> for the >>> + // LockBox in SMRAM itself. It allows us to avoid implementing >>> + // EFI_PEI_SMM_COMMUNICATION_PPI with real functionality, plus we >can >>> + // completely skip PEI_SMM_CONTROL_PPI (which the former would >>> arguably rely >>> + // on). >>> + // >>> + return EFI_NOT_STARTED; >>> +} >>> + >>> +STATIC EFI_PEI_SMM_COMMUNICATION_PPI mCommunication = { >>> + &SmmCommunicate >>> +}; >>> + >>> + >>> +// >>> +// PEI_SMM_ACCESS_PPI implementation. >>> +// >>> + >>> +/** >>> + Opens the SMRAM area to be accessible by a PEIM driver. >>> + >>> + This function "opens" SMRAM so that it is visible while not inside of >SMM. >>> + The function should return EFI_UNSUPPORTED if the hardware does not >>> support >>> + hiding of SMRAM. The function should return EFI_DEVICE_ERROR if the >>> SMRAM >>> + configuration is locked. >>> + >>> + @param PeiServices General purpose services available to >>> every >>> + PEIM. >>> + @param This The pointer to the SMM Access Interface. >>> + @param DescriptorIndex The region of SMRAM to Open. >>> + >>> + @retval EFI_SUCCESS The region was successfully opened. >>> + @retval EFI_DEVICE_ERROR The region could not be opened because >>> locked >>> + by chipset. >>> + @retval EFI_INVALID_PARAMETER The descriptor index was out of >bounds. >>> + >>> +**/ >>> +STATIC >>> +EFI_STATUS >>> +EFIAPI >>> +SmmAccessPeiOpen ( >>> + IN EFI_PEI_SERVICES **PeiServices, >>> + IN PEI_SMM_ACCESS_PPI *This, >>> + IN UINTN DescriptorIndex >>> + ) >>> +{ >>> + if (DescriptorIndex >= DescIdxCount) { >>> + return EFI_INVALID_PARAMETER; >>> + } >>> + >>> + // >>> + // According to current practice, DescriptorIndex is not considered at >>> all, >>> + // beyond validating it. >>> + // >>> + return SmramAccessOpen (&This->LockState, &This->OpenState); >>> +} >>> + >>> +/** >>> + Inhibits access to the SMRAM. >>> + >>> + This function "closes" SMRAM so that it is not visible while outside of >SMM. >>> + The function should return EFI_UNSUPPORTED if the hardware does not >>> support >>> + hiding of SMRAM. >>> + >>> + @param PeiServices General purpose services available to >>> every >>> + PEIM. >>> + @param This The pointer to the SMM Access Interface. >>> + @param DescriptorIndex The region of SMRAM to Close. >>> + >>> + @retval EFI_SUCCESS The region was successfully closed. >>> + @retval EFI_DEVICE_ERROR The region could not be closed because >>> + locked by chipset. >>> + @retval EFI_INVALID_PARAMETER The descriptor index was out of >>> bounds. >>> + >>> +**/ >>> +STATIC >>> +EFI_STATUS >>> +EFIAPI >>> +SmmAccessPeiClose ( >>> + IN EFI_PEI_SERVICES **PeiServices, >>> + IN PEI_SMM_ACCESS_PPI *This, >>> + IN UINTN DescriptorIndex >>> + ) >>> +{ >>> + if (DescriptorIndex >= DescIdxCount) { >>> + return EFI_INVALID_PARAMETER; >>> + } >>> + >>> + // >>> + // According to current practice, DescriptorIndex is not considered at >>> all, >>> + // beyond validating it. >>> + // >>> + return SmramAccessClose (&This->LockState, &This->OpenState); >>> +} >>> + >>> +/** >>> + Inhibits access to the SMRAM. >>> + >>> + This function prohibits access to the SMRAM region. This function is >>> usually >>> + implemented such that it is a write-once operation. >>> + >>> + @param PeiServices General purpose services available to >>> every >>> + PEIM. >>> + @param This The pointer to the SMM Access Interface. >>> + @param DescriptorIndex The region of SMRAM to Close. >>> + >>> + @retval EFI_SUCCESS The region was successfully locked. >>> + @retval EFI_DEVICE_ERROR The region could not be locked because >at >>> + least one range is still open. >>> + @retval EFI_INVALID_PARAMETER The descriptor index was out of >bounds. >>> + >>> +**/ >>> +STATIC >>> +EFI_STATUS >>> +EFIAPI >>> +SmmAccessPeiLock ( >>> + IN EFI_PEI_SERVICES **PeiServices, >>> + IN PEI_SMM_ACCESS_PPI *This, >>> + IN UINTN DescriptorIndex >>> + ) >>> +{ >>> + if (DescriptorIndex >= DescIdxCount) { >>> + return EFI_INVALID_PARAMETER; >>> + } >>> + >>> + // >>> + // According to current practice, DescriptorIndex is not considered at >>> all, >>> + // beyond validating it. >>> + // >>> + return SmramAccessLock (&This->LockState, &This->OpenState); >>> +} >>> + >>> +/** >>> + Queries the memory controller for the possible regions that will support >>> + SMRAM. >>> + >>> + @param PeiServices General purpose services available to every >>> + PEIM. >>> + @param This The pointer to the SmmAccessPpi Interface. >>> + @param SmramMapSize The pointer to the variable containing >size >>> of >>> + the buffer to contain the description >>> + information. >>> + @param SmramMap The buffer containing the data describing >the >>> + Smram region descriptors. >>> + >>> + @retval EFI_BUFFER_TOO_SMALL The user did not provide a sufficient >>> buffer. >>> + @retval EFI_SUCCESS The user provided a sufficiently-sized >>> buffer. >>> + >>> +**/ >>> +STATIC >>> +EFI_STATUS >>> +EFIAPI >>> +SmmAccessPeiGetCapabilities ( >>> + IN EFI_PEI_SERVICES **PeiServices, >>> + IN PEI_SMM_ACCESS_PPI *This, >>> + IN OUT UINTN *SmramMapSize, >>> + IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap >>> + ) >>> +{ >>> + return SmramAccessGetCapabilities (This->LockState, This->OpenState, >>> + SmramMapSize, SmramMap); >>> +} >>> + >>> +// >>> +// LockState and OpenState will be filled in by the entry point. >>> +// >>> +STATIC PEI_SMM_ACCESS_PPI mAccess = { >>> + &SmmAccessPeiOpen, >>> + &SmmAccessPeiClose, >>> + &SmmAccessPeiLock, >>> + &SmmAccessPeiGetCapabilities >>> +}; >>> + >>> + >>> +STATIC EFI_PEI_PPI_DESCRIPTOR mPpiList[] = { >>> + { >>> + EFI_PEI_PPI_DESCRIPTOR_PPI, >>> + &gEfiPeiSmmCommunicationPpiGuid, &mCommunication >>> + }, >>> + { >>> + EFI_PEI_PPI_DESCRIPTOR_PPI | >EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST, >>> + &gPeiSmmAccessPpiGuid, &mAccess >>> + } >>> +}; >>> + >>> + >>> +// >>> +// Utility functions. >>> +// >>> +STATIC >>> +UINT8 >>> +CmosRead8 ( >>> + IN UINT8 Index >>> + ) >>> +{ >>> + IoWrite8 (0x70, Index); >>> + return IoRead8 (0x71); >>> +} >>> + >>> +STATIC >>> +UINT32 >>> +GetSystemMemorySizeBelow4gb ( >>> + VOID >>> + ) >>> +{ >>> + UINT32 Cmos0x34; >>> + UINT32 Cmos0x35; >>> + >>> + Cmos0x34 = CmosRead8 (0x34); >>> + Cmos0x35 = CmosRead8 (0x35); >>> + >>> + return ((Cmos0x35 << 8 | Cmos0x34) << 16) + SIZE_16MB; >>> +} >>> + >>> + >>> +// >>> +// Entry point of this driver. >>> +// >>> +EFI_STATUS >>> +EFIAPI >>> +SmmAccessPeiEntryPoint ( >>> + IN EFI_PEI_FILE_HANDLE FileHandle, >>> + IN CONST EFI_PEI_SERVICES **PeiServices >>> + ) >>> +{ >>> + UINT16 HostBridgeDevId; >>> + UINT8 EsmramcVal; >>> + UINT8 RegMask8; >>> + UINT32 TopOfLowRam, TopOfLowRamMb; >>> + EFI_STATUS Status; >>> + UINTN SmramMapSize; >>> + EFI_SMRAM_DESCRIPTOR SmramMap[DescIdxCount]; >>> + VOID *GuidHob; >>> + >>> + // >>> + // This module should only be included if SMRAM support is required. >>> + // >>> + ASSERT (FeaturePcdGet (PcdSmmSmramRequire)); >>> + >>> + // >>> + // Verify if we're running on a Q35 machine type. >>> + // >>> + HostBridgeDevId = PciRead16 (OVMF_HOSTBRIDGE_DID); >>> + if (HostBridgeDevId != INTEL_Q35_MCH_DEVICE_ID) { >>> + DEBUG ((EFI_D_ERROR, "%a: no SMRAM with host bridge DID=0x%04x; >>> only " >>> + "DID=0x%04x (Q35) is supported\n", __FUNCTION__, HostBridgeDevId, >>> + INTEL_Q35_MCH_DEVICE_ID)); >>> + goto WrongConfig; >>> + } >>> + >>> + // >>> + // Confirm if QEMU supports SMRAM. >>> + // >>> + // With no support for it, the ESMRAMC (Extended System Management >>> RAM >>> + // Control) register reads as zero. If there is support, the cache-enable >>> + // bits are hard-coded as 1 by QEMU. >>> + // >>> + EsmramcVal = PciRead8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC)); >>> + RegMask8 = MCH_ESMRAMC_SM_CACHE | MCH_ESMRAMC_SM_L1 | >>> MCH_ESMRAMC_SM_L2; >>> + if ((EsmramcVal & RegMask8) != RegMask8) { >>> + DEBUG ((EFI_D_ERROR, "%a: this Q35 implementation lacks SMRAM\n", >>> + __FUNCTION__)); >>> + goto WrongConfig; >>> + } >>> + >>> + TopOfLowRam = GetSystemMemorySizeBelow4gb (); >>> + ASSERT ((TopOfLowRam & (SIZE_1MB - 1)) == 0); >>> + TopOfLowRamMb = TopOfLowRam >> 20; >>> + >>> + // >>> + // Some of the following registers are no-ops for QEMU at the moment, >but >>> it >>> + // is recommended to set them correctly, since the ESMRAMC that we >>> ultimately >>> + // care about is in the same set of registers. >>> + // >>> + // First, we disable the integrated VGA, and set both the GTT Graphics >>> Memory >>> + // Size and the Graphics Mode Select memory pre-allocation fields to >zero. >>> + // This takes just one write to the Graphics Control Register. >>> + // >>> + PciWrite16 (DRAMC_REGISTER_Q35 (MCH_GGC), MCH_GGC_IVD); >>> + >>> + // >>> + // Set Top of Low Usable DRAM. >>> + // >>> + PciWrite16 (DRAMC_REGISTER_Q35 (MCH_TOLUD), >>> + (UINT16)(TopOfLowRamMb << MCH_TOLUD_MB_SHIFT)); >>> + >>> + // >>> + // Given the zero graphics memory sizes configured above, set the >>> + // graphics-related stolen memory bases to the same as TOLUD. >>> + // >>> + PciWrite32 (DRAMC_REGISTER_Q35 (MCH_GBSM), >>> + TopOfLowRamMb << MCH_GBSM_MB_SHIFT); >>> + PciWrite32 (DRAMC_REGISTER_Q35 (MCH_BGSM), >>> + TopOfLowRamMb << MCH_BGSM_MB_SHIFT); >>> + >>> + // >>> + // Set TSEG Memory Base. >>> + // >>> + PciWrite32 (DRAMC_REGISTER_Q35 (MCH_TSEGMB), >>> + (TopOfLowRamMb - FixedPcdGet8 (PcdQ35TsegMbytes)) << >>> MCH_TSEGMB_MB_SHIFT); >>> + >>> + // >>> + // Set TSEG size, and disable TSEG visibility outside of SMM. Note that >the >>> + // T_EN bit has inverse meaning; when T_EN is set, then TSEG visibility >>> is >>> + // *restricted* to SMM. >>> + // >>> + EsmramcVal &= ~(UINT32)MCH_ESMRAMC_TSEG_MASK; >>> + EsmramcVal |= FixedPcdGet8 (PcdQ35TsegMbytes) == 8 ? >>> MCH_ESMRAMC_TSEG_8MB : >>> + FixedPcdGet8 (PcdQ35TsegMbytes) == 2 ? >>> MCH_ESMRAMC_TSEG_2MB : >>> + MCH_ESMRAMC_TSEG_1MB; >>> + EsmramcVal |= MCH_ESMRAMC_T_EN; >>> + PciWrite8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC), EsmramcVal); >>> + >>> + // >>> + // TSEG should be closed (see above), but unlocked, initially. Set >>> G_SMRAME >>> + // (Global SMRAM Enable) too, as both D_LCK and T_EN depend on it. >>> + // >>> + PciAndThenOr8 (DRAMC_REGISTER_Q35 (MCH_SMRAM), >>> + (UINT8)((~(UINT32)MCH_SMRAM_D_LCK) & 0xff), >>> MCH_SMRAM_G_SMRAME); >>> + >>> + // >>> + // Create the GUID HOB and point it to the first SMRAM range. >>> + // >>> + GetStates (&mAccess.LockState, &mAccess.OpenState); >>> + SmramMapSize = sizeof SmramMap; >>> + Status = SmramAccessGetCapabilities (mAccess.LockState, >>> mAccess.OpenState, >>> + &SmramMapSize, SmramMap); >>> + ASSERT_EFI_ERROR (Status); >>> + >>> + DEBUG_CODE_BEGIN (); >>> + { >>> + UINTN Count; >>> + UINTN Idx; >>> + >>> + Count = SmramMapSize / sizeof SmramMap[0]; >>> + DEBUG ((EFI_D_VERBOSE, "%a: SMRAM map follows, %d entries\n", >>> __FUNCTION__, >>> + (INT32)Count)); >>> + DEBUG ((EFI_D_VERBOSE, "% 20a % 20a % 20a % 20a\n", >>> "PhysicalStart(0x)", >>> + "PhysicalSize(0x)", "CpuStart(0x)", "RegionState(0x)")); >>> + for (Idx = 0; Idx < Count; ++Idx) { >>> + DEBUG ((EFI_D_VERBOSE, "% 20Lx % 20Lx % 20Lx % 20Lx\n", >>> + SmramMap[Idx].PhysicalStart, SmramMap[Idx].PhysicalSize, >>> + SmramMap[Idx].CpuStart, SmramMap[Idx].RegionState)); >>> + } >>> + } >>> + DEBUG_CODE_END (); >>> + >>> + GuidHob = BuildGuidHob (&gEfiAcpiVariableGuid, >>> + sizeof SmramMap[DescIdxSmmS3ResumeState]); >>> + if (GuidHob == NULL) { >>> + return EFI_OUT_OF_RESOURCES; >>> + } >>> + >>> + CopyMem (GuidHob, &SmramMap[DescIdxSmmS3ResumeState], >>> + sizeof SmramMap[DescIdxSmmS3ResumeState]); >>> + >>> + // >>> + // We're done. The next step should succeed, but even if it fails, we >>> can't >>> + // roll back the above BuildGuidHob() allocation, because PEI doesn't >>> support >>> + // releasing memory. >>> + // >>> + return PeiServicesInstallPpi (mPpiList); >>> + >>> +WrongConfig: >>> + // >>> + // We really don't want to continue in this case. >>> + // >>> + // _ASSERT() is never compiled out, and it respects >PcdDebugPropertyMask >>> (ie. >>> + // prevent further execution with CPU breakpoint vs. dead loop). >>> + // >>> + _ASSERT (FALSE); >>> + return EFI_UNSUPPORTED; >>> +} >>> diff --git a/OvmfPkg/SmmAccess/SmramInternal.c >>> b/OvmfPkg/SmmAccess/SmramInternal.c >>> new file mode 100644 >>> index 0000000..c3267ca >>> --- /dev/null >>> +++ b/OvmfPkg/SmmAccess/SmramInternal.c >>> @@ -0,0 +1,188 @@ >>> +/** @file >>> + >>> + Functions and types shared by the SMM accessor PEI and DXE modules. >>> + >>> + Copyright (C) 2015, Red Hat, Inc. >>> + >>> + This program and the accompanying materials are licensed and made >>> available >>> + under the terms and conditions of the BSD License which accompanies >this >>> + distribution. The full text of the license may be found at >>> + http://opensource.org/licenses/bsd-license.php >>> + >>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" >>> BASIS, WITHOUT >>> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR >>> IMPLIED. >>> + >>> +**/ >>> + >>> +#include <Guid/AcpiS3Context.h> >>> +#include <IndustryStandard/Q35MchIch9.h> >>> +#include <Library/DebugLib.h> >>> +#include <Library/PciLib.h> >>> + >>> +#include "SmramInternal.h" >>> + >>> +/** >>> + Read the MCH_SMRAM and ESMRAMC registers, and update the >LockState >>> and >>> + OpenState fields in the PEI_SMM_ACCESS_PPI / >>> EFI_SMM_ACCESS2_PROTOCOL object, >>> + from the D_LCK and T_EN bits. >>> + >>> + PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL member >>> functions can rely on >>> + the LockState and OpenState fields being up-to-date on entry, and they >>> need >>> + to restore the same invariant on exit, if they touch the bits in >>> question. >>> + >>> + @param[out] LockState Reflects the D_LCK bit on output; TRUE iff >SMRAM >>> is >>> + locked. >>> + @param[out] OpenState Reflects the inverse of the T_EN bit on output; >>> TRUE >>> + iff SMRAM is open. >>> +**/ >>> +VOID >>> +GetStates ( >>> + OUT BOOLEAN *LockState, >>> + OUT BOOLEAN *OpenState >>> +) >>> +{ >>> + UINT8 SmramVal, EsmramcVal; >>> + >>> + SmramVal = PciRead8 (DRAMC_REGISTER_Q35 (MCH_SMRAM)); >>> + EsmramcVal = PciRead8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC)); >>> + >>> + *LockState = !!(SmramVal & MCH_SMRAM_D_LCK); >>> + *OpenState = !(EsmramcVal & MCH_ESMRAMC_T_EN); >>> +} >>> + >>> +// >>> +// The functions below follow the PEI_SMM_ACCESS_PPI and >>> +// EFI_SMM_ACCESS2_PROTOCOL member declarations. The PeiServices >and >>> This >>> +// pointers are removed (TSEG doesn't depend on them), and so is the >>> +// DescriptorIndex parameter (TSEG doesn't support range-wise locking). >>> +// >>> +// The LockState and OpenState members that are common to both >>> +// PEI_SMM_ACCESS_PPI and EFI_SMM_ACCESS2_PROTOCOL are taken >and >>> updated in >>> +// isolation from the rest of the (non-shared) members. >>> +// >>> + >>> +EFI_STATUS >>> +SmramAccessOpen ( >>> + OUT BOOLEAN *LockState, >>> + OUT BOOLEAN *OpenState >>> + ) >>> +{ >>> + // >>> + // Open TSEG by clearing T_EN. >>> + // >>> + PciAnd8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC), >>> + (UINT8)((~(UINT32)MCH_ESMRAMC_T_EN) & 0xff)); >>> + >>> + GetStates (LockState, OpenState); >>> + if (!*OpenState) { >>> + return EFI_DEVICE_ERROR; >>> + } >>> + return EFI_SUCCESS; >>> +} >>> + >>> +EFI_STATUS >>> +SmramAccessClose ( >>> + OUT BOOLEAN *LockState, >>> + OUT BOOLEAN *OpenState >>> + ) >>> +{ >>> + // >>> + // Close TSEG by setting T_EN. >>> + // >>> + PciOr8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC), >MCH_ESMRAMC_T_EN); >>> + >>> + GetStates (LockState, OpenState); >>> + if (*OpenState) { >>> + return EFI_DEVICE_ERROR; >>> + } >>> + return EFI_SUCCESS; >>> +} >>> + >>> +EFI_STATUS >>> +SmramAccessLock ( >>> + OUT BOOLEAN *LockState, >>> + IN OUT BOOLEAN *OpenState >>> + ) >>> +{ >>> + if (*OpenState) { >>> + return EFI_DEVICE_ERROR; >>> + } >>> + >>> + // >>> + // Close & lock TSEG by setting T_EN and D_LCK. >>> + // >>> + PciOr8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC), >MCH_ESMRAMC_T_EN); >>> + PciOr8 (DRAMC_REGISTER_Q35 (MCH_SMRAM), MCH_SMRAM_D_LCK); >>> + >>> + GetStates (LockState, OpenState); >>> + if (*OpenState || !*LockState) { >>> + return EFI_DEVICE_ERROR; >>> + } >>> + return EFI_SUCCESS; >>> +} >>> + >>> +EFI_STATUS >>> +SmramAccessGetCapabilities ( >>> + IN BOOLEAN LockState, >>> + IN BOOLEAN OpenState, >>> + IN OUT UINTN *SmramMapSize, >>> + IN OUT EFI_SMRAM_DESCRIPTOR *SmramMap >>> + ) >>> +{ >>> + UINTN OriginalSize; >>> + UINT32 TsegMemoryBaseMb, TsegMemoryBase; >>> + UINT64 CommonRegionState; >>> + UINT8 TsegSizeBits; >>> + >>> + OriginalSize = *SmramMapSize; >>> + *SmramMapSize = DescIdxCount * sizeof *SmramMap; >>> + if (OriginalSize < *SmramMapSize) { >>> + return EFI_BUFFER_TOO_SMALL; >>> + } >>> + >>> + // >>> + // Read the TSEG Memory Base register. >>> + // >>> + TsegMemoryBaseMb = PciRead32 (DRAMC_REGISTER_Q35 >(MCH_TSEGMB)); >>> + TsegMemoryBase = (TsegMemoryBaseMb >> MCH_TSEGMB_MB_SHIFT) ><< >>> 20; >>> + >>> + // >>> + // Precompute the region state bits that will be set for all regions. >>> + // >>> + CommonRegionState = (OpenState ? EFI_SMRAM_OPEN : >>> EFI_SMRAM_CLOSED) | >>> + (LockState ? EFI_SMRAM_LOCKED : 0) | >>> + EFI_CACHEABLE; >>> + >>> + // >>> + // The first region hosts an SMM_S3_RESUME_STATE object. It is located >at >>> the >>> + // start of TSEG. We round up the size to whole pages, and we report it >as >>> + // EFI_ALLOCATED, so that the SMM_CORE stays away from it. >>> + // >>> + SmramMap[DescIdxSmmS3ResumeState].PhysicalStart = >TsegMemoryBase; >>> + SmramMap[DescIdxSmmS3ResumeState].CpuStart = >TsegMemoryBase; >>> + SmramMap[DescIdxSmmS3ResumeState].PhysicalSize = >>> + EFI_PAGES_TO_SIZE (EFI_SIZE_TO_PAGES (sizeof >>> (SMM_S3_RESUME_STATE))); >>> + SmramMap[DescIdxSmmS3ResumeState].RegionState = >>> + CommonRegionState | EFI_ALLOCATED; >>> + >>> + // >>> + // Get the TSEG size bits from the ESMRAMC register. >>> + // >>> + TsegSizeBits = PciRead8 (DRAMC_REGISTER_Q35 (MCH_ESMRAMC)) & >>> + MCH_ESMRAMC_TSEG_MASK; >>> + >>> + // >>> + // The second region is the main one, following the first. >>> + // >>> + SmramMap[DescIdxMain].PhysicalStart = >>> + SmramMap[DescIdxSmmS3ResumeState].PhysicalStart + >>> + SmramMap[DescIdxSmmS3ResumeState].PhysicalSize; >>> + SmramMap[DescIdxMain].CpuStart = >>> SmramMap[DescIdxMain].PhysicalStart; >>> + SmramMap[DescIdxMain].PhysicalSize = >>> + (TsegSizeBits == MCH_ESMRAMC_TSEG_8MB ? SIZE_8MB : >>> + TsegSizeBits == MCH_ESMRAMC_TSEG_2MB ? SIZE_2MB : >>> + SIZE_1MB) - SmramMap[DescIdxSmmS3ResumeState].PhysicalSize; >>> + SmramMap[DescIdxMain].RegionState = CommonRegionState; >>> + >>> + return EFI_SUCCESS; >>> +} >>> -- >>> 1.8.3.1 >>> >> > >_______________________________________________ >edk2-devel mailing list >edk2-devel@lists.01.org >https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
