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

Reply via email to