[PATCH v2 1/3] OvmfPkg: introduce a common work area


Brijesh Singh
 

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@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@google.com>
Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
---
OvmfPkg/OvmfPkg.dec | 12 ++++
OvmfPkg/OvmfPkgX64.fdf | 9 ++-
OvmfPkg/PlatformPei/PlatformPei.inf | 4 +-
OvmfPkg/Include/Library/MemEncryptSevLib.h | 21 +------
OvmfPkg/Include/WorkArea.h | 67 ++++++++++++++++++++++
OvmfPkg/PlatformPei/MemDetect.c | 32 +++++------
OvmfPkg/OvmfPkgDefines.fdf.inc | 6 ++
7 files changed, 111 insertions(+), 40 deletions(-)
create mode 100644 OvmfPkg/Include/WorkArea.h

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 2ab27f0c73c2..550a58ebcd81 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -330,6 +330,18 @@ [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
+
+ ## The work area contains a fixed size header in the Include/WorkArea.h.
+ # The size of this header is used early boot, and is provided through
+ # a fixed PCD. It need to be kept in sync with any changes to the
+ # header definition.
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader|0|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..23936242e74a 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

+##########################################################################################
+# Set the SEV-ES specific work area PCDs
+#
+SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase = $(MEMFD_BASE_ADDRESS) + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader
+SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize - gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader
+##########################################################################################
+
################################################################################

[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..c16030e3ac0a
--- /dev/null
+++ b/OvmfPkg/Include/WorkArea.h
@@ -0,0 +1,67 @@
+/** @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__
+
+//
+// Guest type for the work area
+//
+typedef enum {
+ GUEST_TYPE_NON_ENCRYPTED,
+ GUEST_TYPE_AMD_SEV,
+ GUEST_TYPE_INTEL_TDX,
+
+} GUEST_TYPE;
+
+//
+// Confidential computing work area header definition. Any change
+// to the structure need to be kept in sync with the
+// PcdOvmfConfidentialComputingWorkAreaHeader.
+//
+typedef struct _CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER {
+ UINT8 GuestType;
+ UINT8 Reserved1[3];
+} CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;
+
+//
+// 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;
+
+//
+// The SEV work area definition.
+//
+typedef struct _SEV_WORK_AREA {
+ CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER Header;
+
+ SEC_SEV_ES_WORK_AREA SevEsWorkArea;
+} SEV_WORK_AREA;
+
+typedef union {
+ CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER Header;
+ SEV_WORK_AREA SevWorkArea;
+} 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
+ );
#endif
}
}
diff --git a/OvmfPkg/OvmfPkgDefines.fdf.inc b/OvmfPkg/OvmfPkgDefines.fdf.inc
index 35fd454b97ab..3b5e45253916 100644
--- a/OvmfPkg/OvmfPkgDefines.fdf.inc
+++ b/OvmfPkg/OvmfPkgDefines.fdf.inc
@@ -82,6 +82,12 @@
SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwSpareBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize = $(VARS_SPARE_SIZE)

+# The OVMF WorkArea contains a fixed size header followed by the actual data.
+# The size of header is accessed through a fixed PCD in the reset vector code.
+# The value need to be kept in sync with the any changes to the Confidential
+# Computing Work Area header defined in the Include/WorkArea.h
+SET gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader = 4
+
!if $(SMM_REQUIRE) == TRUE
SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase = gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase
--
2.17.1

Join devel@edk2.groups.io to automatically receive all group messages.