[edk2-devel] [PATCH v3 1/3] OvmfPkg: introduce a common work area
Min Xu
min.m.xu at intel.com
Thu Aug 19 14:14:33 UTC 2021
Reviewed-by: Min Xu <min.m.xu at intel.com>
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Brijesh
> Singh via groups.io
> Sent: Tuesday, August 17, 2021 9:47 PM
> To: devel at edk2.groups.io
> Cc: James Bottomley <jejb at linux.ibm.com>; Xu, Min M
> <min.m.xu at intel.com>; Yao, Jiewen <jiewen.yao at intel.com>; Tom Lendacky
> <thomas.lendacky at amd.com>; Justen, Jordan L <jordan.l.justen at intel.com>;
> Ard Biesheuvel <ardb+tianocore at kernel.org>; Erdem Aktas
> <erdemaktas at google.com>; Michael Roth <Michael.Roth at amd.com>; Brijesh
> Singh <brijesh.singh at 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 at linux.ibm.com>
> Cc: Min Xu <min.m.xu at intel.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Cc: Tom Lendacky <thomas.lendacky at amd.com>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Erdem Aktas <erdemaktas at google.com>
> Signed-off-by: Brijesh Singh <brijesh.singh at amd.com>
> ---
> OvmfPkg/OvmfPkg.dec | 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
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79580): https://edk2.groups.io/g/devel/message/79580
Mute This Topic: https://groups.io/mt/84947965/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list