From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.22463.1574334945030010904 for ; Thu, 21 Nov 2019 03:15:45 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=O2luA5cC; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574334944; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=HY8wVzZ/Qg7XsVh/SVeYznnTbp0gdpRap1lEZUBWQN0=; b=O2luA5cCeodgGQXUnTeq2RP1U4x4mhb3UOHK+19vaOVlJXXUi2TAYf7oodu7+Aq4tSXg3n MxabzZk7H9fN/OV5nxh2+Y3cEqO2D5B7riA6ZbsUt787gB7u8OGKEPh3yA+qAeoH11oTAj 9+xeKV1DsL1+PdQbQaRFCIfDCwYUryo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-392-vVJYGvPrNmCPPn95pTdBPA-1; Thu, 21 Nov 2019 06:15:42 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id E55F11804974; Thu, 21 Nov 2019 11:15:40 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-197.ams2.redhat.com [10.36.116.197]) by smtp.corp.redhat.com (Postfix) with ESMTP id 877567092F; Thu, 21 Nov 2019 11:15:38 +0000 (UTC) Subject: Re: [edk2-devel] [RFC PATCH v3 07/43] UefiCpuPkg: Implement library support for VMGEXIT To: devel@edk2.groups.io, thomas.lendacky@amd.com Cc: Jordan Justen , Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , Brijesh Singh References: <5c8f36407e5ac7c7757ae5108cf861b36287a3ce.1574280425.git.thomas.lendacky@amd.com> From: "Laszlo Ersek" Message-ID: <49c5bbb0-0b52-c528-c917-212b5a84f1f7@redhat.com> Date: Thu, 21 Nov 2019 12:15:37 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <5c8f36407e5ac7c7757ae5108cf861b36287a3ce.1574280425.git.thomas.lendacky@amd.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-MC-Unique: vVJYGvPrNmCPPn95pTdBPA-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable On 11/20/19 21:06, Lendacky, Thomas wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2198 >=20 > 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. >=20 > 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. >=20 > Cc: Eric Dong > Cc: Ray Ni > Cc: Laszlo Ersek > Signed-off-by: Tom Lendacky > --- > 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 >=20 > 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 > =20 > + ## @libraryclass Provides function to support VMGEXIT processing. > + VmgExitLib|Include/Library/VmgExitLib.h > + > [Guids] > gUefiCpuPkgTokenSpaceGuid =3D { 0xac05bf33, 0x995a, 0x4ed4, { 0xa= a, 0xb8, 0xef, 0x7a, 0xe8, 0xf, 0x5c, 0xb0 }} > gMsegSmramGuid =3D { 0x5802bce4, 0xeeee, 0x4e33, { 0xa= 1, 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/PeiServicesTablePointerLibId= t/PeiServicesTablePointerLibIdt.inf > MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAll= ocationLib.inf > + VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf > =20 > [LibraryClasses.common.PEIM] > MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAll= ocationLib.inf > @@ -74,6 +75,7 @@ [LibraryClasses.common.PEIM] > [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM] > PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibId= t/PeiServicesTablePointerLibIdt.inf > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCp= uExceptionHandlerLib.inf > + VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf > =20 > [LibraryClasses.common.DXE_DRIVER] > MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryA= llocationLib.inf > @@ -81,12 +83,14 @@ [LibraryClasses.common.DXE_DRIVER] > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCp= uExceptionHandlerLib.inf > MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf > RegisterCpuFeaturesLib|UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRe= gisterCpuFeaturesLib.inf > + VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf > =20 > [LibraryClasses.common.DXE_SMM_DRIVER] > SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTabl= eLib.inf > MemoryAllocationLib|MdePkg/Library/SmmMemoryAllocationLib/SmmMemoryAll= ocationLib.inf > HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf > CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCp= uExceptionHandlerLib.inf > + VmgExitLib|UefiCpuPkg/Library/VmgExitLib/VmgExitLib.inf > =20 > [LibraryClasses.common.UEFI_APPLICATION] > UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/Uef= iApplicationEntryPoint.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/Li= brary/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.=
> +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION =3D 0x00010005 > + BASE_NAME =3D VmgExitLib > + MODULE_UNI_FILE =3D VmgExitLib.uni > + FILE_GUID =3D 3cd7368f-ef9b-4a9b-9571-2ed93813677= e > + MODULE_TYPE =3D BASE > + VERSION_STRING =3D 1.0 > + LIBRARY_CLASS =3D VmgExitLib > + > +# > +# The following information is for reference only and not required by th= e build tools. > +# > +# VALID_ARCHITECTURES =3D 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 VMGEXI= T > + 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 > + > + > +/** > + 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 f= ield of > + the GHCB. > + @param[in] ExitInfo1 VMGEXIT information to be assigned to the SwExi= tInfo1 > + field of the GHCB. > + @param[in] ExitInfo2 VMGEXIT information to be assigned to the SwExi= tInfo2 > + 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/Libr= ary/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 > +#include > +#include > +#include > +#include > + > +STATIC > +UINTN > +VmgExitErrorCheck ( > + GHCB *Ghcb > + ) > +{ > + GHCB_EXIT_INFO ExitInfo; > + UINTN Reason, Action; > + > + if (!Ghcb->SaveArea.SwExitInfo1) { > + return 0; > + } > + > + ExitInfo.Uint64 =3D Ghcb->SaveArea.SwExitInfo1; > + Action =3D ExitInfo.Elements.Lower32Bits; > + if (Action =3D=3D 1) { > + Reason =3D 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 =3D ExitCode; > + Ghcb->SaveArea.SwExitInfo1 =3D ExitInfo1; > + Ghcb->SaveArea.SwExitInfo2 =3D 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 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 =3D AsmReadMsr64 (MSR_SEV_ES_GHCB); > + Ghcb =3D Msr.Ghcb; > + > + if (Write) { > + MmioOp =3D SvmExitMmioWrite; > + } else { > + MmioOp =3D SvmExitMmioRead; > + } > + > + ExitInfo1 =3D (UINT64) (UINTN) MmioAddress; > + ExitInfo2 =3D Bytes; > + > + if (Write) { > + CopyMem (Ghcb->SharedBuffer, Buffer, Bytes); > + } > + > + Ghcb->SaveArea.SwScratch =3D (UINT64) (UINTN) Ghcb->SharedBuffer; > + Status =3D VmgExit (Ghcb, MmioOp, ExitInfo1, ExitInfo2); > + if (Status !=3D 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/Li= brary/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.=
> +// 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." > + >=20