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

[Brijesh Singh via groups.io]

Hi Jiewen,

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.
> 

Can you use the sizeof() inside the OvmfPkg.fdf ? I was not able to find 
a reference usage for it in this file. Also, can a .fdf refer the C 
header file ?

We need to know the size of the header so that we can set the fixed 
PcdOvmfSevEsWorkArea. In the current approach the .fdf sets the PCD as 
shown below:

SET PcdOvmfSevEsWorkArea = PcdOvmfWorkAreaBase + <HeaderSize>

I am hard coding the header size to be 4 so that pcd points to the 
correct location within the WorkArea.

thanks


> 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 (#78709): https://edk2.groups.io/g/devel/message/78709
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]
-=-=-=-=-=-=-=-=-=-=-=-