* [PATCH v2 1/3] OvmfPkg: introduce a common work area
2021-08-05 20:42 [PATCH v2 0/3] reuse the SevEsWork area Brijesh Singh
@ 2021-08-05 20:42 ` Brijesh Singh
2021-08-05 20:42 ` [PATCH v2 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format Brijesh Singh
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Brijesh Singh @ 2021-08-05 20:42 UTC (permalink / raw)
To: devel
Cc: James Bottomley, Min Xu, Jiewen Yao, Tom Lendacky, Jordan Justen,
Ard Biesheuvel, Erdem Aktas, Michael Roth, 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format
2021-08-05 20:42 [PATCH v2 0/3] reuse the SevEsWork area Brijesh Singh
2021-08-05 20:42 ` [PATCH v2 1/3] OvmfPkg: introduce a common work area Brijesh Singh
@ 2021-08-05 20:42 ` Brijesh Singh
2021-08-05 20:42 ` [PATCH v2 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm Brijesh Singh
2021-08-15 3:00 ` [PATCH v2 0/3] reuse the SevEsWork area Min Xu
3 siblings, 0 replies; 6+ messages in thread
From: Brijesh Singh @ 2021-08-05 20:42 UTC (permalink / raw)
To: devel
Cc: James Bottomley, Min Xu, Jiewen Yao, Tom Lendacky, Jordan Justen,
Ard Biesheuvel, Erdem Aktas, Michael Roth, Brijesh Singh
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
Update the SEV support to switch to using the newer work area format.
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/ResetVector/ResetVector.inf | 1 +
OvmfPkg/Sec/SecMain.inf | 2 ++
OvmfPkg/Sec/SecMain.c | 32 ++++++++++++++++++++++-
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 ++++++
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 4 +++
OvmfPkg/ResetVector/ResetVector.nasmb | 1 +
6 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
index d028c92d8cfa..6ec9cca40c3a 100644
--- a/OvmfPkg/ResetVector/ResetVector.inf
+++ b/OvmfPkg/ResetVector/ResetVector.inf
@@ -34,6 +34,7 @@ [BuildOptions]
*_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/
[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize
diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 7f78dcee2772..b650345770f2 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -56,6 +56,7 @@ [Ppis]
gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED
[Pcd]
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
@@ -70,6 +71,7 @@ [Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack
+ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader
[FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 9db67e17b2aa..27a1a4af0e4a 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -807,6 +807,36 @@ SevEsProtocolCheck (
Ghcb->GhcbUsage = GHCB_STANDARD_USAGE;
}
+/**
+ Determine if the SEV is active.
+
+ During the early booting, GuestType is set in the work area. Verify that it
+ is an SEV guest.
+
+ @retval TRUE SEV is enabled
+ @retval FALSE SEV is not enabled
+
+**/
+STATIC
+BOOLEAN
+IsSevGuest (
+ VOID
+ )
+{
+ OVMF_WORK_AREA *WorkArea;
+
+ //
+ // Ensure that the size of the Confidential Computing work area header
+ // is same as what is provided through a fixed PCD.
+ //
+ ASSERT ((UINTN) FixedPcdGet32 (PcdOvmfConfidentialComputingWorkAreaHeader) ==
+ sizeof(CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER));
+
+ WorkArea = (OVMF_WORK_AREA *) FixedPcdGet32 (PcdOvmfWorkAreaBase);
+
+ return ((WorkArea != NULL) && (WorkArea->Header.GuestType == GUEST_TYPE_AMD_SEV));
+}
+
/**
Determine if SEV-ES is active.
@@ -828,7 +858,7 @@ SevEsIsEnabled (
SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
- return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
+ return (((IsSevGuest()) && SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
}
VOID
diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index aa95d06eaddb..87d81b01e263 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -171,6 +171,9 @@ CheckSevFeatures:
bt eax, 0
jnc NoSev
+ ; Set the work area header to indicate that the SEV is enabled
+ mov byte[WORK_AREA_GUEST_TYPE], 1
+
; Check for SEV-ES memory encryption feature:
; CPUID Fn8000_001F[EAX] - Bit 3
; CPUID raises a #VC exception if running as an SEV-ES guest
@@ -257,6 +260,11 @@ SevExit:
IsSevEsEnabled:
xor eax, eax
+ ; During CheckSevFeatures, the WORK_AREA_GUEST_TYPE is set
+ ; to 1 if SEV is enabled.
+ cmp byte[WORK_AREA_GUEST_TYPE], 1
+ jne SevEsDisabled
+
; During CheckSevFeatures, the SEV_ES_WORK_AREA was set to 1 if
; SEV-ES is enabled.
cmp byte[SEV_ES_WORK_AREA], 1
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index eacdb69ddb9f..f688909f1c7d 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -42,6 +42,10 @@ BITS 32
;
SetCr3ForPageTables64:
+ ; Clear the WorkArea header. The SEV probe routines will populate the
+ ; work area when detected.
+ mov byte[WORK_AREA_GUEST_TYPE], 0
+
OneTimeCall CheckSevFeatures
xor edx, edx
test eax, eax
diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb
index acec46a32450..d1d800c56745 100644
--- a/OvmfPkg/ResetVector/ResetVector.nasmb
+++ b/OvmfPkg/ResetVector/ResetVector.nasmb
@@ -72,6 +72,7 @@
%define GHCB_PT_ADDR (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase))
%define GHCB_BASE (FixedPcdGet32 (PcdOvmfSecGhcbBase))
%define GHCB_SIZE (FixedPcdGet32 (PcdOvmfSecGhcbSize))
+ %define WORK_AREA_GUEST_TYPE (FixedPcdGet32 (PcdOvmfWorkAreaBase))
%define SEV_ES_WORK_AREA (FixedPcdGet32 (PcdSevEsWorkAreaBase))
%define SEV_ES_WORK_AREA_RDRAND (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 8)
%define SEV_ES_WORK_AREA_ENC_MASK (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 16)
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm
2021-08-05 20:42 [PATCH v2 0/3] reuse the SevEsWork area Brijesh Singh
2021-08-05 20:42 ` [PATCH v2 1/3] OvmfPkg: introduce a common work area Brijesh Singh
2021-08-05 20:42 ` [PATCH v2 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format Brijesh Singh
@ 2021-08-05 20:42 ` Brijesh Singh
2021-08-15 3:00 ` [PATCH v2 0/3] reuse the SevEsWork area Min Xu
3 siblings, 0 replies; 6+ messages in thread
From: Brijesh Singh @ 2021-08-05 20:42 UTC (permalink / raw)
To: devel
Cc: James Bottomley, Min Xu, Jiewen Yao, Tom Lendacky, Jordan Justen,
Ard Biesheuvel, Erdem Aktas, Michael Roth, Brijesh Singh
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
While build the initial page table, the SetCr3ForPageTables64 checks
whether SEV-ES is enabled. If so, clear the page encryption mask from the
GHCB page. Move the logic to clear the page encryption mask in the
AmdSev.asm.
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/ResetVector/Ia32/AmdSev.asm | 113 +++++++++++++++++-----
OvmfPkg/ResetVector/Ia32/PageTables64.asm | 53 ++--------
2 files changed, 94 insertions(+), 72 deletions(-)
diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 87d81b01e263..fd2e6abcd4a0 100644
--- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
+++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
@@ -44,6 +44,27 @@ BITS 32
; The unexpected response code
%define TERM_UNEXPECTED_RESP_CODE 2
+%define PAGE_PRESENT 0x01
+%define PAGE_READ_WRITE 0x02
+%define PAGE_USER_SUPERVISOR 0x04
+%define PAGE_WRITE_THROUGH 0x08
+%define PAGE_CACHE_DISABLE 0x010
+%define PAGE_ACCESSED 0x020
+%define PAGE_DIRTY 0x040
+%define PAGE_PAT 0x080
+%define PAGE_GLOBAL 0x0100
+%define PAGE_2M_MBO 0x080
+%define PAGE_2M_PAT 0x01000
+
+%define PAGE_4K_PDE_ATTR (PAGE_ACCESSED + \
+ PAGE_DIRTY + \
+ PAGE_READ_WRITE + \
+ PAGE_PRESENT)
+
+%define PAGE_PDP_ATTR (PAGE_ACCESSED + \
+ PAGE_READ_WRITE + \
+ PAGE_PRESENT)
+
; Macro is used to issue the MSR protocol based VMGEXIT. The caller is
; responsible to populate values in the EDX:EAX registers. After the vmmcall
@@ -117,6 +138,72 @@ BITS 32
SevEsUnexpectedRespTerminate:
TerminateVmgExit TERM_UNEXPECTED_RESP_CODE
+; If SEV-ES is enabled then initialize the make the GHCB page shared
+SevClearPageEncMaskFromGHCBPage:
+ ; Check if SEV is enabled
+ cmp byte[WORK_AREA_GUEST_TYPE], 1
+ jnz SevClearPageEncMaskFromGHCBPageExit
+
+ ; Check if SEV-ES is enabled
+ cmp byte[SEV_ES_WORK_AREA], 1
+ jnz SevClearPageEncMaskFromGHCBPageExit
+
+ ;
+ ; The initial GHCB will live at GHCB_BASE and needs to be un-encrypted.
+ ; This requires the 2MB page for this range be broken down into 512 4KB
+ ; pages. All will be marked encrypted, except for the GHCB.
+ ;
+ mov ecx, (GHCB_BASE >> 21)
+ mov eax, GHCB_PT_ADDR + PAGE_PDP_ATTR
+ mov [ecx * 8 + PT_ADDR (0x2000)], eax
+
+ ;
+ ; Page Table Entries (512 * 4KB entries => 2MB)
+ ;
+ mov ecx, 512
+pageTableEntries4kLoop:
+ mov eax, ecx
+ dec eax
+ shl eax, 12
+ add eax, GHCB_BASE & 0xFFE0_0000
+ add eax, PAGE_4K_PDE_ATTR
+ mov [ecx * 8 + GHCB_PT_ADDR - 8], eax
+ mov [(ecx * 8 + GHCB_PT_ADDR - 8) + 4], edx
+ loop pageTableEntries4kLoop
+
+ ;
+ ; Clear the encryption bit from the GHCB entry
+ ;
+ mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
+ mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0
+
+ mov ecx, GHCB_SIZE / 4
+ xor eax, eax
+clearGhcbMemoryLoop:
+ mov dword[ecx * 4 + GHCB_BASE - 4], eax
+ loop clearGhcbMemoryLoop
+
+SevClearPageEncMaskFromGHCBPageExit:
+ OneTimeCallRet SevClearPageEncMaskFromGHCBPage
+
+; Check if SEV is enabled, and get the C-bit mask above 31.
+; Modified: EDX
+;
+; The value is returned in the EDX
+GetSevCBitMaskAbove31:
+ ; Check if SEV is enabled
+ cmp byte[WORK_AREA_GUEST_TYPE], 1
+ jnz NoCbitValue
+
+ mov edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
+ jmp GetSevCBitMaskAbove31Exit
+
+NoCbitValue:
+ xor edx, edx
+
+GetSevCBitMaskAbove31Exit:
+ OneTimeCallRet GetSevCBitMaskAbove31
+
; Check if Secure Encrypted Virtualization (SEV) features are enabled.
;
; Register usage is tight in this routine, so multiple calls for the
@@ -249,32 +336,6 @@ SevExit:
OneTimeCallRet CheckSevFeatures
-; Check if Secure Encrypted Virtualization - Encrypted State (SEV-ES) feature
-; is enabled.
-;
-; Modified: EAX
-;
-; If SEV-ES is enabled then EAX will be non-zero.
-; If SEV-ES is disabled then EAX will be zero.
-;
-IsSevEsEnabled:
- xor eax, eax
-
- ; During CheckSevFeatures, the WORK_AREA_GUEST_TYPE is set
- ; to 1 if SEV is enabled.
- cmp byte[WORK_AREA_GUEST_TYPE], 1
- jne SevEsDisabled
-
- ; During CheckSevFeatures, the SEV_ES_WORK_AREA was set to 1 if
- ; SEV-ES is enabled.
- cmp byte[SEV_ES_WORK_AREA], 1
- jne SevEsDisabled
-
- mov eax, 1
-
-SevEsDisabled:
- OneTimeCallRet IsSevEsEnabled
-
; Start of #VC exception handling routines
;
diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
index f688909f1c7d..0e8ba4dde534 100644
--- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm
+++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm
@@ -46,16 +46,13 @@ SetCr3ForPageTables64:
; work area when detected.
mov byte[WORK_AREA_GUEST_TYPE], 0
+ ; Check whether the SEV is active and populate the SevEsWorkArea
OneTimeCall CheckSevFeatures
- xor edx, edx
- test eax, eax
- jz SevNotActive
- ; If SEV is enabled, C-bit is always above 31
- sub eax, 32
- bts edx, eax
-
-SevNotActive:
+ ; If SEV is enabled, the C-bit position is always above 31.
+ ; The mask will be saved in the EDX and applied during the
+ ; the page table build below.
+ OneTimeCall GetSevCBitMaskAbove31
;
; For OVMF, build some initial page tables at
@@ -105,44 +102,8 @@ pageTableEntriesLoop:
mov [(ecx * 8 + PT_ADDR (0x2000 - 8)) + 4], edx
loop pageTableEntriesLoop
- OneTimeCall IsSevEsEnabled
- test eax, eax
- jz SetCr3
-
- ;
- ; The initial GHCB will live at GHCB_BASE and needs to be un-encrypted.
- ; This requires the 2MB page for this range be broken down into 512 4KB
- ; pages. All will be marked encrypted, except for the GHCB.
- ;
- mov ecx, (GHCB_BASE >> 21)
- mov eax, GHCB_PT_ADDR + PAGE_PDP_ATTR
- mov [ecx * 8 + PT_ADDR (0x2000)], eax
-
- ;
- ; Page Table Entries (512 * 4KB entries => 2MB)
- ;
- mov ecx, 512
-pageTableEntries4kLoop:
- mov eax, ecx
- dec eax
- shl eax, 12
- add eax, GHCB_BASE & 0xFFE0_0000
- add eax, PAGE_4K_PDE_ATTR
- mov [ecx * 8 + GHCB_PT_ADDR - 8], eax
- mov [(ecx * 8 + GHCB_PT_ADDR - 8) + 4], edx
- loop pageTableEntries4kLoop
-
- ;
- ; Clear the encryption bit from the GHCB entry
- ;
- mov ecx, (GHCB_BASE & 0x1F_FFFF) >> 12
- mov [ecx * 8 + GHCB_PT_ADDR + 4], strict dword 0
-
- mov ecx, GHCB_SIZE / 4
- xor eax, eax
-clearGhcbMemoryLoop:
- mov dword[ecx * 4 + GHCB_BASE - 4], eax
- loop clearGhcbMemoryLoop
+ ; Clear the C-bit from the GHCB page if the SEV-ES is enabled.
+ OneTimeCall SevClearPageEncMaskFromGHCBPage
SetCr3:
;
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/3] reuse the SevEsWork area
2021-08-05 20:42 [PATCH v2 0/3] reuse the SevEsWork area Brijesh Singh
` (2 preceding siblings ...)
2021-08-05 20:42 ` [PATCH v2 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm Brijesh Singh
@ 2021-08-15 3:00 ` Min Xu
2021-08-16 13:47 ` Brijesh Singh
3 siblings, 1 reply; 6+ messages in thread
From: Min Xu @ 2021-08-15 3:00 UTC (permalink / raw)
To: Brijesh Singh, devel@edk2.groups.io
Cc: James Bottomley, Yao, Jiewen, Tom Lendacky, Justen, Jordan L,
Ard Biesheuvel, Erdem Aktas, Michael Roth
Hi, Brijesh
May I know is there some progress of this patch set? I noticed some comments have been given to the patch-set.
Thanks!
Min
> -----Original Message-----
> From: Brijesh Singh <brijesh.singh@amd.com>
> Sent: Friday, August 6, 2021 4:42 AM
> To: devel@edk2.groups.io
> Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M
> <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
> Erdem Aktas <erdemaktas@google.com>; Michael Roth
> <Michael.Roth@amd.com>; Brijesh Singh <brijesh.singh@amd.com>
> Subject: [PATCH v2 0/3] reuse the SevEsWork area
>
> Based on the discussion on the mailing list, we agreed that instead of wasting
> extra page in the MEMFD, we can reuse the SevEsWorkArea buffer for the
> TDX. To avoid any confusion, lets introduce a OvmfWorkArea that will
> contains 32 bytes of header followed by the actual workarea.
>
> While at it, move the code to clear the GHCB page from PageTable build to
> AmdSev.asm.
>
> I have used the existing TDX BZ for it because the request came during the
> TDX patch review. if anyone have concern please let me know and I will
> happily create a new BZ.
>
> Full tree is at: https://github.com/AMDESE/ovmf/tree/sev-new-work-area
>
> Brijesh Singh (3):
> OvmfPkg: introduce a common work area
> OvmfPkg/ResetVector: update SEV support to use new work area format
> OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm
>
> 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>
>
> Changes since v1:
> - address Jiewen's feedback.
>
> Brijesh Singh (3):
> OvmfPkg: introduce a common work area
> OvmfPkg/ResetVector: update SEV support to use new work area format
> OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm
>
> OvmfPkg/OvmfPkg.dec | 12 +++
> OvmfPkg/OvmfPkgX64.fdf | 9 +-
> OvmfPkg/PlatformPei/PlatformPei.inf | 4 +-
> OvmfPkg/ResetVector/ResetVector.inf | 1 +
> OvmfPkg/Sec/SecMain.inf | 2 +
> OvmfPkg/Include/Library/MemEncryptSevLib.h | 21 +---
> OvmfPkg/Include/WorkArea.h | 67 +++++++++++++
> OvmfPkg/PlatformPei/MemDetect.c | 32 +++---
> OvmfPkg/Sec/SecMain.c | 32 +++++-
> OvmfPkg/OvmfPkgDefines.fdf.inc | 6 ++
> OvmfPkg/ResetVector/Ia32/AmdSev.asm | 111 +++++++++++++++++---
> -
> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 57 ++---------
> OvmfPkg/ResetVector/ResetVector.nasmb | 1 +
> 13 files changed, 247 insertions(+), 108 deletions(-) create mode 100644
> OvmfPkg/Include/WorkArea.h
>
> --
> 2.17.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 0/3] reuse the SevEsWork area
2021-08-15 3:00 ` [PATCH v2 0/3] reuse the SevEsWork area Min Xu
@ 2021-08-16 13:47 ` Brijesh Singh
0 siblings, 0 replies; 6+ messages in thread
From: Brijesh Singh @ 2021-08-16 13:47 UTC (permalink / raw)
To: Xu, Min M, devel@edk2.groups.io
Cc: brijesh.singh, James Bottomley, Yao, Jiewen, Tom Lendacky,
Justen, Jordan L, Ard Biesheuvel, Erdem Aktas, Michael Roth
Hi Min,
I was giving sometime for other reviewers to comment. I will respin v3
this week with Tom's feedback addressed.
thanks
On 8/14/21 10:00 PM, Xu, Min M wrote:
> Hi, Brijesh
> May I know is there some progress of this patch set? I noticed some comments have been given to the patch-set.
>
> Thanks!
> Min
>
>> -----Original Message-----
>> From: Brijesh Singh <brijesh.singh@amd.com>
>> Sent: Friday, August 6, 2021 4:42 AM
>> To: devel@edk2.groups.io
>> Cc: James Bottomley <jejb@linux.ibm.com>; Xu, Min M
>> <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom
>> Lendacky <thomas.lendacky@amd.com>; Justen, Jordan L
>> <jordan.l.justen@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>;
>> Erdem Aktas <erdemaktas@google.com>; Michael Roth
>> <Michael.Roth@amd.com>; Brijesh Singh <brijesh.singh@amd.com>
>> Subject: [PATCH v2 0/3] reuse the SevEsWork area
>>
>> Based on the discussion on the mailing list, we agreed that instead of wasting
>> extra page in the MEMFD, we can reuse the SevEsWorkArea buffer for the
>> TDX. To avoid any confusion, lets introduce a OvmfWorkArea that will
>> contains 32 bytes of header followed by the actual workarea.
>>
>> While at it, move the code to clear the GHCB page from PageTable build to
>> AmdSev.asm.
>>
>> I have used the existing TDX BZ for it because the request came during the
>> TDX patch review. if anyone have concern please let me know and I will
>> happily create a new BZ.
>>
>> Full tree is at: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAMDESE%2Fovmf%2Ftree%2Fsev-new-work-area&data=04%7C01%7Cbrijesh.singh%40amd.com%7C0e9d9486a8974e77448a08d95f98d95e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637645932352135436%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=2kSK48j5Nghl5F%2FCDyun6tGkFRwTpb1QAf5kpPAnbIA%3D&reserved=0
>>
>> Brijesh Singh (3):
>> OvmfPkg: introduce a common work area
>> OvmfPkg/ResetVector: update SEV support to use new work area format
>> OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm
>>
>> 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>
>>
>> Changes since v1:
>> - address Jiewen's feedback.
>>
>> Brijesh Singh (3):
>> OvmfPkg: introduce a common work area
>> OvmfPkg/ResetVector: update SEV support to use new work area format
>> OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm
>>
>> OvmfPkg/OvmfPkg.dec | 12 +++
>> OvmfPkg/OvmfPkgX64.fdf | 9 +-
>> OvmfPkg/PlatformPei/PlatformPei.inf | 4 +-
>> OvmfPkg/ResetVector/ResetVector.inf | 1 +
>> OvmfPkg/Sec/SecMain.inf | 2 +
>> OvmfPkg/Include/Library/MemEncryptSevLib.h | 21 +---
>> OvmfPkg/Include/WorkArea.h | 67 +++++++++++++
>> OvmfPkg/PlatformPei/MemDetect.c | 32 +++---
>> OvmfPkg/Sec/SecMain.c | 32 +++++-
>> OvmfPkg/OvmfPkgDefines.fdf.inc | 6 ++
>> OvmfPkg/ResetVector/Ia32/AmdSev.asm | 111 +++++++++++++++++---
>> -
>> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 57 ++---------
>> OvmfPkg/ResetVector/ResetVector.nasmb | 1 +
>> 13 files changed, 247 insertions(+), 108 deletions(-) create mode 100644
>> OvmfPkg/Include/WorkArea.h
>>
>> --
>> 2.17.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread