On 11/12/15 04:33, Yao, Jiewen wrote:
> Thanks Jorden, for your reminder.
> 
> Hi Laszlo
> Per my understand, you add NULL implementation for PiSmmCommunicationPpi, 
> just because SmmLockBoxPeiLib depends on it. Right?

Yes.

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

Your suggestion works very well, thanks for it.

I just submitted it as a separate patch:

  [PATCH] MdeModulePkg: SmmLockBoxPeiLib: work without
          EFI_PEI_SMM_COMMUNICATION_PPI

  http://thread.gmane.org/gmane.comp.bios.edk2.devel/4283

If you think it is fine, I can commit it (please review it in its own
thread), and then I can drop the SmmCommunication PPI null
implementation from this OVMF patch. (I've tested S3 that way.)

Thanks!
Laszlo

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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to