[edk2-devel] [PATCH 0/3] reuse the SevEsWork area

[Brijesh Singh via groups.io]

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 at edk2.groups.io <devel at edk2.groups.io> On Behalf Of Brijesh
>> Singh via groups.io
>> Sent: Thursday, August 5, 2021 4:20 AM
>> 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 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 <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>
>>
>>  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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-