From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM03-BY2-obe.outbound.protection.outlook.com (mail-by2nam03on060a.outbound.protection.outlook.com [IPv6:2a01:111:f400:fe4a::60a]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 8535380368 for ; Wed, 8 Mar 2017 06:56:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amdcloud.onmicrosoft.com; s=selector1-amd-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=ZI/bfze8LKcW0vG4bf28Ptnck0InAQwZZAqvkabIpSc=; b=pghAxaAocAACml2DxuXHOyygEksloySfFaX3qgg1/eJnqUZ/GLralsKWQOoFU6+DSuYBKCP1C9SSjcLsk/nZ7Qd1wjmjE8kHSOpkrIxiZ75j+Wyd4uW4lq5fG93QzC0KEPVXH/p1FlmTBroAEAgsmYAWaNZob+O0iuEL2zKUQwQ= Received: from DM5PR12MB1243.namprd12.prod.outlook.com (10.168.237.22) by DM5PR12MB1611.namprd12.prod.outlook.com (10.172.40.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P384) id 15.1.947.12; Wed, 8 Mar 2017 14:56:38 +0000 Received: from DM5PR12MB1243.namprd12.prod.outlook.com ([10.168.237.22]) by DM5PR12MB1243.namprd12.prod.outlook.com ([10.168.237.22]) with mapi id 15.01.0947.020; Wed, 8 Mar 2017 14:56:36 +0000 From: "Duran, Leo" To: 'Laszlo Ersek' , Brijesh Singh CC: "jordan.l.justen@intel.com" , "edk2-devel@ml01.01.org" , "Singh, Brijesh" , "Lendacky, Thomas" Thread-Topic: [edk2] [RFC PATCH v1 2/5] OvmfPkg/MemcryptSevLib: Add SEV helper library Thread-Index: AQHSltFC3Ijqm5RzH0KOWILkGW0qzaGJnJqAgAAj0QCAADCUAIABF+Cg Date: Wed, 8 Mar 2017 14:56:36 +0000 Message-ID: References: <148884284887.29188.7643544710695103939.stgit@brijesh-build-machine> <148884286215.29188.1084675072356724555.stgit@brijesh-build-machine> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=leo.duran@amd.com; x-originating-ip: [165.204.77.1] x-microsoft-exchange-diagnostics: 1; DM5PR12MB1611; 7:UX0A2I6U4bGleWrE2XEdqKNKFto+Csjj+Oun0SUG6tYesqyRQdQesNH7J5l0Xr3GYG148114wGogfoxtvT2anjq+W5NHD0isDO5tJ80SVNMM8Y70CSixYT4UzOIsRMgfMPDau+K2tMdIznKskWlsNmwVXLvbuVOff1qDTcz07EajLTq0NZg1xuRZ0k2VAPMVZhIiRkA4e6/T2b3OAHgeC0thZWqrDFR5iAQBmLGnzokzKf/gHgdJFmaQ2x2i+bMUI5xUwJ8mIR8T0MlKlJpp9MhJf8yQ0m1sIOzkG1i3qB7G2eOWlaRcNnlH/a5CFyuslHGmDWzzEapH7Iu6k1CsXg==; 20:fqodH7u50PUOWp6KQdTFc/RwHjI03plZV/Z/6cL+sgI4b/ct0eSnkdmLcNeYp+QXHMLLI1RhZZrSU9JuXpupB2w2efSTBij0ewdgSz6oedHLRvJdZ6MeaQIWhl7ic6XWT1N+7GfR7XQsbulhXMNZZynL2mGcvSAB6XpeOV6qhmcc8jNK6+9/b7ZW4tmbhbZb0iD+s4zJGgo+CAkHsnrd09jqh7MBOk54MNVa537uc5B6Z3UvvRRkDeKG2AdQS/TC x-forefront-antispam-report: SFV:SKI; SCL:-1SFV:NSPM; SFS:(10009020)(6009001)(39450400003)(39840400002)(39850400002)(39410400002)(39860400002)(377454003)(13464003)(24454002)(74316002)(7736002)(189998001)(50986999)(54356999)(76176999)(305945005)(106116001)(33656002)(93886004)(122556002)(15188155005)(16799955002)(38730400002)(3280700002)(966004)(6246003)(53376002)(39060400002)(4326008)(53936002)(8936002)(8676002)(5660300001)(81166006)(2950100002)(7696004)(77096006)(6436002)(66066001)(3846002)(53946003)(9686003)(102836003)(25786008)(6506006)(6306002)(54906002)(2900100001)(55016002)(6116002)(99286003)(86362001)(229853002)(53546006)(3660700001)(2906002)(575784001)(217873001)(19627235001); DIR:OUT; SFP:1101; SCL:1; SRVR:DM5PR12MB1611; H:DM5PR12MB1243.namprd12.prod.outlook.com; FPR:; SPF:None; MLV:ovrnspm; PTR:InfoNoRecords; LANG:en; x-ms-office365-filtering-correlation-id: 8b31fbb2-7021-45fd-d860-08d4663351b1 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: UriScan:; BCL:0; PCL:0; RULEID:(22001)(48565401081); SRVR:DM5PR12MB1611; x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:(244540007438412)(166708455590820)(767451399110)(228905959029699); x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(6040375)(601004)(2401047)(8121501046)(5005006)(3002001)(10201501046)(6055026)(6041248)(20161123560025)(20161123555025)(20161123558025)(20161123564025)(20161123562025)(6072148); SRVR:DM5PR12MB1611; BCL:0; PCL:0; RULEID:; SRVR:DM5PR12MB1611; x-forefront-prvs: 02408926C4 received-spf: None (protection.outlook.com: amd.com does not designate permitted sender hosts) spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM MIME-Version: 1.0 X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Mar 2017 14:56:36.3925 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1611 Subject: Re: [RFC PATCH v1 2/5] OvmfPkg/MemcryptSevLib: Add SEV helper library X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Mar 2017 14:56:42 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable For libraries, I've noticed this usage pattern: INF File: - Name: .inf - Path: xxxPkg/Library//.inf INCLUDE File: - Name: .h - Path: xxxPkg/Include/Library/.h Leo > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Tuesday, March 07, 2017 4:08 PM > To: Brijesh Singh > Cc: jordan.l.justen@intel.com; edk2-devel@ml01.01.org; Duran, Leo > ; Singh, Brijesh ; Lendacky, > Thomas > Subject: Re: [edk2] [RFC PATCH v1 2/5] OvmfPkg/MemcryptSevLib: Add SEV > helper library >=20 > On 03/07/17 20:14, Brijesh Singh wrote: > > On Tue, Mar 7, 2017 at 11:06 AM, Laszlo Ersek wrote= : > > > >> On 03/07/17 00:27, Brijesh Singh wrote: > >>> The library contain common helper routines for SEV feature. > >>> > >>> Contributed-under: TianoCore Contribution Agreement 1.0 > >>> Signed-off-by: Brijesh Singh > >>> --- > >>> OvmfPkg/Include/Library/MemcryptSevLib.h | 42 > +++++++++++++ > >>> OvmfPkg/Library/MemcryptSevLib/MemcryptSevLib.c | 66 > >> +++++++++++++++++++++ > >>> OvmfPkg/Library/MemcryptSevLib/MemcryptSevLib.inf | 44 > ++++++++++++++ > >>> OvmfPkg/OvmfPkgIa32X64.dsc | 4 + > >>> OvmfPkg/OvmfPkgX64.dsc | 4 + > >>> 5 files changed, 160 insertions(+) > >>> create mode 100644 OvmfPkg/Include/Library/MemcryptSevLib.h > >>> create mode 100644 > OvmfPkg/Library/MemcryptSevLib/MemcryptSevLib.c > >>> create mode 100644 > >>> OvmfPkg/Library/MemcryptSevLib/MemcryptSevLib.inf > >> > >> New files -- can you please double-check they are CRLF terminated? > >> > >> > > This version does not have CRLF, I will ensure that nex rev contains CR= LF. > > > > > >>> > >>> diff --git a/OvmfPkg/Include/Library/MemcryptSevLib.h > >> b/OvmfPkg/Include/Library/MemcryptSevLib.h > >>> new file mode 100644 > >>> index 0000000..89f9c86 > >>> --- /dev/null > >>> +++ b/OvmfPkg/Include/Library/MemcryptSevLib.h > >>> @@ -0,0 +1,42 @@ > >>> +/** @file > >> > >> Please add one or two sentences about the purpose of this library clas= s. > > > > > > Will do. > > > > > >> > >> > >> + Copyright (C) 2017 Advanced Micro Devices. > >>> + > >>> + This program and the accompanying materials are licensed and made > >> available > >>> + under the terms and conditions of the BSD License which > >>> + accompanies > >> this > >>> + distribution. The full text of the license may be found at > >>> + http://opensource.org/licenses/bsd-license.php > >>> + > >>> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS > IS" > >>> + BASIS, > >> WITHOUT > >>> + WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS > OR IMPLIED. > >>> +**/ > >>> + > >>> +#ifndef __MEMCRYPT_SEV_LIB_H__ > >>> +#define __MEMCRYPT_SEV_LIB_H__ > >>> + > >>> +#include > >> > >> I think it shouldn't be necessary to include this, especially not for > >> a BASE library (class). What type exactly did you need this include fo= r? > >> > >> > > I should be able to remove the inclusion of this header. > > > > > >>> +#include > >>> + > >>> +/** > >>> + > >>> + Initialize SEV memory encryption > >>> + > >>> +**/ > >>> + > >>> +RETURN_STATUS > >>> +EFIAPI > >>> +MemcryptSevInitialize ( > >>> + VOID > >>> + ); > >>> + > >>> +/** > >>> + > >>> + Return TRUE when SEV is active otherwise FALSE > >>> + > >>> + **/ > >> > >> Is this function restricted to code that runs after a successful > >> invocation of MemcryptSevInitialize()? If so, please document that. > >> > >> > > Both are independent functions, SevActive() function returns whether > > SEV is active or not. Whereas SevInitialize(), calls SevActive() and > > if SEV is active then it sets the dynamic PtePcdMemoryEncryptionMask. > > > > > > Please also document the return values (with @retval and/or @return), > >> even though the explanation is likely trivial. > >> > >> I will document it. > > > > > > > >>> +BOOLEAN > >>> +EFIAPI > >>> +SevActive ( > >>> + VOID > >>> + ); > >>> + > >> > >> I'd prefer if we could use a common prefix for the two functions, > >> something that is unique to / characteristic of the new library class. > >> > >> > > Will fix it. In general, are we are okay with MemcryptSevLib naming > > convensio ? >=20 > Regarding library naming conventions, we can consider functions, and libr= ary > instance names. >=20 > For functions, I tend to prefer a common prefix, although I don't believe= this > is a hard requirement in edk2. >=20 > For library instance names, the naming convention seems to be: >=20 > LibClassName >=20 > Where describes the phases the library instance can be used in (s= o, > Base, Dxe, Pei, PeiDxe, and so on), and describes, very tersely= , > the underlying implementation (for example, Null means the library does > nothing). I think can be omitted if there's only one library > instance. >=20 >=20 > > If we are okay with that then I was going to prefix all the function > > with Sev e.g > > > > SevInitialize() : this sets the dynamic PCD > > SevActive() : returns TRUE when SEV is active > > > >> +#endif > >>> +#include > >>> +#include > >>> +#include > >>> +#include > >>> + > >>> +#define KVM_FEATURE_MEMORY_ENCRYPTION 0x100 > >> > >> Can you move all these magic constants (CPUID leaves as well) to > >> OvmfPkg/Include/IndustryStandard/? I guess the filename could be > >> "AmdSev.h", or something similar. > >> > >> Such header files are also prime locations to capture the great > >> standards references that you added to the blurb. > >> > >> > > Will do it. > > > > > >>> + > >>> +RETURN_STATUS > >>> +EFIAPI > >>> +MemcryptSevInitialize ( > >>> + VOID > >>> + ) > >>> +{ > >>> + UINT32 EBX; > >> > >> Should be Ebx, IMO. > >> > >> > > Will fix it > > > >> + UINT64 MeMask =3D 0; > >> > >> Initialization of local variables is forbidden by the edk2 coding styl= e. > >> > >> > > Will fix it. > > > > > >>> + > >>> + if (SevActive ()) { > >> > >> Aha! So it's the other way around than I expected. > >> > >>> + // CPUID Fn8000_001f[EBX] - Bit 5:0 (memory encryption bit > >> position) > >> > >> Comment style is > >> > >> // > >> // CPUID ... > >> // > >> > >> > > Will fix it. > > > > > >>> + AsmCpuid(0x8000001f, NULL, &EBX, NULL, NULL); > >>> + MeMask =3D (1ULL << (EBX & 0x3f)); > >> > >> You can't bit shift 64-bit values in edk2 with the C-language > >> operator; it won't build for Ia32. > >> > >> Please either use LShiftU64() here, or -- if you are sure the code > >> will never be reached in Ia32 guests -- use (UINTN)1, and shift that w= ith > <<. > >> > >> > > I will fix it to use LShiftU64() version. > > > > > >> BTW, in what PI phases do you intend to use this library? For > >> example, in the Ia32X64 build of OVMF, the PEI phase runs as 32-bit, > >> and the DXE phase runs in 64-bit. > >> > >> > > So my rational behind creating a new library was to have a flexibilty > > of adding more SEV specifc functions. The functions which I have mind i= s: > > > > SevChangeMemoryAttributeEncrypted(Address, Length) : set the > > encryption attribute SevChangeMemoryAttributeDecrypted(Address, > > Length) : clear the encryption attribute > > > > SevChangeMemoryAttribute*() are not implemented in this RFC and I am > > working to add in next rev. This will be mainly execute in Dxe phase. > > These functions will be update the pagetable to clear/set encryption > > masks. It will be mainly used for Dma libraries and possibly > > QemuVideoDxe (since framebuffer is shared between HV and Guest > hence > > we will need to clear the encryption attribute of framebuffer memory). > > > > I would potentially add two libraries: > > > > * SevBaseLib: it provides SevActive() and SevInitialize() routines > > which can be called anytime > > * SevDxeLib: it will provide routines which can be called used by Dxe > > drivers. > > > > Does it make sense to you. I am open to suggestions. >=20 > I think SevActive() and SevInitialize() should become part of PlatformPei= only > (if that's possible). >=20 > The upcoming PTE massaging functions could become part of the DMA lib > stuff that you mention (as functions with external linkage), and then you > could pull the DMA lib into QemuVideoDxe just to make these functions > available. >=20 > Presently the suggested functions don't seem to justify two (or even > one) new libclass. >=20 > Thanks > Laszlo >=20 > > > > Hm, in the next patch, it seems that the library is put to use in > >> PlatformPei (and only there). Since these functions are really small, > >> I think I would prefer having a new file under OvmfPkg/PlatformPei onl= y. > >> > >> More on the Ia32X64 build of OVMF -- assuming the PEI phase runs in > >> 32-bit, but the DXE phase (and the OS) run in 64-bit, does SEV appear > >> active when the 32-bit PlatformPei module queries it? > >> > >> Yes SEV will appear active on both Ia32X64 and X64. I have tried > >> running > > the OVMF build with both version. > > > > > >> * If it doesn't, that's a problem, because then the PCD will be set > >> incorrectly. In this case, it might make sense to limit SEV only to > >> the > >> X64 build of OVMF. > >> > >> * If SEV does appear active when the 32-bit PlatformPei module > >> queries it, then we definitely need to use LShiftU64 here. > >> > >> Have you tested this series in all three builds of OVMF? (Ia32, > >> Ia32X64, and X64?) I'm not asking about SMM, I can see it's on the TOD= O > list. > >> > >> I have tried Ia3264 and X64 but have not tried Ia32. I will try to > >> and let > > you know If I find any issues. > > > > > >>> + DEBUG ((DEBUG_INFO, "KVM Secure Encrypted Virtualization (SEV) > >>> + is > >> enabled\n")); > >>> + DEBUG ((DEBUG_INFO, "MemEncryptionMask 0x%lx\n", MeMask)); > } > >>> + > >>> + PcdSet64S (PcdPteMemoryEncryptionAddressOrMask, MeMask); > >> > >> Whiile we do exped PcdSet64S to succeed, please add a separate Status > >> variable, and add an ASSERT_RETURN_ERROR on that. > >> > >> > > Will do > > > >> + > >>> + return RETURN_SUCCESS; > >>> +} > >>> + > >>> +BOOLEAN > >>> +EFIAPI > >>> +SevActive ( > >>> + VOID > >>> + ) > >>> +{ > >>> + UINT32 KVMFeatures, EAX; > >> > >> Should be KvmFeatures and Eax. > >> > >>> + > >>> + // Check if KVM memory encyption feature is set > >> > >> comment style > >> > >>> + AsmCpuid(0x40000001, &KVMFeatures, NULL, NULL, NULL); if > >>> + (KVMFeatures & KVM_FEATURE_MEMORY_ENCRYPTION) { > >>> + > >>> + // Check whether SEV is enabled > >>> + // CPUID Fn8000_001f[EAX] - Bit 0 (SEV is enabled) > >>> + AsmCpuid(0x8000001f, &EAX, NULL, NULL, NULL); > >> > >> Tom commented on this -- the result is not checked. > >> > >> > > Will fix it. > > > > > >>> + > >>> + return TRUE; > >>> + } > >>> + > >>> + return FALSE; > >>> +} > >>> + > >>> diff --git a/OvmfPkg/Library/MemcryptSevLib/MemcryptSevLib.inf > >> b/OvmfPkg/Library/MemcryptSevLib/MemcryptSevLib.inf > >>> new file mode 100644 > >>> index 0000000..8e8d7e0 > >>> --- /dev/null > >>> +++ b/OvmfPkg/Library/MemcryptSevLib/MemcryptSevLib.inf > >>> @@ -0,0 +1,44 @@ > >>> +## @file > >>> +# > >>> +# Copyright (c) 2017 Advanced Micro Devices. All rights > >>> +reserved.
# # This program and the accompanying materials # > >>> +are licensed and made available under the terms and conditions of > >> the BSD License > >>> +# which accompanies this distribution. The full text of the > >>> +license > >> may be found at > >>> +# http://opensource.org/licenses/bsd-license.php > >>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS > IS" > >>> +BASIS, # WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER > >>> +EXPRESS OR > >> IMPLIED. > >>> +# > >>> +# > >>> +## > >>> + > >>> +[Defines] > >>> + INF_VERSION =3D 0x00010005 > >>> + BASE_NAME =3D MemcryptSevLib > >>> + FILE_GUID =3D c1594631-3888-4be4-949f-9c630db= c842b > >>> + MODULE_TYPE =3D BASE > >>> + VERSION_STRING =3D 1.0 > >>> + LIBRARY_CLASS =3D MemcryptSevLib > >>> + > >>> +# > >>> +# The following information is for reference only and not required > >>> +by > >> the build tools. > >>> +# > >>> +# VALID_ARCHITECTURES =3D IA32 X64 > >>> +# > >>> + > >>> +[Packages] > >>> + MdePkg/MdePkg.dec > >>> + MdeModulePkg/MdeModulePkg.dec > >>> + OvmfPkg/OvmfPkg.dec > >>> + UefiCpuPkg/UefiCpuPkg.dec > >>> + > >>> +[Sources] > >>> + MemcryptSevLib.c > >>> + > >>> +[LibraryClasses] > >>> + BaseLib > >>> + DebugLib > >>> + PcdLib > >>> + > >>> +[Pcd] > >>> + > >>> > +gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOr > Mask > >>> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc > b/OvmfPkg/OvmfPkgIa32X64.dsc > >>> index 56f7ff9..a35e1d2 100644 > >>> --- a/OvmfPkg/OvmfPkgIa32X64.dsc > >>> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc > >>> @@ -129,6 +129,7 @@ > >>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf > >>> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf > >>> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf > >>> + > MemcryptSevLib|OvmfPkg/Library/MemcryptSevLib/MemcryptSevLib.inf > >>> !if $(SMM_REQUIRE) =3D=3D FALSE > >>> LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf > >>> !endif > >> > >> Please configure your git instance so that it includes the section > >> names of INI-style files in hunk headers: > >> > >> https://github.com/tianocore/tianocore.github.io/wiki/ > >> Laszlo's-unkempt-git-guide-for-edk2-contributors-and- > >> maintainers#contrib-05 > >> > >> see xfuncname there, and: > >> > >> https://github.com/tianocore/tianocore.github.io/wiki/ > >> Laszlo's-unkempt-git-guide-for-edk2-contributors-and- > >> maintainers#contrib-09 > >> > >> > > > > I will go through INI-Style file and git setting etc. > > > > > >> > >>> @@ -509,6 +510,9 @@ > >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64 > >>> > gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000 > >>> > >>> + # Set memory encryption mask > >>> + > gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressO > >> rMask|0x0 > >>> + > >>> !if $(SMM_REQUIRE) =3D=3D TRUE > >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01 > >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000 > >>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc > index > >>> d0b0b0e..5d853d6 100644 > >>> --- a/OvmfPkg/OvmfPkgX64.dsc > >>> +++ b/OvmfPkg/OvmfPkgX64.dsc > >>> @@ -129,6 +129,7 @@ > >>> QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf > >>> VirtioLib|OvmfPkg/Library/VirtioLib/VirtioLib.inf > >>> LoadLinuxLib|OvmfPkg/Library/LoadLinuxLib/LoadLinuxLib.inf > >>> + > MemcryptSevLib|OvmfPkg/Library/MemcryptSevLib/MemcryptSevLib.inf > >>> !if $(SMM_REQUIRE) =3D=3D FALSE > >>> LockBoxLib|OvmfPkg/Library/LockBoxLib/LockBoxBaseLib.inf > >>> !endif > >>> @@ -508,6 +509,9 @@ > >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64 > >>> > gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|50000 > >>> > >>> + # Set memory encryption mask > >>> + > gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressO > >> rMask|0x0 > >>> + > >>> !if $(SMM_REQUIRE) =3D=3D TRUE > >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x01 > >>> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmApSyncTimeout|100000 > >>> > >> > >> The OvmfPkgIa32.dsc file is not modified. The MemcryptSevLib class is > >> not resolved for the Ia32 build, hence PlatformPei will fail to build > >> in the next patch. > >> > >> Please build and regression-test all three builds of OVMF (SMM > >> disabled, for now) in the development of this feature. > >> > >> Regression-testing should in particular include S3 suspend/resume > >> (again, with SMM disabled, for now). If it breaks (and it is expected > >> to break), please extend the S3Verification() function so that it > >> prevents the firmware from booting (with a reasonable error message) > >> if it sees that SEV is active and S3 was *not* disabled on the QEMU > command line. > >> Losing a guest at S3 resume is very annoying, it's better not to boot > >> in that case (and to ask the user to disable S3 on the QEMU command > line). > >> > >> Anyhow, I think these functions should go directly under > >> OvmfPkg/PlatformPei, into a new file. > >> > >> I may have missed a few things, but I'm (theoretically) at a > >> conference, and right now it seems more correct for me to give > >> (perhaps spotty) feedback *quickly*, than to let my backlog pile up. > >> > >> > > Appriciate your quick feedbacks, I will go through each of them and > > will try to address in v2. > > > > > >> Thanks! > >> Laszlo > >> > > > > > >