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

Brijesh Singh via groups.io posted 3 patches 2 years, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
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
[edk2-devel] [PATCH 0/3] reuse the SevEsWork area
Posted by Brijesh Singh via groups.io 2 years, 7 months ago
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 <jejb@linux.ibm.com>
Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Erdem Aktas <erdemaktas@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 (#78667): https://edk2.groups.io/g/devel/message/78667
Mute This Topic: https://groups.io/mt/84670983/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/3] reuse the SevEsWork area
Posted by Yao, Jiewen 2 years, 7 months ago
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 <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>;
> Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
> <thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> Ard Biesheuvel <ardb+tianocore@kernel.org>; Erdem Aktas
> <erdemaktas@google.com>; Michael Roth <Michael.Roth@amd.com>; Brijesh
> Singh <brijesh.singh@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 <jejb@linux.ibm.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Erdem Aktas <erdemaktas@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/3] reuse the SevEsWork area
Posted by Brijesh Singh via groups.io 2 years, 7 months ago
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 <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>;
>> Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
>> <thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
>> Ard Biesheuvel <ardb+tianocore@kernel.org>; Erdem Aktas
>> <erdemaktas@google.com>; Michael Roth <Michael.Roth@amd.com>; Brijesh
>> Singh <brijesh.singh@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&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C4c55a642f1804a803c4e08d957b75e61%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637637267367225365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=NSsUVfQodJMDUcpLCsHSpTaRDHM8et%2BWZJOS8lCS3Kw%3D&amp;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@linux.ibm.com>
>> Cc: Min Xu <min.m.xu@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Erdem Aktas <erdemaktas@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/3] reuse the SevEsWork area
Posted by Brijesh Singh via groups.io 2 years, 7 months ago
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@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 <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>;
>> Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
>> <thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
>> Ard Biesheuvel <ardb+tianocore@kernel.org>; Erdem Aktas
>> <erdemaktas@google.com>; Michael Roth <Michael.Roth@amd.com>; Brijesh
>> Singh <brijesh.singh@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&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C4c55a642f1804a803c4e08d957b75e61%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637637267367225365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=NSsUVfQodJMDUcpLCsHSpTaRDHM8et%2BWZJOS8lCS3Kw%3D&amp;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@linux.ibm.com>
>> Cc: Min Xu <min.m.xu@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Cc: Tom Lendacky <thomas.lendacky@amd.com>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Erdem Aktas <erdemaktas@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH 0/3] reuse the SevEsWork area
Posted by Yao, Jiewen 2 years, 7 months ago
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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
> Singh via groups.io
> Sent: Thursday, August 5, 2021 10:44 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> Cc: brijesh.singh@amd.com; James Bottomley <jejb@linux.ibm.com>; Xu, Min M
> <min.m.xu@intel.com>; Tom Lendacky <thomas.lendacky@amd.com>; Justen,
> Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Erdem Aktas <erdemaktas@google.com>;
> Michael Roth <Michael.Roth@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@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 <jejb@linux.ibm.com>; Xu, Min M
> <min.m.xu@intel.com>;
> >> Yao, Jiewen <jiewen.yao@intel.com>; Tom Lendacky
> >> <thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
> >> Ard Biesheuvel <ardb+tianocore@kernel.org>; Erdem Aktas
> >> <erdemaktas@google.com>; Michael Roth <Michael.Roth@amd.com>;
> Brijesh
> >> Singh <brijesh.singh@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&amp;data=04%7C01%7Cbrijesh.singh%40amd.com%7C4c55a642f1804a80
> 3c4e08d957b75e61%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C6
> 37637267367225365%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAi
> LCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=NSs
> UVfQodJMDUcpLCsHSpTaRDHM8et%2BWZJOS8lCS3Kw%3D&amp;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@linux.ibm.com>
> >> Cc: Min Xu <min.m.xu@intel.com>
> >> Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> >> Cc: Jordan Justen <jordan.l.justen@intel.com>
> >> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> >> Cc: Erdem Aktas <erdemaktas@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-