Thanks Jorden, for your reminder.

Hi Laszlo
Per my understand, you add NULL implementation for PiSmmCommunicationPpi, just 
because SmmLockBoxPeiLib depends on it. Right?
How  about we update SmmLockBoxPeiLib to call 
InternalRestoreLockBoxFromSmram(), if PiSmmCommunicationPpi is not located?
==========================
  Status = PeiServicesLocatePpi (
             &gEfiPeiSmmCommunicationPpiGuid,
             0,
             NULL,
             (VOID **)&SmmCommunicationPpi
             );
  if (EFI_ERROR (Status)) {
-    return EFI_NOT_STARTED;
+    return = InternalRestoreLockBoxFromSmram (Guid, Buffer, Length);
  }
==========================

SmmAccess is design to be silicon specific driver. I really do not want to let 
it touch generic infrastructure code like SmmCommunication.

Would you please do me a favor to try if it works?
If yes, we can update SmmLockBoxPeiLib.
If no, please let us know where is wrong, and we can try to figure out other 
better way.

Thank you
Yao Jiewen

-----Original Message-----
From: Justen, Jordan L 
Sent: Thursday, November 12, 2015 6:41 AM
To: Kinney, Michael D; Yao, Jiewen; Laszlo Ersek
Cc: edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v4 07/41] OvmfPkg: add PEIM for providing 
TSEG-as-SMRAM during PEI

On 2015-11-05 07:45:21, Kinney, Michael D wrote:
> 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.

Mike, Jiewen,

Can you guys review Laszlo's patches 7-9?

Obviously I'd like to avoid adding forked versions of any modules under OVMF if 
the goal had been to have common modules that should work for all platforms.

You guys probably know better if we ought to be able to use common modules 
instead. If the common modules don't work, then hopefully it is something we 
can fix.

Laszlo,

Maybe it would be good to note in the commit message the common module location 
that were forced to fork, and why you needed to fork it.

-Jordan

> 
> >-----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

Reply via email to