On 11/20/19 21:06, Lendacky, Thomas wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
>
> 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>
> Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
> ---
> UefiCpuPkg/UefiCpuPkg.dec | 3 +
> UefiCpuPkg/UefiCpuPkg.dsc | 5 +
> UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf | 33 +++++
> UefiCpuPkg/Include/Library/VmgExitLib.h | 96 ++++++++++++++
> UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c | 132 +++++++++++++++++++
> UefiCpuPkg/Library/VmgExitLib/VmgExitLib.uni | 15 +++
> 6 files changed, 284 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 12f4413ea5b0..90feb9166dc8 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..b5639fbfa1a5
> --- /dev/null
> +++ b/UefiCpuPkg/Include/Library/VmgExitLib.h
> @@ -0,0 +1,96 @@
> +/** @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
> + );
> +
> +#endif
> diff --git a/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c
> b/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c
> new file mode 100644
> index 000000000000..23965b7ff022
> --- /dev/null
> +++ b/UefiCpuPkg/Library/VmgExitLib/VmgExitLib.c
> @@ -0,0 +1,132 @@
> +/** @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
> +UINTN
> +VmgExitErrorCheck (
> + GHCB *Ghcb
> + )
> +{
> + GHCB_EXIT_INFO ExitInfo;
> + UINTN Reason, Action;
> +
> + if (!Ghcb->SaveArea.SwExitInfo1) {
> + return 0;
> + }
> +
> + ExitInfo.Uint64 = Ghcb->SaveArea.SwExitInfo1;
> + Action = ExitInfo.Elements.Lower32Bits;
> + if (Action == 1) {
> + Reason = ExitInfo.Elements.Upper32Bits;
> +
> + switch (Reason) {
> + case UD_EXCEPTION:
> + case GP_EXCEPTION:
> + return Reason;
> + }
> + }
> +
> + ASSERT (0);
> + return GP_EXCEPTION;
> +}
> +
> +UINTN
> +EFIAPI
> +VmgExit (
> + GHCB *Ghcb,
> + UINT64 ExitCode,
> + UINT64 ExitInfo1,
> + UINT64 ExitInfo2
> + )
> +{
> + Ghcb->SaveArea.SwExitCode = ExitCode;
> + Ghcb->SaveArea.SwExitInfo1 = ExitInfo1;
> + Ghcb->SaveArea.SwExitInfo2 = ExitInfo2;
> + AsmVmgExit ();
This patch looks good to me (and, in general, I'd like to defer to Ray
and Eric on the UefiCpuPkg patches); just one comment:
AsmVmgExit() orchestrates guest-host communication in guest RAM, and in
that sense, it is somewhat similar to virtio. In virtio (per spec), we
use MemoryFence() calls carefully.
Now, if you check the MemoryFence() implementation in
"MdePkg/Library/BaseLib/X64/GccInline.c", it is only
__asm__ __volatile__ ("":::"memory");
which is already part of AsmVmgExit(), from the previous patch:
[edk2-devel] [RFC PATCH v3 06/43]
MdePkg/BaseLib: Add support for the VMGEXIT instruction
__asm__ __volatile__ ("rep; vmmcall":::"memory");
So, I think it's not necessary *in practice* to add a MemoryFence()
ahead of the AsmVmgExit(). But, I think it is needed afterwards.
The comment in "MdePkg/Library/BaseLib/X64/GccInline.c" says, "it is
more about the compiler that it is actually processor synchronization".
With that in mind, I'd like to suggest one of two alternatives:
- modify the AsmVmgExit() documentation (function level comment in the
lib class header) so that it spell out that all barriers (before &
after) are contained within,
- or please modify the call site so that it is both preceded and
succeeded by MemoryFence().
With either option implemented:
Acked-by: Laszlo Ersek <ler...@redhat.com>
Thanks
Laszlo
> +
> + 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;
> + UINT64 ExitInfo1, ExitInfo2;
> + UINTN Status;
> + GHCB *Ghcb;
> + MSR_SEV_ES_GHCB_REGISTER Msr;
> +
> + Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB);
> + Ghcb = Msr.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);
> + }
> +
> + return 0;
> +}
> +
> +VOID
> +EFIAPI
> +VmgMmioWrite (
> + UINT8 *Dest,
> + UINT8 *Src,
> + UINTN Bytes
> + )
> +{
> + VmgMmio (Dest, Src, Bytes, TRUE);
> +}
> +
> 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."
> +
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#51048): https://edk2.groups.io/g/devel/message/51048
Mute This Topic: https://groups.io/mt/60973103/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
