[edk2-devel] [PATCH 1/3] OvmfPkg: introduce a common work area
Brijesh Singh via groups.io
brijesh.singh=amd.com at groups.io
Mon Aug 9 17:18:17 UTC 2021
On 8/9/21 11:40 AM, Tom Lendacky wrote:
> On 8/4/21 3:20 PM, Brijesh Singh wrote:
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
>>
>> Both the TDX and SEV support needs to reserve a page in MEMFD as a work
>> area. The page will contain meta data specific to the guest type.
>> Currently, the SEV-ES support reserves a page in MEMFD
>> (PcdSevEsWorkArea) for the work area. This page can be reused as a TDX
>> work area when Intel TDX is enabled.
>>
>> Based on the discussion [1], it was agreed to rename the SevEsWorkArea
>> to the OvmfWorkArea, and add a header that can be used to indicate the
>> work area type.
>>
>> [1] https://edk2.groups.io/g/devel/message/78262?p=,,,20,0,0,0::\
>> created,0,SNP,20,2,0,84476064
>>
>> Cc: James Bottomley <jejb at linux.ibm.com>
>> Cc: Min Xu <min.m.xu at intel.com>
>> Cc: Jiewen Yao <jiewen.yao at intel.com>
>> Cc: Tom Lendacky <thomas.lendacky at amd.com>
>> Cc: Jordan Justen <jordan.l.justen at intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
>> Cc: Erdem Aktas <erdemaktas at google.com>
>> Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
>> ---
>> OvmfPkg/OvmfPkg.dec | 6 +++
>> OvmfPkg/OvmfPkgX64.fdf | 9 +++-
>> OvmfPkg/PlatformPei/PlatformPei.inf | 4 +-
>> OvmfPkg/Include/Library/MemEncryptSevLib.h | 21 +--------
>> OvmfPkg/Include/WorkArea.h | 53 ++++++++++++++++++++++
>> OvmfPkg/PlatformPei/MemDetect.c | 32 ++++++-------
>> 6 files changed, 85 insertions(+), 40 deletions(-)
>> create mode 100644 OvmfPkg/Include/WorkArea.h
>>
>> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
>> index 2ab27f0c73c2..9d31ec45c78a 100644
>> --- a/OvmfPkg/OvmfPkg.dec
>> +++ b/OvmfPkg/OvmfPkg.dec
>> @@ -330,6 +330,12 @@ [PcdsFixedAtBuild]
>> gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableBase|0x0|UINT32|0x47
>> gUefiOvmfPkgTokenSpaceGuid.PcdQemuHashTableSize|0x0|UINT32|0x48
>>
>> + ## The base address and size of the work area used during the SEC
>> + # phase by the SEV and TDX supports.
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|0|UINT32|0x49
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize|0|UINT32|0x50
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaHeaderSize|4|UINT32|0x51
>> +
>> [PcdsDynamic, PcdsDynamicEx]
>> gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10
>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
>> index 5fa8c0895808..418e0ea5add4 100644
>> --- a/OvmfPkg/OvmfPkgX64.fdf
>> +++ b/OvmfPkg/OvmfPkgX64.fdf
>> @@ -83,7 +83,7 @@ [FD.MEMFD]
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
>>
>> 0x00B000|0x001000
>> -gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
>> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize
>>
>> 0x00C000|0x001000
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
>> @@ -99,6 +99,13 @@ [FD.MEMFD]
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
>> FV = DXEFV
>>
>> +##########################################################################################
>> +# SEV specific PCD settings
>> +SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaHeaderSize = 0x4
>> +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase = $(MEMFD_BASE_ADDRESS) + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaHeaderSize
>> +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaHeaderSize
>> +##########################################################################################
>> +
>> ################################################################################
>>
>> [FV.SECFV]
>> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf
>> index 89d1f7636870..67eb7aa7166b 100644
>> --- a/OvmfPkg/PlatformPei/PlatformPei.inf
>> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf
>> @@ -116,8 +116,8 @@ [FixedPcd]
>> gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiRuntimeServicesData
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase
>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupSize
>> - gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
>> - gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize
>>
>> [FeaturePcd]
>> gUefiOvmfPkgTokenSpaceGuid.PcdCsmEnable
>> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h b/OvmfPkg/Include/Library/MemEncryptSevLib.h
>> index 76d06c206c8b..adc490e466ec 100644
>> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
>> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
>> @@ -12,6 +12,7 @@
>> #define _MEM_ENCRYPT_SEV_LIB_H_
>>
>> #include <Base.h>
>> +#include <WorkArea.h>
>>
>> //
>> // Define the maximum number of #VCs allowed (e.g. the level of nesting
>> @@ -36,26 +37,6 @@ typedef struct {
>> VOID *GhcbBackupPages;
>> } SEV_ES_PER_CPU_DATA;
>>
>> -//
>> -// Internal structure for holding SEV-ES information needed during SEC phase
>> -// and valid only during SEC phase and early PEI during platform
>> -// initialization.
>> -//
>> -// This structure is also used by assembler files:
>> -// OvmfPkg/ResetVector/ResetVector.nasmb
>> -// OvmfPkg/ResetVector/Ia32/PageTables64.asm
>> -// OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
>> -// any changes must stay in sync with its usage.
>> -//
>> -typedef struct _SEC_SEV_ES_WORK_AREA {
>> - UINT8 SevEsEnabled;
>> - UINT8 Reserved1[7];
>> -
>> - UINT64 RandomData;
>> -
>> - UINT64 EncryptionMask;
>> -} SEC_SEV_ES_WORK_AREA;
>> -
>> //
>> // Memory encryption address range states.
>> //
>> diff --git a/OvmfPkg/Include/WorkArea.h b/OvmfPkg/Include/WorkArea.h
>> new file mode 100644
>> index 000000000000..0aaad7e1da67
>> --- /dev/null
>> +++ b/OvmfPkg/Include/WorkArea.h
>> @@ -0,0 +1,53 @@
>> +/** @file
>> +
>> + Work Area structure definition
>> +
>> + Copyright (c) 2021, AMD Inc.
>> +
>> + SPDX-License-Identifier: BSD-2-Clause-Patent
>> +**/
>> +
>> +#ifndef __OVMF_WORK_AREA_H__
>> +#define __OVMF_WORK_AREA_H__
>> +
>> +//
>> +// Internal structure for holding SEV-ES information needed during SEC phase
>> +// and valid only during SEC phase and early PEI during platform
>> +// initialization.
>> +//
>> +// This structure is also used by assembler files:
>> +// OvmfPkg/ResetVector/ResetVector.nasmb
>> +// OvmfPkg/ResetVector/Ia32/PageTables64.asm
>> +// OvmfPkg/ResetVector/Ia32/Flat32ToFlat64.asm
>> +// any changes must stay in sync with its usage.
>> +//
>> +typedef struct _SEC_SEV_ES_WORK_AREA {
>> + UINT8 SevEsEnabled;
>> + UINT8 Reserved1[7];
>> +
>> + UINT64 RandomData;
>> +
>> + UINT64 EncryptionMask;
>> +} SEC_SEV_ES_WORK_AREA;
>> +
>> +//
>> +// Guest type for the work area
>> +//
>> +typedef enum {
>> + GUEST_TYPE_NON_ENCRYPTED,
>> + GUEST_TYPE_AMD_SEV,
>> + GUEST_TYPE_INTEL_TDX,
>> +
>> +} GUEST_TYPE;
>> +
>> +//
>> +// The work area structure header definition.
>> +//
>> +typedef struct _OVMF_WORK_AREA {
>> + UINT8 GuestType;
>> + UINT8 Reserved1[3];
>> +
>> + SEC_SEV_ES_WORK_AREA SevEsWorkArea;
>> +} OVMF_WORK_AREA;
>> +
>> +#endif
>> diff --git a/OvmfPkg/PlatformPei/MemDetect.c b/OvmfPkg/PlatformPei/MemDetect.c
>> index 2deec128f464..4c53b0fdf2fe 100644
>> --- a/OvmfPkg/PlatformPei/MemDetect.c
>> +++ b/OvmfPkg/PlatformPei/MemDetect.c
>> @@ -939,23 +939,21 @@ InitializeRamRegions (
>> }
>>
>> #ifdef MDE_CPU_X64
>> - if (MemEncryptSevEsIsEnabled ()) {
>> - //
>> - // If SEV-ES is enabled, reserve the SEV-ES work area.
>> - //
>> - // Since this memory range will be used by the Reset Vector on S3
>> - // resume, it must be reserved as ACPI NVS.
>> - //
>> - // If S3 is unsupported, then various drivers might still write to the
>> - // work area. We ought to prevent DXE from serving allocation requests
>> - // such that they would overlap the work area.
>> - //
>> - BuildMemoryAllocationHob (
>> - (EFI_PHYSICAL_ADDRESS)(UINTN) FixedPcdGet32 (PcdSevEsWorkAreaBase),
>> - (UINT64)(UINTN) FixedPcdGet32 (PcdSevEsWorkAreaSize),
>> - mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>> - );
>> - }
>> + //
>> + // Reserve the work area.
>> + //
>> + // Since this memory range will be used by the Reset Vector on S3
>> + // resume, it must be reserved as ACPI NVS.
>> + //
>> + // If S3 is unsupported, then various drivers might still write to the
>> + // work area. We ought to prevent DXE from serving allocation requests
>> + // such that they would overlap the work area.
>> + //
>> + BuildMemoryAllocationHob (
>> + (EFI_PHYSICAL_ADDRESS)(UINTN) FixedPcdGet32 (PcdOvmfWorkAreaBase),
>> + (UINT64)(UINTN) FixedPcdGet32 (PcdOvmfWorkAreaSize),
>> + mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>> + );
>
> If SEV-ES is enabled, then we previously had already verified that the
> work area was present. Without that check now, it may not be. Just for
> safety, it is probably worth replacing the:
>
> if (MemEncryptSevEsIsEnabled ()) {
>
> with
>
> if (FixedPcdGet32 (PcdOvmfWorkAreaSize) != 0) {
>
Noted.
thanks
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78981): https://edk2.groups.io/g/devel/message/78981
Mute This Topic: https://groups.io/mt/84670984/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