On 5/8/20 8:06 PM, Dong, Eric wrote:
Hi Tom,

-----Original Message-----
From: Tom Lendacky <thomas.lenda...@amd.com>
Sent: Thursday, April 23, 2020 1:41 AM
To: devel@edk2.groups.io
Cc: Justen, Jordan L <jordan.l.jus...@intel.com>; Laszlo Ersek
<ler...@redhat.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>; Kinney,
Michael D <michael.d.kin...@intel.com>; Gao, Liming
<liming....@intel.com>; Dong, Eric <eric.d...@intel.com>; Ni, Ray
<ray...@intel.com>; Brijesh Singh <brijesh.si...@amd.com>
Subject: [PATCH v7 08/43] 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%7C263f75f798c64b75e23508d7f3b53108%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637245831845366269&amp;sdata=TndzK3JmXQFNGBHSfbsSPr%2BGY8NMVX%2B1Durfner4k1I%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.d...@intel.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Acked-by: Laszlo Ersek <ler...@redhat.com>
Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
---
  UefiCpuPkg/UefiCpuPkg.dec                    |   3 +
  UefiCpuPkg/UefiCpuPkg.dsc                    |   2 +
  UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf |  33 +++
  UefiCpuPkg/Include/Library/VmgExitLib.h      | 117 ++++++++
  UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c   | 293
+++++++++++++++++++
  UefiCpuPkg/Library/VmgExitLib/VmgExitLib.uni |  15 +
  6 files changed, 463 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 df5d02bae6b4..cb92f34b6f55 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..997840452218 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -56,6 +56,7 @@ [LibraryClasses]

PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/Base
PeCoffGetEntryPointLib.inf

PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BaseP
eCoffExtraActionLibNull.inf

TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/Tp
mMeasurementLibNull.inf
+  VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf

  [LibraryClasses.common.SEC]

PlatformSecLib|UefiCpuPkg/Library/PlatformSecLibNull/PlatformSecLibNull.i
nf
@@ -136,6 +137,7 @@ [Components.IA32, Components.X64]

UefiCpuPkg/Library/SmmCpuPlatformHookLibNull/SmmCpuPlatformHookLib
Null.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..3bf05bebd326
--- /dev/null
+++ b/UefiCpuPkg/Include/Library/VmgExitLib.h
@@ -0,0 +1,117 @@
+/** @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, out]  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 propagated.
+
+**/
+UINT64
+EFIAPI
+VmgExit (
+  IN OUT GHCB                *Ghcb,
+  IN     UINT64              ExitCode,
+  IN     UINT64              ExitInfo1,
+  IN     UINT64              ExitInfo2
+  );
+
+/**
+  Perform pre-VMGEXIT initialization/preparation.
+
+  Performs the necessary steps in preparation for invoking VMGEXIT.
+ Must be  called before setting any fields within the GHCB.
+
+  @param[in, out]  Ghcb       A pointer to the GHCB
+
+**/
+VOID
+EFIAPI
+VmgInit (
+  IN OUT GHCB                *Ghcb
+  );
+
+/**
+  Perform post-VMGEXIT cleanup.
+
+  Performs the necessary steps to cleanup after invoking VMGEXIT. Must
+ be  called after obtaining needed fields within the GHCB.
+
+  @param[in, out]  Ghcb       A pointer to the GHCB
+
+**/
+VOID
+EFIAPI
+VmgDone (
+  IN OUT 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, out]  Dest       A pointer to the destination buffer
+  @param[in]       Src        A pointer to the source data to be written
+  @param[in]       Bytes      Number of bytes to write
+
+**/
+VOID
+EFIAPI
+VmgMmioWrite (
+  IN OUT UINT8               *Dest,
+  IN     UINT8               *Src,
+  IN     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]  Address  Physical address of the AP Jump Table
+
+  @retval  0           VMGEXIT succeeded.
+  @retval  Others      VMGEXIT processing did not succeed. Exception
+                       number to be propagated.
+
+**/
+UINT64
+EFIAPI
+VmgExitSetAPJumpTable (
+  IN EFI_PHYSICAL_ADDRESS  Address
+  );

I think above two APIs should not been added to this library, they are not the 
basic actions for VmgExit.
Remove these two APIs will make the library more stable.

Also, I check all the code in this patch series, only one caller for each API, 
so I think we can directly
move these codes to the caller. If later more and more callers need to use 
these two APIs, we can
create another service library to convenient the callers.

Ok, let me check into that. It should be very doable, especially with the change to a NULL library implementation that I want for the general case.

Thanks,
Tom


I ignore all the coding style related issues in this patch because I assume you 
have passed ECC
checks in your new patches.

Thanks,
Eric
+
+#endif
diff --git a/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c
b/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c
new file mode 100644
index 000000000000..6137b1a0eb64
--- /dev/null
+++ b/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c
@@ -0,0 +1,293 @@
+/** @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>
+
+/**
+  Check for VMGEXIT error
+
+  Check if the hypervisor has returned an error after completion of the
+ VMGEXIT  by examining the SwExitInfo1 field of the GHCB.
+
+  @param[in]  Ghcb       A pointer to the GHCB
+
+  @retval  0             VMGEXIT succeeded.
+  @retval  Others        VMGEXIT processing did not succeed. Exception
number to
+                         be propagated.
+
+**/
+STATIC
+UINT64
+VmgExitErrorCheck (
+  IN 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;
+}
+
+/**
+  Perform VMGEXIT.
+
+  Sets the necessary fields of the GHCB, invokes the VMGEXIT
+ instruction and  then handles the return actions.
+
+  @param[in, out]  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 propagated.
+
+**/
+UINT64
+EFIAPI
+VmgExit (
+  IN OUT GHCB                *Ghcb,
+  IN     UINT64              ExitCode,
+  IN     UINT64              ExitInfo1,
+  IN     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);
+}
+
+/**
+  Perform pre-VMGEXIT initialization/preparation.
+
+  Performs the necessary steps in preparation for invoking VMGEXIT.
+ Must be  called before setting any fields within the GHCB.
+
+  @param[in, out]  Ghcb       A pointer to the GHCB
+
+**/
+VOID
+EFIAPI
+VmgInit (
+  IN OUT GHCB                *Ghcb
+  )
+{
+  SetMem (&Ghcb->SaveArea, sizeof (Ghcb->SaveArea), 0); }
+
+/**
+  Perform post-VMGEXIT cleanup.
+
+  Performs the necessary steps to cleanup after invoking VMGEXIT. Must
+ be  called after obtaining needed fields within the GHCB.
+
+  @param[in, out]  Ghcb       A pointer to the GHCB
+
+**/
+VOID
+EFIAPI
+VmgDone (
+  IN OUT GHCB                *Ghcb
+  )
+{
+}
+
+/**
+  Perform VMGEXIT MMIO read or write.
+
+  Performs the requested MMIO read or write using the VMGEXIT
instruction.
+
+  For an MMIO read, the data that has been read during the VMGEXIT is
+ placed in  the SharedBuffer area of the GHCB. This is then copied to
+ the actual  destination buffer within the guest.
+
+  For an MMIO write, the data to be written is copied into the
+ SharedBuffer area  of the GHCB by the guest. This is then copied to
+ the actual destination buffer  by the hypervisor during the VMGEXIT.
+
+  @param[in, out]  MmioAddress  A pointer to the MMIO buffer to be
read/written
+  @param[in, out]  Buffer       A pointer to the buffer to hold the data thas
+                                has been read or hold the data to be written
+  @param[in]       Bytes        Number of bytes to read or write
+  @param[in]       Write        If set, the request is for an MMIO write, else
+                                it is an MMIO read.
+
+  @retval  0                    VMGEXIT succeeded.
+  @retval  Others               VMGEXIT processing did not succeed. Exception
+                                number to be propagated.
+
+**/
+STATIC
+UINT64
+EFIAPI
+VmgMmio (
+  IN OUT UINT8               *MmioAddress,
+  IN OUT UINT8               *Buffer,
+  IN     UINTN               Bytes,
+  IN     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;
+
+  //
+  // This function is about to set fields in the GHCB. Do not execute
+ // anything that will cause a #VC before issuing the VmgExit(). Any
+ #VC  // will result in all GHCB settings being overwritten (this
+ means, e.g.,  // do not add DEBUG() statements).
+  //
+  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;
+}
+
+/**
+  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, out]  Dest       A pointer to the destination buffer
+  @param[in]       Src        A pointer to the source data to be written
+  @param[in]       Bytes      Number of bytes to write
+
+**/
+VOID
+EFIAPI
+VmgMmioWrite (
+  IN OUT UINT8               *Dest,
+  IN     UINT8               *Src,
+  IN     UINTN                Bytes
+  )
+{
+  VmgMmio (Dest, Src, Bytes, TRUE);
+}
+
+/**
+  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]  Address  Physical address of the AP Jump Table
+
+  @retval  0           VMGEXIT succeeded.
+  @retval  Others      VMGEXIT processing did not succeed. Exception
+                       number to be propagated.
+
+**/
+UINT64
+EFIAPI
+VmgExitSetAPJumpTable (
+  IN 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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#58956): https://edk2.groups.io/g/devel/message/58956
Mute This Topic: https://groups.io/mt/73201897/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to