Re: [edk2-devel] [PATCH v4 07/40] UefiCpuPkg: Implement library support for VMGEXIT

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 2/13/20 6:42 PM, Dong, Eric wrote:
> Hi Tom,
> 
> For this patch, I found the function implementations not have function header comments for them. This is not follow edk2 coding style, please update them.

Yup, missed them, will do.

Thanks,
Tom

> 
> Thanks,
> Eric
> 
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Lendacky, Thomas
> Sent: Wednesday, February 5, 2020 7:01 AM
> To: devel@edk2.groups.io
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Laszlo Ersek <lersek@redhat.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Brijesh Singh <brijesh.singh@amd.com>
> Subject: [edk2-devel] [PATCH v4 07/40] UefiCpuPkg: Implement library support for VMGEXIT
> 
> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7Ca5720d7a6a5d4e744f9108d7b0e6bf80%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637172377418610890&amp;sdata=OpW4YzwW9NW4qr9PRvh%2FTgWO28Dt3vGXxA7%2FnYwUsjw%3D&amp;reserved=0
> 
> To support issuing a VMGEXIT instruction, create a library that can be used to perform GHCB and VMGEXIT related operations and to issue the actual VMGEXIT instruction when using the GHCB.
> 
> Additionally, two VMGEXIT / MMIO related functions are created to support flash emulation. Flash emulation currently is done by marking the flash area as read-only and taking a nested page fault to perform the emulation of the instruction. However, emulation cannot be performed because there is no instruction decode assist support when SEV-ES is enabled. Provide routines to initiate an MMIO request to perform actual writes to flash.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Acked-by: Laszlo Ersek <lersek@redhat.com>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  UefiCpuPkg/UefiCpuPkg.dec                    |   3 +
>  UefiCpuPkg/UefiCpuPkg.dsc                    |   5 +
>  UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf |  33 ++++
>  UefiCpuPkg/Include/Library/VmgExitLib.h      | 111 +++++++++++
>  UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c   | 187 +++++++++++++++++++
>  UefiCpuPkg/Library/VmgExitLib/VmgExitLib.uni |  15 ++
>  6 files changed, 354 insertions(+)
>  create mode 100644 UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>  create mode 100644 UefiCpuPkg/Include/Library/VmgExitLib.h
>  create mode 100644 UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c
>  create mode 100644 UefiCpuPkg/Library/VmgExitLib/VmgExitLib.uni
> 
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index 005703d8a3e7..893d2d06b0f2 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -53,6 +53,9 @@ [LibraryClasses.IA32, LibraryClasses.X64]
>    ##
>    MpInitLib|Include/Library/MpInitLib.h
>  
> +  ##  @libraryclass  Provides function to support VMGEXIT processing.
> +  VmgExitLib|Include/Library/VmgExitLib.h
> +
>  [Guids]
>    gUefiCpuPkgTokenSpaceGuid      = { 0xac05bf33, 0x995a, 0x4ed4, { 0xaa, 0xb8, 0xef, 0x7a, 0xe8, 0xf, 0x5c, 0xb0 }}
>    gMsegSmramGuid                 = { 0x5802bce4, 0xeeee, 0x4e33, { 0xa1, 0x30, 0xeb, 0xad, 0x27, 0xf0, 0xe4, 0x39 }}
> diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc index d28cb5cccb52..5ab7e423e8ab 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dsc
> +++ b/UefiCpuPkg/UefiCpuPkg.dsc
> @@ -63,6 +63,7 @@ [LibraryClasses.common.SEC]
>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
>    PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>  
>  [LibraryClasses.common.PEIM]
>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
> @@ -74,6 +75,7 @@ [LibraryClasses.common.PEIM]  [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM]
>    PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf
>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf
> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>  
>  [LibraryClasses.common.DXE_DRIVER]
>    MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf
> @@ -81,12 +83,14 @@ [LibraryClasses.common.DXE_DRIVER]
>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptionHandlerLib.inf
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
>    RegisterCpuFeaturesLib|UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.inf
> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>  
>  [LibraryClasses.common.DXE_SMM_DRIVER]
>    SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf
>    MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAllocationLib.inf
>    HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>    CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptionHandlerLib.inf
> +  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>  
>  [LibraryClasses.common.UEFI_APPLICATION]
>    UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
> @@ -136,6 +140,7 @@ [Components.IA32, Components.X64]
>    UefiCpuPkg/Library/SmmCpuPlatformHookLibNull/SmmCpuPlatformHookLibNull.inf
>    UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
>    UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> +  UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
>    UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
>    UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
>    UefiCpuPkg/SecCore/SecCore.inf
> diff --git a/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf b/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
> new file mode 100644
> index 000000000000..6acfa779e75a
> --- /dev/null
> +++ b/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf
> @@ -0,0 +1,33 @@
> +## @file
> +#  VMGEXIT Support Library.
> +#
> +#  Copyright (c) 2019, Advanced Micro Devices, Inc. All rights 
> +reserved.<BR> #  SPDX-License-Identifier: BSD-2-Clause-Patent # ##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = VmgExitLib
> +  MODULE_UNI_FILE                = VmgExitLib.uni
> +  FILE_GUID                      = 3cd7368f-ef9b-4a9b-9571-2ed93813677e
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = VmgExitLib
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64
> +#
> +
> +[Sources]
> +  VmgExitLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +
> diff --git a/UefiCpuPkg/Include/Library/VmgExitLib.h b/UefiCpuPkg/Include/Library/VmgExitLib.h
> new file mode 100644
> index 000000000000..602b213f1f63
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Library/VmgExitLib.h
> @@ -0,0 +1,111 @@
> +/** @file
> +  Public header file for the VMGEXIT Support library class.
> +
> +  This library class defines some routines used when invoking the 
> + VMGEXIT  instruction in support of SEV-ES.
> +
> +  Copyright (c) 2019, Advanced Micro Devices, Inc. All rights 
> + reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef __VMG_EXIT_LIB_H__
> +#define __VMG_EXIT_LIB_H__
> +
> +#include <Register/Amd/Ghcb.h>
> +
> +
> +/**
> +  Perform VMGEXIT.
> +
> +  Sets the necessary fields of the GHCB, invokes the VMGEXIT 
> + instruction and  then handles the return actions.
> +
> +  @param[in]  GHCB       A pointer to the GHCB
> +  @param[in]  ExitCode   VMGEXIT code to be assigned to the SwExitCode field of
> +                         the GHCB.
> +  @param[in]  ExitInfo1  VMGEXIT information to be assigned to the SwExitInfo1
> +                         field of the GHCB.
> +  @param[in]  ExitInfo2  VMGEXIT information to be assigned to the SwExitInfo2
> +                         field of the GHCB.
> +
> +  @retval  0       VMGEXIT succeeded.
> +  @retval  Others  VMGEXIT processing did not succeed. Exception number to
> +                   be issued.
> +
> +**/
> +UINTN
> +EFIAPI
> +VmgExit (
> +  GHCB                *Ghcb,
> +  UINT64              ExitCode,
> +  UINT64              ExitInfo1,
> +  UINT64              ExitInfo2
> +  );
> +
> +/**
> +  Perform pre-VMGEXIT initialization/preparation.
> +
> +  Performs the necessary steps in preparation for invoking VMGEXIT.
> +
> +  @param[in]  GHCB       A pointer to the GHCB
> +
> +**/
> +VOID
> +EFIAPI
> +VmgInit (
> +  GHCB                *Ghcb
> +  );
> +
> +/**
> +  Perform post-VMGEXIT cleanup.
> +
> +  Performs the necessary steps to cleanup after invoking VMGEXIT.
> +
> +  @param[in]  GHCB       A pointer to the GHCB
> +
> +**/
> +VOID
> +EFIAPI
> +VmgDone (
> +  GHCB                *Ghcb
> +  );
> +
> +#define VMGMMIO_READ   False
> +#define VMGMMIO_WRITE  True
> +
> +/**
> +  Perform MMIO write of a buffer to a non-MMIO marked range.
> +
> +  Performs an MMIO write without taking a #VC. This is useful  for 
> + Flash devices, which are marked read-only.
> +
> +  @param[in]  UINT8      A pointer to the destination buffer
> +  @param[in]  UINTN      The immediate value to write
> +  @param[in]  UINTN      Number of bytes to write
> +
> +**/
> +VOID
> +EFIAPI
> +VmgMmioWrite (
> +  UINT8               *Dest,
> +  UINT8               *Src,
> +  UINTN                Bytes
> +  );
> +
> +/**
> +  Issue the GHCB set AP Jump Table VMGEXIT.
> +
> +  Performs a VMGEXIT using the GHCB AP Jump Table exit code to save the  
> + AP Jump Table address with the hypervisor for retrieval at a later time.
> +
> +  @param[in]  EFI_PHYSICAL_ADDRESS  Physical address of the AP Jump 
> + Table
> +
> +**/
> +UINTN
> +EFIAPI
> +VmgExitSetAPJumpTable (
> +  EFI_PHYSICAL_ADDRESS  Address
> +  );
> +
> +#endif
> diff --git a/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c b/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c
> new file mode 100644
> index 000000000000..4e7315a58202
> --- /dev/null
> +++ b/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c
> @@ -0,0 +1,187 @@
> +/** @file
> +  VMGEXIT Support Library.
> +
> +  Copyright (c) 2019, Advanced Micro Devices, Inc. All rights 
> + reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Base.h>
> +#include <Uefi.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Register/Amd/Ghcb.h>
> +#include <Register/Amd/Msr.h>
> +
> +STATIC
> +UINT64
> +VmgExitErrorCheck (
> +  GHCB                *Ghcb
> +  )
> +{
> +  GHCB_EVENT_INJECTION  Event;
> +  GHCB_EXIT_INFO        ExitInfo;
> +  UINT64                Status;
> +
> +  ExitInfo.Uint64 = Ghcb->SaveArea.SwExitInfo1;  ASSERT 
> + ((ExitInfo.Elements.Lower32Bits == 0) ||
> +          (ExitInfo.Elements.Lower32Bits == 1));
> +
> +  Status = 0;
> +  if (ExitInfo.Elements.Lower32Bits == 0) {
> +    return Status;
> +  }
> +
> +  if (ExitInfo.Elements.Lower32Bits == 1) {
> +    ASSERT (Ghcb->SaveArea.SwExitInfo2 != 0);
> +
> +    // Check that the return event is valid
> +    Event.Uint64 = Ghcb->SaveArea.SwExitInfo2;
> +    if (Event.Elements.Valid &&
> +        Event.Elements.Type == GHCB_EVENT_INJECTION_TYPE_EXCEPTION) {
> +      switch (Event.Elements.Vector) {
> +      case GP_EXCEPTION:
> +      case UD_EXCEPTION:
> +        // Use returned event as return code
> +        Status = Event.Uint64;
> +      }
> +    }
> +  }
> +
> +  if (Status == 0) {
> +    GHCB_EVENT_INJECTION  Event;
> +
> +    Event.Uint64 = 0;
> +    Event.Elements.Vector = GP_EXCEPTION;
> +    Event.Elements.Type   = GHCB_EVENT_INJECTION_TYPE_EXCEPTION;
> +    Event.Elements.Valid  = 1;
> +
> +    Status = Event.Uint64;
> +  }
> +
> +  return Status;
> +}
> +
> +UINT64
> +EFIAPI
> +VmgExit (
> +  GHCB                *Ghcb,
> +  UINT64              ExitCode,
> +  UINT64              ExitInfo1,
> +  UINT64              ExitInfo2
> +  )
> +{
> +  Ghcb->SaveArea.SwExitCode = ExitCode;
> +  Ghcb->SaveArea.SwExitInfo1 = ExitInfo1;
> +  Ghcb->SaveArea.SwExitInfo2 = ExitInfo2;
> +
> +  //
> +  // Guest memory is used for the guest-hypervisor communication, so 
> + fence  // the invocation of the VMGEXIT instruction to ensure GHCB 
> + accesses are  // synchronized properly.
> +  //
> +  MemoryFence ();
> +  AsmVmgExit ();
> +  MemoryFence ();
> +
> +  return VmgExitErrorCheck (Ghcb);
> +}
> +
> +VOID
> +EFIAPI
> +VmgInit (
> +  GHCB                *Ghcb
> +  )
> +{
> +  SetMem (&Ghcb->SaveArea, sizeof (Ghcb->SaveArea), 0); }
> +
> +VOID
> +EFIAPI
> +VmgDone (
> +  GHCB                *Ghcb
> +  )
> +{
> +}
> +
> +UINTN
> +EFIAPI
> +VmgMmio (
> +  UINT8               *MmioAddress,
> +  UINT8               *Buffer,
> +  UINTN               Bytes,
> +  BOOLEAN             Write
> +  )
> +{
> +  UINT64                    MmioOp, ExitInfo1, ExitInfo2, Status;
> +  GHCB                      *Ghcb;
> +  MSR_SEV_ES_GHCB_REGISTER  Msr;
> +
> +  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);  Ghcb = 
> + Msr.Ghcb;
> +
> +  VmgInit (Ghcb);
> +
> +  if (Write) {
> +    MmioOp = SvmExitMmioWrite;
> +  } else {
> +    MmioOp = SvmExitMmioRead;
> +  }
> +
> +  ExitInfo1 = (UINT64) (UINTN) MmioAddress;
> +  ExitInfo2 = Bytes;
> +
> +  if (Write) {
> +    CopyMem (Ghcb->SharedBuffer, Buffer, Bytes);  }
> +
> +  Ghcb->SaveArea.SwScratch = (UINT64) (UINTN) Ghcb->SharedBuffer;  
> + Status = VmgExit (Ghcb, MmioOp, ExitInfo1, ExitInfo2);  if (Status != 
> + 0) {
> +    return Status;
> +  }
> +
> +  if (!Write) {
> +    CopyMem (Buffer, Ghcb->SharedBuffer, Bytes);  }
> +
> +  VmgDone (Ghcb);
> +
> +  return 0;
> +}
> +
> +VOID
> +EFIAPI
> +VmgMmioWrite (
> +  UINT8               *Dest,
> +  UINT8               *Src,
> +  UINTN                Bytes
> +  )
> +{
> +  VmgMmio (Dest, Src, Bytes, TRUE);
> +}
> +
> +UINTN
> +EFIAPI
> +VmgExitSetAPJumpTable (
> +  EFI_PHYSICAL_ADDRESS  Address
> +  )
> +{
> +  UINT64                    ExitInfo1, ExitInfo2, Status;
> +  GHCB                      *Ghcb;
> +  MSR_SEV_ES_GHCB_REGISTER  Msr;
> +
> +  Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);  Ghcb = 
> + Msr.Ghcb;
> +
> +  VmgInit (Ghcb);
> +
> +  ExitInfo1 = 0;
> +  ExitInfo2 = (UINT64) (UINTN) Address;
> +
> +  Status = VmgExit (Ghcb, SvmExitApJumpTable, ExitInfo1, ExitInfo2);
> +
> +  VmgDone (Ghcb);
> +
> +  return Status;
> +}
> +
> diff --git a/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.uni b/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.uni
> new file mode 100644
> index 000000000000..e8656aae4726
> --- /dev/null
> +++ b/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.uni
> @@ -0,0 +1,15 @@
> +// /** @file
> +// VMGEXIT support library instance.
> +//
> +// VMGEXIT support library instance.
> +//
> +// Copyright (c) 2019, Advanced Micro Devices, Inc. All rights 
> +reserved.<BR> // SPDX-License-Identifier: BSD-2-Clause-Patent // // **/
> +
> +
> +#string STR_MODULE_ABSTRACT             #language en-US "VMGEXIT Support Library."
> +
> +#string STR_MODULE_DESCRIPTION          #language en-US "VMGEXIT Support Library."
> +
> --
> 2.17.1
> 
> 
> 
>