[edk2-devel] [PATCH 0/3] reuse the SevEsWork area
Yao, Jiewen
jiewen.yao at intel.com
Thu Aug 5 15:07:25 UTC 2021
Ah, sorry, I did not realize that you are using that in FDF.
If we have to define the PcdOvmfWorkAreaHeaderSize, then I would suggest:
1) Add detail comment in the PCD definition - it must be same as sizeof(Header).
2) Add ASSERT in the code to ensure PcdOvmfWorkAreaHeaderSize is same as sizeof(Header).
Just in case someone change only one value, it can be caught in the code.
Thank you
Yao Jiewen
> -----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 10:44 PM
> To: Yao, Jiewen <jiewen.yao at intel.com>; devel at edk2.groups.io
> Cc: brijesh.singh at amd.com; James Bottomley <jejb at linux.ibm.com>; Xu, Min M
> <min.m.xu 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>
> Subject: Re: [edk2-devel] [PATCH 0/3] reuse the SevEsWork area
>
> 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.c
> om%2FAMDESE%2Fovmf%2Ftree%2Fsev-new-work-
> area&data=04%7C01%7Cbrijesh.singh%40amd.com%7C4c55a642f1804a80
> 3c4e08d957b75e61%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6
> 37637267367225365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi
> LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=NSs
> UVfQodJMDUcpLCsHSpTaRDHM8et%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 (#78716): https://edk2.groups.io/g/devel/message/78716
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]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list