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.8645.1609942977352102432 for ; Wed, 06 Jan 2021 06:22:57 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=ZEcQ3sYX; 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=1609942976; 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=+9fk8dpyfdDorH8s/pIBsjoGe4+jMaAbYt8N6OqlksY=; b=ZEcQ3sYX1CRvzuXSOUepMVVS/LDP+dgFah8p+bYj7TAaPqFf2kxxrWsMvNvh/V4MCks5nY jJkgL3r0ZhpMgyCSPDCeEGRWdeRtiMD8kF6dILBrqb8jKBF+YvP3kQDTnLfS9SKTvqw1ZF tJvyOTjwlUIklktiS8dBo4nEXmkHuHA= 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-174--MMZKIv-MP64KshxAmV0wg-1; Wed, 06 Jan 2021 09:22:52 -0500 X-MC-Unique: -MMZKIv-MP64KshxAmV0wg-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 4B7D4180A094; Wed, 6 Jan 2021 14:22:51 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-59.ams2.redhat.com [10.36.114.59]) by smtp.corp.redhat.com (Postfix) with ESMTP id CB61260BE5; Wed, 6 Jan 2021 14:22:49 +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> <3bd1ca24-c743-5688-9ec5-2af467a8c595@redhat.com> From: "Laszlo Ersek" Message-ID: <1cbce2e4-73b3-dcd2-e0d8-e0f1271287c8@redhat.com> Date: Wed, 6 Jan 2021 15:22:48 +0100 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 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 01/05/21 16:38, Lendacky, Thomas wrote: > On 1/5/21 8:34 AM, Tom Lendacky wrote: >> On 1/5/21 3:40 AM, Laszlo Ersek wrote: >>> On 12/15/20 21:51, Lendacky, Thomas wrote: >>>> From: Tom Lendacky >>>> >>>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3108&data=04%7C01%7Cthomas.lendacky%40amd.com%7C1440a9afd7f1450ba93d08d8b15e02a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637454364641627971%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=P52g0gS3SEhdgkF2qRY6l1J8%2FLjJm1DNR3LLlEmKSBk%3D&reserved=0 >>>> >>>> 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) >> >> Yup, I'll fix that. >> >>> >>>> >>>> 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). >> >> The PCD is used in MemEncryptSevLocateInitialSmramSaveStateMapPages() in >> the MemEncryptSevLibInternal.c file, which is part of the library. Because >> of that, I assumed that it needed to be added even though the function >> that uses it isn't called during SEC. >> >> I'll remove it. > > Removing it does cause an error. > > If we really don't want to include this PCD, I can create SEC and PEI/DXE > specific versions of the MemEncryptSevLibInternal.c file and just return > RETURN_UNSUPPORTED for the SEC version of > MemEncryptSevLocateInitialSmramSaveStateMapPages(). This one seems like the best way forward. Thanks! Laszlo > > Alternatively, I can just remove the MemEncryptSevLibInternal.c file from > the build of the SEC library. This should be ok during SEC because there > are no calls to MemEncryptSevLocateInitialSmramSaveStateMapPages(). If, > for some reason a call is added later, then the build will fail, but it > should be obvious why it failed. > > Or I can just leave the FeaturePcd section in the SEC inf file. > > Thoughts?