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&data=02%7C01%7Cthomas.lendacky%40amd.com%7C263f75f798c64b75e23508d7f3b53108%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637245831845366269&sdata=TndzK3JmXQFNGBHSfbsSPr%2BGY8NMVX%2B1Durfner4k1I%3D&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] -=-=-=-=-=-=-=-=-=-=-=-