From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web12.3117.1609839660986329986 for ; Tue, 05 Jan 2021 01:41:01 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Qs6DUOtK; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1609839660; 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=ydk1/tYJ8n8pwtxZLN8ucH0xo7lqCCIkH/37kOvJb/E=; b=Qs6DUOtKD6NVCnY9llVtSzHJ3vTR1siE588QM9tLc0FY8wp/5vCNmw/0nDGJd8S/31G908 zpPxpz6w2QB/2Mu5E6ja53mKffs0dkjcByOUfB8/DB1GIJQTsvvHQrUl7UL5/3acMuyzcW xMMkDNvOPtnwp5hha7bqm+lX7W1sqvI= 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-348-C7JiurKfNF--qSuDoe47zA-1; Tue, 05 Jan 2021 04:40:55 -0500 X-MC-Unique: C7JiurKfNF--qSuDoe47zA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8AC5D107ACE3; Tue, 5 Jan 2021 09:40:54 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-233.ams2.redhat.com [10.36.112.233]) by smtp.corp.redhat.com (Postfix) with ESMTP id 78C21100AE2F; Tue, 5 Jan 2021 09:40:52 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 08/12] OvmfPkg/MemEncryptSevLib: Make the MemEncryptSevLib available for SEC To: devel@edk2.groups.io, thomas.lendacky@amd.com Cc: Brijesh Singh , James Bottomley , Jordan Justen , Ard Biesheuvel References: <6b14be9c75dd31656e986c8e7611b739c2b22a9e.1608065471.git.thomas.lendacky@amd.com> From: "Laszlo Ersek" Message-ID: <3bd1ca24-c743-5688-9ec5-2af467a8c595@redhat.com> Date: Tue, 5 Jan 2021 10:40:51 +0100 MIME-Version: 1.0 In-Reply-To: <6b14be9c75dd31656e986c8e7611b739c2b22a9e.1608065471.git.thomas.lendacky@amd.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 12/15/20 21:51, Lendacky, Thomas wrote: > From: Tom Lendacky > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3108 > > In preparation for a new interface to be added to the MemEncryptSevLib > library that will be used in SEC, create an SEC version of the library. > > This requires the creation of SEC specific files. > > Some of the current MemEncryptSevLib functions perform memory allocations > which cannot be performed in SEC, so these interfaces will return an error > during SEC. Also, the current MemEncryptSevLib library uses some static > variables to optimize access to variables, which cannot be used in SEC. > > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Brijesh Singh > Signed-off-by: Tom Lendacky > --- > .../DxeBaseMemEncryptSevLib.inf | 2 +- > .../PeiBaseMemEncryptSevLib.inf | 2 +- > .../SecBaseMemEncryptSevLib.inf | 54 ++++++++ > .../SecMemEncryptSevLibInternal.c | 130 ++++++++++++++++++ > ...{VirtualMemory.c => PeiDxeVirtualMemory.c} | 12 +- > .../X64/SecVirtualMemory.c | 80 +++++++++++ > 6 files changed, 272 insertions(+), 8 deletions(-) > create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/SecBaseMemEncryptSevLib.inf > create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c > rename OvmfPkg/Library/BaseMemEncryptSevLib/X64/{VirtualMemory.c => PeiDxeVirtualMemory.c} (95%) > create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c (1) /s/SecBase/Sec/ (in filenames and in filename references; the BASE_NAME is OK) > > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeBaseMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeBaseMemEncryptSevLib.inf > index 2be6ca1fa737..390f2d60677f 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/DxeBaseMemEncryptSevLib.inf > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/DxeBaseMemEncryptSevLib.inf > @@ -33,7 +33,7 @@ [Sources.X64] > DxeMemEncryptSevLibInternal.c > MemEncryptSevLibInternal.c > X64/MemEncryptSevLib.c > - X64/VirtualMemory.c > + X64/PeiDxeVirtualMemory.c > X64/VirtualMemory.h > > [Sources.IA32] > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiBaseMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiBaseMemEncryptSevLib.inf > index 7bdf8cb5210d..cb973fdeb868 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/PeiBaseMemEncryptSevLib.inf > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/PeiBaseMemEncryptSevLib.inf > @@ -33,7 +33,7 @@ [Sources.X64] > PeiMemEncryptSevLibInternal.c > MemEncryptSevLibInternal.c > X64/MemEncryptSevLib.c > - X64/VirtualMemory.c > + X64/PeiDxeVirtualMemory.c > X64/VirtualMemory.h > > [Sources.IA32] > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecBaseMemEncryptSevLib.inf b/OvmfPkg/Library/BaseMemEncryptSevLib/SecBaseMemEncryptSevLib.inf > new file mode 100644 > index 000000000000..b26f739d69fd > --- /dev/null > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecBaseMemEncryptSevLib.inf > @@ -0,0 +1,54 @@ > +## @file > +# Library provides the helper functions for SEV guest > +# > +# Copyright (c) 2020 Advanced Micro Devices. All rights reserved.
> +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +# > +## > + > +[Defines] > + INF_VERSION = 1.25 > + BASE_NAME = SecMemEncryptSevLib > + FILE_GUID = 046388b4-430e-4e61-88f6-51ea21db2632 > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = MemEncryptSevLib|SEC > + > +# > +# The following information is for reference only and not required by the build > +# tools. > +# > +# VALID_ARCHITECTURES = IA32 X64 > +# > + > +[Packages] > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + OvmfPkg/OvmfPkg.dec > + UefiCpuPkg/UefiCpuPkg.dec > + > +[Sources.X64] > + SecMemEncryptSevLibInternal.c > + MemEncryptSevLibInternal.c > + X64/MemEncryptSevLib.c > + X64/SecVirtualMemory.c > + X64/VirtualMemory.h > + > +[Sources.IA32] > + SecMemEncryptSevLibInternal.c > + MemEncryptSevLibInternal.c > + Ia32/MemEncryptSevLib.c > + > +[LibraryClasses] > + BaseLib > + CpuLib > + DebugLib > + PcdLib > + > +[FeaturePcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire > + (2) This PCD does not look useful for the new library instance (at least at this stage). > +[FixedPcd] > + gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c > new file mode 100644 > index 000000000000..30d2ebe1d6e9 > --- /dev/null > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/SecMemEncryptSevLibInternal.c > @@ -0,0 +1,130 @@ > +/** @file > + > + Secure Encrypted Virtualization (SEV) library helper function > + > + Copyright (c) 2020, Advanced Micro Devices, Inc. All rights reserved.
> + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +/** > + Reads and sets the status of SEV features. > + > + **/ > +STATIC > +UINT32 > +EFIAPI > +InternalMemEncryptSevStatus ( > + VOID > + ) > +{ > + UINT32 RegEax; > + CPUID_MEMORY_ENCRYPTION_INFO_EAX Eax; > + BOOLEAN ReadSevMsr; > + SEC_SEV_ES_WORK_AREA *SevEsWorkArea; > + > + ReadSevMsr = FALSE; > + > + SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase); > + if (SevEsWorkArea != NULL && SevEsWorkArea->EncryptionMask != 0) { > + // > + // The MSR has been read before, so it is safe to read it again and avoid > + // having to validate the CPUID information. > + // > + ReadSevMsr = TRUE; > + } else { > + // > + // Check if memory encryption leaf exist > + // > + AsmCpuid (CPUID_EXTENDED_FUNCTION, &RegEax, NULL, NULL, NULL); > + if (RegEax >= CPUID_MEMORY_ENCRYPTION_INFO) { > + // > + // CPUID Fn8000_001F[EAX] Bit 1 (Sev supported) > + // > + AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, &Eax.Uint32, NULL, NULL, NULL); > + > + if (Eax.Bits.SevBit) { > + ReadSevMsr = TRUE; > + } > + } > + } > + > + return ReadSevMsr ? AsmReadMsr32 (MSR_SEV_STATUS) : 0; > +} > + > +/** > + Returns a boolean to indicate whether SEV-ES is enabled. > + > + @retval TRUE SEV-ES is enabled > + @retval FALSE SEV-ES is not enabled > +**/ > +BOOLEAN > +EFIAPI > +MemEncryptSevEsIsEnabled ( > + VOID > + ) > +{ > + MSR_SEV_STATUS_REGISTER Msr; > + > + Msr.Uint32 = InternalMemEncryptSevStatus (); > + > + return Msr.Bits.SevEsBit ? TRUE : FALSE; > +} > + > +/** > + Returns a boolean to indicate whether SEV is enabled. > + > + @retval TRUE SEV is enabled > + @retval FALSE SEV is not enabled > +**/ > +BOOLEAN > +EFIAPI > +MemEncryptSevIsEnabled ( > + VOID > + ) > +{ > + MSR_SEV_STATUS_REGISTER Msr; > + > + Msr.Uint32 = InternalMemEncryptSevStatus (); > + > + return Msr.Bits.SevBit ? TRUE : FALSE; > +} > + > +/** > + Returns the SEV encryption mask. > + > + @return The SEV pagtable encryption mask > +**/ > +UINT64 > +EFIAPI > +MemEncryptSevGetEncryptionMask ( > + VOID > + ) > +{ > + CPUID_MEMORY_ENCRYPTION_INFO_EBX Ebx; > + SEC_SEV_ES_WORK_AREA *SevEsWorkArea; > + UINT64 EncryptionMask; > + > + SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase); > + if (SevEsWorkArea != NULL) { > + EncryptionMask = SevEsWorkArea->EncryptionMask; > + } else { > + // > + // CPUID Fn8000_001F[EBX] Bit 0:5 (memory encryption bit position) > + // > + AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, NULL, &Ebx.Uint32, NULL, NULL); > + EncryptionMask = LShiftU64 (1, Ebx.Bits.PtePosBits); > + } > + > + return EncryptionMask; > +} > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > similarity index 95% > rename from OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c > rename to OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > index 6422bc53bd5d..3a5bab657bd7 100644 > --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiDxeVirtualMemory.c > @@ -192,7 +192,8 @@ Split2MPageTo4K ( > { > PHYSICAL_ADDRESS PhysicalAddress4K; > UINTN IndexOfPageTableEntries; > - PAGE_TABLE_4K_ENTRY *PageTableEntry, *PageTableEntry1; > + PAGE_TABLE_4K_ENTRY *PageTableEntry; > + PAGE_TABLE_4K_ENTRY *PageTableEntry1; > UINT64 AddressEncMask; > > PageTableEntry = AllocatePageTableMemory(1); > @@ -472,7 +473,7 @@ Split1GPageTo2M ( > /** > Set or Clear the memory encryption bit > > - @param[in] PagetablePoint Page table entry pointer (PTE). > + @param[in, out] PageTablePointer Page table entry pointer (PTE). > @param[in] Mode Set or Clear encryption bit > > **/ > @@ -562,7 +563,6 @@ EnableReadOnlyPageWriteProtect ( > @retval RETURN_UNSUPPORTED Setting the memory encyrption attribute > is not supported > **/ > - > STATIC > RETURN_STATUS > EFIAPI > @@ -635,7 +635,7 @@ SetMemoryEncDec ( > > Status = EFI_SUCCESS; > > - while (Length) > + while (Length != 0) > { > // > // If Cr3BaseAddress is not specified then read the current CR3 > @@ -683,7 +683,7 @@ SetMemoryEncDec ( > // Valid 1GB page > // If we have at least 1GB to go, we can just update this entry > // > - if (!(PhysicalAddress & (BIT30 - 1)) && Length >= BIT30) { > + if ((PhysicalAddress & (BIT30 - 1)) == 0 && Length >= BIT30) { > SetOrClearCBit(&PageDirectory1GEntry->Uint64, Mode); > DEBUG (( > DEBUG_VERBOSE, > @@ -744,7 +744,7 @@ SetMemoryEncDec ( > // Valid 2MB page > // If we have at least 2MB left to go, we can just update this entry > // > - if (!(PhysicalAddress & (BIT21-1)) && Length >= BIT21) { > + if ((PhysicalAddress & (BIT21-1)) == 0 && Length >= BIT21) { > SetOrClearCBit (&PageDirectory2MEntry->Uint64, Mode); > PhysicalAddress += BIT21; > Length -= BIT21; (3) The style fixes in this file seem unrelated to the subject. Please split them to a different patch. (Were they motivated by ECC?) Looks OK otherwise. Thanks! Laszlo > diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c > new file mode 100644 > index 000000000000..5c337ea0b820 > --- /dev/null > +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecVirtualMemory.c > @@ -0,0 +1,80 @@ > +/** @file > + > + Virtual Memory Management Services to set or clear the memory encryption bit > + > + Copyright (c) 2020, AMD Incorporated. All rights reserved.
> + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include > + > +#include "VirtualMemory.h" > + > +/** > + This function clears memory encryption bit for the memory region specified by > + PhysicalAddress and Length from the current page table context. > + > + @param[in] Cr3BaseAddress Cr3 Base Address (if zero then use > + current CR3) > + @param[in] PhysicalAddress The physical address that is the start > + address of a memory region. > + @param[in] Length The length of memory region > + @param[in] Flush Flush the caches before applying the > + encryption mask > + > + @retval RETURN_SUCCESS The attributes were cleared for the > + memory region. > + @retval RETURN_INVALID_PARAMETER Number of pages is zero. > + @retval RETURN_UNSUPPORTED Clearing the memory encyrption attribute > + is not supported > +**/ > +RETURN_STATUS > +EFIAPI > +InternalMemEncryptSevSetMemoryDecrypted ( > + IN PHYSICAL_ADDRESS Cr3BaseAddress, > + IN PHYSICAL_ADDRESS PhysicalAddress, > + IN UINTN Length, > + IN BOOLEAN Flush > + ) > +{ > + // > + // This function is not available during SEC. > + // > + return RETURN_UNSUPPORTED; > +} > + > +/** > + This function sets memory encryption bit for the memory region specified by > + PhysicalAddress and Length from the current page table context. > + > + @param[in] Cr3BaseAddress Cr3 Base Address (if zero then use > + current CR3) > + @param[in] PhysicalAddress The physical address that is the start > + address of a memory region. > + @param[in] Length The length of memory region > + @param[in] Flush Flush the caches before applying the > + encryption mask > + > + @retval RETURN_SUCCESS The attributes were set for the memory > + region. > + @retval RETURN_INVALID_PARAMETER Number of pages is zero. > + @retval RETURN_UNSUPPORTED Setting the memory encyrption attribute > + is not supported > +**/ > +RETURN_STATUS > +EFIAPI > +InternalMemEncryptSevSetMemoryEncrypted ( > + IN PHYSICAL_ADDRESS Cr3BaseAddress, > + IN PHYSICAL_ADDRESS PhysicalAddress, > + IN UINTN Length, > + IN BOOLEAN Flush > + ) > +{ > + // > + // This function is not available during SEC. > + // > + return RETURN_UNSUPPORTED; > +} >