[edk2-devel] [PATCH 08/12] OvmfPkg/MemEncryptSevLib: Make the MemEncryptSevLib available for SEC
Laszlo Ersek
lersek at redhat.com
Wed Jan 6 14:22:48 UTC 2021
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 <thomas.lendacky at amd.com>
>>>>
>>>> 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 <jordan.l.justen at intel.com>
>>>> Cc: Laszlo Ersek <lersek at redhat.com>
>>>> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
>>>> Cc: Brijesh Singh <brijesh.singh at amd.com>
>>>> Signed-off-by: Tom Lendacky <thomas.lendacky at amd.com>
>>>> ---
>>>> .../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.<BR>
>>>> +#
>>>> +# 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?
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69826): https://edk2.groups.io/g/devel/message/69826
Mute This Topic: https://groups.io/mt/78986172/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list