public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/3] reuse the SevEsWork area
@ 2021-08-17 13:46 Brijesh Singh
  2021-08-17 13:46 ` [PATCH v3 1/3] OvmfPkg: introduce a common work area Brijesh Singh
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Brijesh Singh @ 2021-08-17 13:46 UTC (permalink / raw)
  To: devel
  Cc: James Bottomley, Min Xu, Jiewen Yao, Tom Lendacky, Jordan Justen,
	Ard Biesheuvel, Erdem Aktas, Michael Roth, Brijesh Singh

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 v2:
 - address Tom's feedback

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            |   8 +-
 OvmfPkg/Sec/SecMain.c                      |  36 ++++++-
 OvmfPkg/OvmfPkgDefines.fdf.inc             |   6 ++
 OvmfPkg/ResetVector/Ia32/AmdSev.asm        | 109 +++++++++++++++++----
 OvmfPkg/ResetVector/Ia32/PageTables64.asm  |  57 +++--------
 OvmfPkg/ResetVector/ResetVector.nasmb      |   1 +
 13 files changed, 238 insertions(+), 95 deletions(-)
 create mode 100644 OvmfPkg/Include/WorkArea.h

-- 
2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3 1/3] OvmfPkg: introduce a common work area
  2021-08-17 13:46 [PATCH v3 0/3] reuse the SevEsWork area Brijesh Singh
@ 2021-08-17 13:46 ` Brijesh Singh
  2021-08-19 14:14   ` [edk2-devel] " Min Xu
  2021-08-17 13:46 ` [PATCH v3 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format Brijesh Singh
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Brijesh Singh @ 2021-08-17 13:46 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            |  8 +--
 OvmfPkg/OvmfPkgDefines.fdf.inc             |  6 ++
 7 files changed, 100 insertions(+), 27 deletions(-)
 create mode 100644 OvmfPkg/Include/WorkArea.h

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 8fb6f257e8e8..c37dafad49bb 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -329,6 +329,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..2c2c4641ec8a 100644
--- a/OvmfPkg/PlatformPei/MemDetect.c
+++ b/OvmfPkg/PlatformPei/MemDetect.c
@@ -939,9 +939,9 @@ InitializeRamRegions (
     }
 
 #ifdef MDE_CPU_X64
-    if (MemEncryptSevEsIsEnabled ()) {
+    if (FixedPcdGet32 (PcdOvmfWorkAreaSize) != 0) {
       //
-      // If SEV-ES is enabled, reserve the SEV-ES work area.
+      // 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.
@@ -951,8 +951,8 @@ InitializeRamRegions (
       // such that they would overlap the work area.
       //
       BuildMemoryAllocationHob (
-        (EFI_PHYSICAL_ADDRESS)(UINTN) FixedPcdGet32 (PcdSevEsWorkAreaBase),
-        (UINT64)(UINTN) FixedPcdGet32 (PcdSevEsWorkAreaSize),
+        (EFI_PHYSICAL_ADDRESS)(UINTN) FixedPcdGet32 (PcdOvmfWorkAreaBase),
+        (UINT64)(UINTN) FixedPcdGet32 (PcdOvmfWorkAreaSize),
         mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
         );
     }
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] 9+ messages in thread

* [PATCH v3 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format
  2021-08-17 13:46 [PATCH v3 0/3] reuse the SevEsWork area Brijesh Singh
  2021-08-17 13:46 ` [PATCH v3 1/3] OvmfPkg: introduce a common work area Brijesh Singh
@ 2021-08-17 13:46 ` Brijesh Singh
  2021-08-19 14:15   ` Min Xu
  2021-08-17 13:46 ` [PATCH v3 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm Brijesh Singh
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Brijesh Singh @ 2021-08-17 13:46 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                     | 36 ++++++++++++++++++++++-
 OvmfPkg/ResetVector/Ia32/AmdSev.asm       |  8 +++++
 OvmfPkg/ResetVector/Ia32/PageTables64.asm |  4 +++
 OvmfPkg/ResetVector/ResetVector.nasmb     |  1 +
 6 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf
index d028c92d8cfa..a2520dde5508 100644
--- a/OvmfPkg/ResetVector/ResetVector.inf
+++ b/OvmfPkg/ResetVector/ResetVector.inf
@@ -43,6 +43,7 @@ [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
 
 [FixedPcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase
diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 7f78dcee2772..ea4b9611f52d 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -70,6 +70,8 @@ [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
   gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHeader
+  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
 
 [FeaturePcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index 9db67e17b2aa..707b0d4bbff4 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.
 
@@ -826,9 +856,13 @@ SevEsIsEnabled (
 {
   SEC_SEV_ES_WORK_AREA  *SevEsWorkArea;
 
+  if (!IsSevGuest()) {
+    return FALSE;
+  }
+
   SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase);
 
-  return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
+  return (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] 9+ messages in thread

* [PATCH v3 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm
  2021-08-17 13:46 [PATCH v3 0/3] reuse the SevEsWork area Brijesh Singh
  2021-08-17 13:46 ` [PATCH v3 1/3] OvmfPkg: introduce a common work area Brijesh Singh
  2021-08-17 13:46 ` [PATCH v3 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format Brijesh Singh
@ 2021-08-17 13:46 ` Brijesh Singh
  2021-08-19 14:15   ` Min Xu
  2021-08-25  6:31 ` [PATCH v3 0/3] reuse the SevEsWork area Yao, Jiewen
  2021-08-27 13:14 ` Yao, Jiewen
  4 siblings, 1 reply; 9+ messages in thread
From: Brijesh Singh @ 2021-08-17 13:46 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       | 111 +++++++++++++++++-----
 OvmfPkg/ResetVector/Ia32/PageTables64.asm |  53 ++---------
 2 files changed, 92 insertions(+), 72 deletions(-)

diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
index 87d81b01e263..250ac8d8b180 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,70 @@ BITS    32
 SevEsUnexpectedRespTerminate:
     TerminateVmgExit    TERM_UNEXPECTED_RESP_CODE
 
+; If SEV-ES is enabled then initialize and make the GHCB page shared
+SevClearPageEncMaskForGhcbPage:
+    ; Check if SEV is enabled
+    cmp       byte[WORK_AREA_GUEST_TYPE], 1
+    jnz       SevClearPageEncMaskForGhcbPageExit
+
+    ; Check if SEV-ES is enabled
+    cmp       byte[SEV_ES_WORK_AREA], 1
+    jnz       SevClearPageEncMaskForGhcbPageExit
+
+    ;
+    ; 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
+
+SevClearPageEncMaskForGhcbPageExit:
+    OneTimeCallRet SevClearPageEncMaskForGhcbPage
+
+; Check if SEV is enabled, and get the C-bit mask above 31.
+; Modified: EDX
+;
+; The value is returned in the EDX
+GetSevCBitMaskAbove31:
+    xor       edx, edx
+
+    ; Check if SEV is enabled
+    cmp       byte[WORK_AREA_GUEST_TYPE], 1
+    jnz       GetSevCBitMaskAbove31Exit
+
+    mov       edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
+
+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 +334,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..07b6ca070909 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   SevClearPageEncMaskForGhcbPage
 
 SetCr3:
     ;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [edk2-devel] [PATCH v3 1/3] OvmfPkg: introduce a common work area
  2021-08-17 13:46 ` [PATCH v3 1/3] OvmfPkg: introduce a common work area Brijesh Singh
@ 2021-08-19 14:14   ` Min Xu
  0 siblings, 0 replies; 9+ messages in thread
From: Min Xu @ 2021-08-19 14:14 UTC (permalink / raw)
  To: devel@edk2.groups.io, brijesh.singh@amd.com
  Cc: James Bottomley, Yao, Jiewen, Tom Lendacky, Justen, Jordan L,
	Ard Biesheuvel, Erdem Aktas, Michael Roth

Reviewed-by: Min Xu <min.m.xu@intel.com>

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
> Singh via groups.io
> Sent: Tuesday, August 17, 2021 9:47 PM
> 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: [edk2-devel] [PATCH v3 1/3] OvmfPkg: introduce a common work area
> 
> 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            |  8 +--
>  OvmfPkg/OvmfPkgDefines.fdf.inc             |  6 ++
>  7 files changed, 100 insertions(+), 27 deletions(-)  create mode 100644
> OvmfPkg/Include/WorkArea.h
> 
> diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec index
> 8fb6f257e8e8..c37dafad49bb 100644
> --- a/OvmfPkg/OvmfPkg.dec
> +++ b/OvmfPkg/OvmfPkg.dec
> @@ -329,6 +329,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.PcdOvmfConfidentialComputingWorkAreaHead
> er|
> + 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|gUefiOvmfPkgTokenSpac
> eGuid.PcdOvmfSecGhcbSize
> 
>  0x00B000|0x001000
> -
> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpac
> eGuid.PcdSevEsWorkAreaSize
> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase|gUefiOvmfPkgTokenS
> paceGu
> +id.PcdOvmfWorkAreaSize
> 
>  0x00C000|0x001000
> 
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBackupBase|gUefiOvmfPkgTok
> enSpaceGuid.PcdOvmfSecGhcbBackupSize
> @@ -99,6 +99,13 @@ [FD.MEMFD]
> 
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenS
> paceGuid.PcdOvmfDxeMemFvSize
>  FV = DXEFV
> 
> +################################################################
> #######
> +################### # Set the SEV-ES specific work area PCDs # SET
> +gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase =
> $(MEMFD_BASE_ADDRESS)
> ++  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase +
> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHea
> der
> +SET gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaSize =
> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaSize -
> +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHea
> der
> +################################################################
> #######
> +###################
> +
> 
> #################################################################
> ###############
> 
>  [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..2c2c4641ec8a
> 100644
> --- a/OvmfPkg/PlatformPei/MemDetect.c
> +++ b/OvmfPkg/PlatformPei/MemDetect.c
> @@ -939,9 +939,9 @@ InitializeRamRegions (
>      }
> 
>  #ifdef MDE_CPU_X64
> -    if (MemEncryptSevEsIsEnabled ()) {
> +    if (FixedPcdGet32 (PcdOvmfWorkAreaSize) != 0) {
>        //
> -      // If SEV-ES is enabled, reserve the SEV-ES work area.
> +      // 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.
> @@ -951,8 +951,8 @@ InitializeRamRegions (
>        // such that they would overlap the work area.
>        //
>        BuildMemoryAllocationHob (
> -        (EFI_PHYSICAL_ADDRESS)(UINTN) FixedPcdGet32
> (PcdSevEsWorkAreaBase),
> -        (UINT64)(UINTN) FixedPcdGet32 (PcdSevEsWorkAreaSize),
> +        (EFI_PHYSICAL_ADDRESS)(UINTN) FixedPcdGet32
> (PcdOvmfWorkAreaBase),
> +        (UINT64)(UINTN) FixedPcdGet32 (PcdOvmfWorkAreaSize),
>          mS3Supported ? EfiACPIMemoryNVS : EfiBootServicesData
>          );
>      }
> 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.PcdOvmfConfidentialComputingWorkAreaHea
> der
> += 4
> +
>  !if $(SMM_REQUIRE) == TRUE
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64 =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageVariableBase
>  SET gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase =
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashNvStorageFtwWorkingBase
> --
> 2.17.1
> 
> 
> 
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format
  2021-08-17 13:46 ` [PATCH v3 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format Brijesh Singh
@ 2021-08-19 14:15   ` Min Xu
  0 siblings, 0 replies; 9+ messages in thread
From: Min Xu @ 2021-08-19 14:15 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

Reviewed-by: Min Xu <min.m.xu@intel.com>

> -----Original Message-----
> From: Brijesh Singh <brijesh.singh@amd.com>
> Sent: Tuesday, August 17, 2021 9:47 PM
> 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 v3 2/3] OvmfPkg/ResetVector: update SEV support to use new
> work area format
> 
> 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                     | 36 ++++++++++++++++++++++-
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm       |  8 +++++
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm |  4 +++
>  OvmfPkg/ResetVector/ResetVector.nasmb     |  1 +
>  6 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/ResetVector/ResetVector.inf
> b/OvmfPkg/ResetVector/ResetVector.inf
> index d028c92d8cfa..a2520dde5508 100644
> --- a/OvmfPkg/ResetVector/ResetVector.inf
> +++ b/OvmfPkg/ResetVector/ResetVector.inf
> @@ -43,6 +43,7 @@ [Pcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
> 
>  [FixedPcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdSevLaunchSecretBase
> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf index
> 7f78dcee2772..ea4b9611f52d 100644
> --- a/OvmfPkg/Sec/SecMain.inf
> +++ b/OvmfPkg/Sec/SecMain.inf
> @@ -70,6 +70,8 @@ [Pcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize
>    gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd
>    gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack
> +
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfConfidentialComputingWorkAreaHead
> er
> +  gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
> 
>  [FeaturePcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c index
> 9db67e17b2aa..707b0d4bbff4 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.
> 
> @@ -826,9 +856,13 @@ SevEsIsEnabled (
>  {
>    SEC_SEV_ES_WORK_AREA  *SevEsWorkArea;
> 
> +  if (!IsSevGuest()) {
> +    return FALSE;
> +  }
> +
>    SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32
> (PcdSevEsWorkAreaBase);
> 
> -  return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
> +  return (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	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm
  2021-08-17 13:46 ` [PATCH v3 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm Brijesh Singh
@ 2021-08-19 14:15   ` Min Xu
  0 siblings, 0 replies; 9+ messages in thread
From: Min Xu @ 2021-08-19 14:15 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

Reviewed-by: Min Xu <min.m.xu@intel.com>

> -----Original Message-----
> From: Brijesh Singh <brijesh.singh@amd.com>
> Sent: Tuesday, August 17, 2021 9:47 PM
> 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 v3 3/3] OvmfPkg/ResetVector: move the GHCB page setup in
> AmdSev.asm
> 
> 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       | 111 +++++++++++++++++-----
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm |  53 ++---------
>  2 files changed, 92 insertions(+), 72 deletions(-)
> 
> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> b/OvmfPkg/ResetVector/Ia32/AmdSev.asm
> index 87d81b01e263..250ac8d8b180 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,70 @@ BITS    32
>  SevEsUnexpectedRespTerminate:
>      TerminateVmgExit    TERM_UNEXPECTED_RESP_CODE
> 
> +; If SEV-ES is enabled then initialize and make the GHCB page shared
> +SevClearPageEncMaskForGhcbPage:
> +    ; Check if SEV is enabled
> +    cmp       byte[WORK_AREA_GUEST_TYPE], 1
> +    jnz       SevClearPageEncMaskForGhcbPageExit
> +
> +    ; Check if SEV-ES is enabled
> +    cmp       byte[SEV_ES_WORK_AREA], 1
> +    jnz       SevClearPageEncMaskForGhcbPageExit
> +
> +    ;
> +    ; 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
> +
> +SevClearPageEncMaskForGhcbPageExit:
> +    OneTimeCallRet SevClearPageEncMaskForGhcbPage
> +
> +; Check if SEV is enabled, and get the C-bit mask above 31.
> +; Modified: EDX
> +;
> +; The value is returned in the EDX
> +GetSevCBitMaskAbove31:
> +    xor       edx, edx
> +
> +    ; Check if SEV is enabled
> +    cmp       byte[WORK_AREA_GUEST_TYPE], 1
> +    jnz       GetSevCBitMaskAbove31Exit
> +
> +    mov       edx, dword[SEV_ES_WORK_AREA_ENC_MASK + 4]
> +
> +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
> +334,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..07b6ca070909 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   SevClearPageEncMaskForGhcbPage
> 
>  SetCr3:
>      ;
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 0/3] reuse the SevEsWork area
  2021-08-17 13:46 [PATCH v3 0/3] reuse the SevEsWork area Brijesh Singh
                   ` (2 preceding siblings ...)
  2021-08-17 13:46 ` [PATCH v3 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm Brijesh Singh
@ 2021-08-25  6:31 ` Yao, Jiewen
  2021-08-27 13:14 ` Yao, Jiewen
  4 siblings, 0 replies; 9+ messages in thread
From: Yao, Jiewen @ 2021-08-25  6:31 UTC (permalink / raw)
  To: Brijesh Singh, devel@edk2.groups.io
  Cc: James Bottomley, Xu, Min M, Tom Lendacky, Justen, Jordan L,
	Ard Biesheuvel, Erdem Aktas, Michael Roth

Thank you Brijesh.
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

Since we are in code freeze, I will merge after we finalize the stable tag 202108.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Brijesh Singh <brijesh.singh@amd.com>
> Sent: Tuesday, August 17, 2021 9:47 PM
> 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 v3 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 v2:
>  - address Tom's feedback
> 
> 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            |   8 +-
>  OvmfPkg/Sec/SecMain.c                      |  36 ++++++-
>  OvmfPkg/OvmfPkgDefines.fdf.inc             |   6 ++
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm        | 109 +++++++++++++++++----
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm  |  57 +++--------
>  OvmfPkg/ResetVector/ResetVector.nasmb      |   1 +
>  13 files changed, 238 insertions(+), 95 deletions(-)
>  create mode 100644 OvmfPkg/Include/WorkArea.h
> 
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3 0/3] reuse the SevEsWork area
  2021-08-17 13:46 [PATCH v3 0/3] reuse the SevEsWork area Brijesh Singh
                   ` (3 preceding siblings ...)
  2021-08-25  6:31 ` [PATCH v3 0/3] reuse the SevEsWork area Yao, Jiewen
@ 2021-08-27 13:14 ` Yao, Jiewen
  4 siblings, 0 replies; 9+ messages in thread
From: Yao, Jiewen @ 2021-08-27 13:14 UTC (permalink / raw)
  To: Brijesh Singh, devel@edk2.groups.io
  Cc: James Bottomley, Xu, Min M, Tom Lendacky, Justen, Jordan L,
	Ard Biesheuvel, Erdem Aktas, Michael Roth

Patch is pushed - 80e67af9afcac3b860384cdb1f4613f7240e1630.. b9af5037b270c4767b275fd5d23b942c422e742f

> -----Original Message-----
> From: Brijesh Singh <brijesh.singh@amd.com>
> Sent: Tuesday, August 17, 2021 9:47 PM
> 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 v3 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 v2:
>  - address Tom's feedback
> 
> 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            |   8 +-
>  OvmfPkg/Sec/SecMain.c                      |  36 ++++++-
>  OvmfPkg/OvmfPkgDefines.fdf.inc             |   6 ++
>  OvmfPkg/ResetVector/Ia32/AmdSev.asm        | 109 +++++++++++++++++----
>  OvmfPkg/ResetVector/Ia32/PageTables64.asm  |  57 +++--------
>  OvmfPkg/ResetVector/ResetVector.nasmb      |   1 +
>  13 files changed, 238 insertions(+), 95 deletions(-)
>  create mode 100644 OvmfPkg/Include/WorkArea.h
> 
> --
> 2.17.1


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-08-27 13:14 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-17 13:46 [PATCH v3 0/3] reuse the SevEsWork area Brijesh Singh
2021-08-17 13:46 ` [PATCH v3 1/3] OvmfPkg: introduce a common work area Brijesh Singh
2021-08-19 14:14   ` [edk2-devel] " Min Xu
2021-08-17 13:46 ` [PATCH v3 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format Brijesh Singh
2021-08-19 14:15   ` Min Xu
2021-08-17 13:46 ` [PATCH v3 3/3] OvmfPkg/ResetVector: move the GHCB page setup in AmdSev.asm Brijesh Singh
2021-08-19 14:15   ` Min Xu
2021-08-25  6:31 ` [PATCH v3 0/3] reuse the SevEsWork area Yao, Jiewen
2021-08-27 13:14 ` Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox