HI Brijesh
Thanks for the startup. Feedback below:
1) I don't think we need a PCD to indicate the header.
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaHeaderSize|4|UINT32|0x51
Instead, if we define a HEADER structure, we can use sizeof() naturally.
Otherwise, when we update this header, we need update 2 different places, which
is not preferred.
typedef struct {
UINT8 GuestType;
UINT8 Reserved1[3];
} CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER;
2) I don't think we can define a common structure OVMF_WORK_AREA to contain SEV
specific field.
typedef struct _OVMF_WORK_AREA {
UINT8 GuestType;
UINT8 Reserved1[3];
SEC_SEV_ES_WORK_AREA SevEsWorkArea;
} OVMF_WORK_AREA;
A common patter is to define each individual structure, then use UNION.
For example,
typedef struct {
UINT8 GuestType;
UINT8 Reserved1[3];
SEC_SEV_ES_WORK_AREA SevEsWorkArea;
} SEV_WORK_AREA;
typedef union {
CONFIDENTIAL_COMPUTING_WORK_AREA_HEADER Header;
SEV_WORK_AREA Sev;
} OVMF_WORK_AREA;
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
> Singh via groups.io
> Sent: Thursday, August 5, 2021 4:20 AM
> To: devel@edk2.groups.io
> Cc: James Bottomley <j...@linux.ibm.com>; Xu, Min M <min.m...@intel.com>;
> Yao, Jiewen <jiewen....@intel.com>; Tom Lendacky
> <thomas.lenda...@amd.com>; Justen, Jordan L <jordan.l.jus...@intel.com>;
> Ard Biesheuvel <ardb+tianoc...@kernel.org>; Erdem Aktas
> <erdemak...@google.com>; Michael Roth <michael.r...@amd.com>; Brijesh
> Singh <brijesh.si...@amd.com>
> Subject: [edk2-devel] [PATCH 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 <j...@linux.ibm.com>
> Cc: Min Xu <min.m...@intel.com>
> Cc: Jiewen Yao <jiewen....@intel.com>
> Cc: Tom Lendacky <thomas.lenda...@amd.com>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org>
> Cc: Erdem Aktas <erdemak...@google.com>
>
> OvmfPkg/OvmfPkg.dec | 6 ++
> OvmfPkg/OvmfPkgX64.fdf | 9 +-
> OvmfPkg/PlatformPei/PlatformPei.inf | 4 +-
> OvmfPkg/ResetVector/ResetVector.inf | 1 +
> OvmfPkg/Sec/SecMain.inf | 1 +
> OvmfPkg/Include/Library/MemEncryptSevLib.h | 21 +---
> OvmfPkg/Include/WorkArea.h | 53 ++++++++++
> OvmfPkg/PlatformPei/MemDetect.c | 32 +++---
> OvmfPkg/Sec/SecMain.c | 25 ++++-
> OvmfPkg/ResetVector/Ia32/AmdSev.asm | 111 +++++++++++++++++----
> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 57 ++---------
> OvmfPkg/ResetVector/ResetVector.nasmb | 1 +
> 12 files changed, 213 insertions(+), 108 deletions(-)
> create mode 100644 OvmfPkg/Include/WorkArea.h
>
> --
> 2.17.1
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78690): https://edk2.groups.io/g/devel/message/78690
Mute This Topic: https://groups.io/mt/84670983/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
