Hi Jiewen,
Thanks for the quick feedback. I will make the recommended change and
send the updated patch. I was under assumption that union will be done
when Min adds the SGX support because that's when we start reusing the
WorkArea for SEV and TDX. But I guess its good idea for me to do it now
so that Min does not have to touch the SEV code in his series.
thanks
On 8/4/21 9:18 PM, Yao, Jiewen wrote:
> 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://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%7C4c55a642f1804a803c4e08d957b75e61%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637637267367225365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=NSsUVfQodJMDUcpLCsHSpTaRDHM8et%2BWZJOS8lCS3Kw%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 <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 (#78706): https://edk2.groups.io/g/devel/message/78706
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]
-=-=-=-=-=-=-=-=-=-=-=-
