From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.120]) by mx.groups.io with SMTP id smtpd.web12.10306.1590079935127868861 for ; Thu, 21 May 2020 09:52:15 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=f+M+BbzB; spf=pass (domain: redhat.com, ip: 207.211.31.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1590079934; 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=bPn0nQeTz0An1Pi+3DCLN9pyrMB1emCuVBqMwaxndJI=; b=f+M+BbzBacFEJpS0mFwpT13bwnhrXtPNsFXm++ZBv0jqlJMB7IA2U11YmgrdqbUncwnQnR 7RSrZCzaFmtNhlPDdArs7mNxpwVdCnh1xOt1WLwaTnCpBAiLvhAMuS2YtXzFKDJGezmpT8 Mv7+wg1fXu3RP07xCyrcmN4Pdjhgwqs= 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-183--JrjgXnSMn-O_ZabHQD30A-1; Thu, 21 May 2020 12:52:11 -0400 X-MC-Unique: -JrjgXnSMn-O_ZabHQD30A-1 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C46F5835B59; Thu, 21 May 2020 16:52:09 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-225.ams2.redhat.com [10.36.113.225]) by smtp.corp.redhat.com (Postfix) with ESMTP id EEFDC60C87; Thu, 21 May 2020 16:52:06 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v8 12/46] OvmfPkg/VmgExitLib: Implement library support for VmgExitLib in OVMF 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 , Ard Biesheuvel References: From: "Laszlo Ersek" Message-ID: Date: Thu, 21 May 2020 18:52:05 +0200 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: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 05/19/20 23:50, Lendacky, Thomas wrote: > The base VmgExitLib library provides a default limited interface. As it > does not provide full support, create an OVMF version of this library to > begin the process of providing full support of SEV-ES within OVMF. > > SEV-ES support is only provided for X64 builds, so only OvmfPkgX64.dsc is > updated to make use of the OvmfPkg version of the library. > > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Signed-off-by: Tom Lendacky > --- > OvmfPkg/OvmfPkgX64.dsc | 2 +- > OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 36 ++++ > OvmfPkg/Library/VmgExitLib/VmgExitLib.c | 155 ++++++++++++++++++ > .../Library/VmgExitLib/X64/VmgExitVcHandler.c | 81 +++++++++ > OvmfPkg/Library/VmgExitLib/VmgExitLib.uni | 15 ++ (1) Please drop the UNI file. UNI files are needed (to my understanding) with UPT (UEFI Packaging Tool), but OvmfPkg content is not distributed like that. So UNI files would only be a distraction under OvmfPkg. > 5 files changed, 288 insertions(+), 1 deletion(-) > create mode 100644 OvmfPkg/Library/VmgExitLib/VmgExitLib.inf > create mode 100644 OvmfPkg/Library/VmgExitLib/VmgExitLib.c > create mode 100644 OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c > create mode 100644 OvmfPkg/Library/VmgExitLib/VmgExitLib.uni > > diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index 0b9189ab1e38..b5f3859420d0 100644 > --- a/OvmfPkg/OvmfPkgX64.dsc > +++ b/OvmfPkg/OvmfPkgX64.dsc > @@ -232,7 +232,7 @@ [LibraryClasses] > > [LibraryClasses.common] > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > - VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf > + VmgExitLib|OvmfPkg/Library/VmgExitLib/VmgExitLib.inf > > [LibraryClasses.common.SEC] > TimerLib|OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.inf > diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf > new file mode 100644 > index 000000000000..0e6bc8432314 > --- /dev/null > +++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.inf > @@ -0,0 +1,36 @@ > +## @file > +# VMGEXIT Support Library. > +# > +# Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = VmgExitLib > + MODULE_UNI_FILE = VmgExitLib.uni (2) Please drop MODULE_UNI_FILE too, according to (1). > + FILE_GUID = 0e923c25-13cd-430b-8714-ffe85652a97b > + 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 = X64 > +# > + > +[Sources.X64] > + X64/VmgExitVcHandler.c > + > +[Sources.common] > + VmgExitLib.c (3) I think this split for [Sources] does not make sense under OvmfPkg. I'd only use one [Sources] Section, and also move VmgExitVcHandler.c out of the X64 subdir. The X64 subdir per se doesn't look useful either. > + > +[Packages] > + MdePkg/MdePkg.dec > + UefiCpuPkg/UefiCpuPkg.dec > + > +[LibraryClasses] > + BaseLib > + > diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.c b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c > new file mode 100644 > index 000000000000..7b7ebea85256 > --- /dev/null > +++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c > @@ -0,0 +1,155 @@ > +/** @file > + VMGEXIT Support Library. > + > + Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include > +#include > +#include > +#include > + > +/** > + 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 (4) Please prepend and append empty "//" lines. > + 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 (5) Same as (4). With these addressed: Acked-by: Laszlo Ersek Thanks, Laszlo > + Status = Event.Uint64; > + } > + } > + } > + > + if (Status == 0) { > + GHCB_EVENT_INJECTION GpEvent; > + > + GpEvent.Uint64 = 0; > + GpEvent.Elements.Vector = GP_EXCEPTION; > + GpEvent.Elements.Type = GHCB_EVENT_INJECTION_TYPE_EXCEPTION; > + GpEvent.Elements.Valid = 1; > + > + Status = GpEvent.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 > + event 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 > + ) > +{ > +} > + > diff --git a/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c > new file mode 100644 > index 000000000000..036f030d6b34 > --- /dev/null > +++ b/OvmfPkg/Library/VmgExitLib/X64/VmgExitVcHandler.c > @@ -0,0 +1,81 @@ > +/** @file > + X64 #VC Exception Handler functon. > + > + Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include > +#include > +#include > +#include > + > +/** > + Handle a #VC exception. > + > + Performs the necessary processing to handle a #VC exception. > + > + @param[in, out] ExceptionType Pointer to an EFI_EXCEPTION_TYPE to be set > + as value to use on error. > + @param[in, out] SystemContext Pointer to EFI_SYSTEM_CONTEXT > + > + @retval EFI_SUCCESS Exception handled > + @retval EFI_UNSUPPORTED #VC not supported, (new) exception value to > + propagate provided > + @retval EFI_PROTOCOL_ERROR #VC handling failed, (new) exception value to > + propagate provided > + > +**/ > +EFI_STATUS > +EFIAPI > +VmgExitHandleVc ( > + IN OUT EFI_EXCEPTION_TYPE *ExceptionType, > + IN OUT EFI_SYSTEM_CONTEXT SystemContext > + ) > +{ > + MSR_SEV_ES_GHCB_REGISTER Msr; > + EFI_SYSTEM_CONTEXT_X64 *Regs; > + GHCB *Ghcb; > + UINT64 ExitCode, Status; > + EFI_STATUS VcRet; > + > + VcRet = EFI_SUCCESS; > + > + Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB); > + ASSERT (Msr.GhcbInfo.Function == 0); > + ASSERT (Msr.Ghcb != 0); > + > + Regs = SystemContext.SystemContextX64; > + Ghcb = Msr.Ghcb; > + > + VmgInit (Ghcb); > + > + ExitCode = Regs->ExceptionData; > + switch (ExitCode) { > + default: > + Status = VmgExit (Ghcb, SVM_EXIT_UNSUPPORTED, ExitCode, 0); > + if (Status == 0) { > + Regs->ExceptionData = 0; > + *ExceptionType = GP_EXCEPTION; > + } else { > + GHCB_EVENT_INJECTION Event; > + > + Event.Uint64 = Status; > + if (Event.Elements.ErrorCodeValid) { > + Regs->ExceptionData = Event.Elements.ErrorCode; > + } else { > + Regs->ExceptionData = 0; > + } > + > + *ExceptionType = Event.Elements.Vector; > + } > + > + VcRet = EFI_PROTOCOL_ERROR; > + } > + > + VmgDone (Ghcb); > + > + return VcRet; > +} > diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.uni b/OvmfPkg/Library/VmgExitLib/VmgExitLib.uni > new file mode 100644 > index 000000000000..a919b484c319 > --- /dev/null > +++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.uni > @@ -0,0 +1,15 @@ > +// /** @file > +// VMGEXIT support library instance. > +// > +// VMGEXIT support library instance. > +// > +// Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.
> +// SPDX-License-Identifier: BSD-2-Clause-Patent > +// > +// **/ > + > + > +#string STR_MODULE_ABSTRACT #language en-US "OVMF VMGEXIT Support Library." > + > +#string STR_MODULE_DESCRIPTION #language en-US "OVMF VMGEXIT Support Library." > + >