Re: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector


Min Xu
 

I noticed SEV has the memory region of SEV_ES_WORK_AREA (gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase) in MEMFD
The definition is below:
typedef struct _SEC_SEV_ES_WORK_AREA {
UINT8 SevEsEnabled;
UINT8 Reserved1[7];
UINT64 RandomData;
UINT64 EncryptionMask;
} SEC_SEV_ES_WORK_AREA;

In ResetVector SEV flag is recorded in the first byte of SEV_ES_WORK_AREA. In SecMain.c this flag is read to determine SEV.

I am thinking whether this memory region can be used by both TDX and SEV. (This is option 5)

SEV_ES_WORK_AREA will be added in tdx metadata so that it is initialized by host VMM and can be accessed in Tdx guest.
In SEV guest I believe SEV_ES_WORK_AREA can be accessed without any error.
The first 8 bytes of SEV_ES_WORK_AREA can be redefined as [CC Indicator Flags].

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Wednesday, July 28, 2021 3:55 PM
To: Xu, Min M <min.m.xu@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>; devel@edk2.groups.io; Ard
Biesheuvel <ardb+tianocore@kernel.org>; Justen, Jordan L
<jordan.l.justen@intel.com>; Erdem Aktas <erdemaktas@google.com>;
James Bottomley <jejb@linux.ibm.com>; Tom Lendacky
<thomas.lendacky@amd.com>
Subject: RE: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

Yes. I am thinking the same thing.

[CC Flag memory location]
1) A general purpose register, such as EBP.

2) A global variable, such as
.data
TeeFlags: DD 0

3) A fixed region in stack, such as
dword[STACK_TOP - 4]

4) A new CC common fixed region, such as dword[CC_COMMON_FLAGS]

5) A fixed region piggyback on existing CC working area, such as
dword[CC_WORKING_AREA]

Hi Brijesh/Min
Any preference?

[CC Indicator Flags]
Proposal: UINT8[4]

Byte [0] Version: 0
byte [1] Length: 4
byte [2] Type:
0: legacy
1: SEV
2: TDX
byte [3] Sub Type:
If Type is 0 (legacy), then
0: legacy
If Type is 1 (SEV), then
0: SEV
1: SEV-ES
2: SEV-SNP
If Type is 2 (TDX), then
0: TDX 1.0

Thank you
Yao Jiewen


-----Original Message-----
From: Xu, Min M <min.m.xu@intel.com>
Sent: Wednesday, July 28, 2021 2:58 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Brijesh Singh <brijesh.singh@amd.com>; devel@edk2.groups.io; Ard
Biesheuvel <ardb+tianocore@kernel.org>; Justen, Jordan L
<jordan.l.justen@intel.com>; Erdem Aktas <erdemaktas@google.com>;
James Bottomley <jejb@linux.ibm.com>; Tom Lendacky
<thomas.lendacky@amd.com>
Subject: RE: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector

On July 28, 2021 2:05 PM, Yao, Jiewen wrote:
It does not necessary to be a working area.

We just need a common TEE flag to indicate if the system run in
legacy, SEV,
or
TDX, right?
Right. We need somewhere to store this flag, either in a Register or in
Memory.
If it is memory, then in Tdx the memory region should be initialized by host
VMM.

thank you!
Yao, Jiewen


在 2021年7月28日,下午1:07,Xu, Min M <min.m.xu@intel.com>
写道:

On July 27, 2021 8:46 PM, Yao, Jiewen wrote:
HI Min
I agree with Brijesh.

The basic rule is: SEV file shall never refer to TDX data structure.
TDX file shall never refer to SEV data structure.
These code should be isolated clearly.

Do we still need that logic if we follow the new pattern?
I have replied to Brijesh's mail about the concern of the new pattern.

I have some concern in the current pattern:
====================
OneTimeCall PreMainFunctionHookSev
OneTimeCall PreMainFunctionHookTdx
MainFunction:
XXXXXX
OneTimeCall PostMainFunctionHookSev
OneTimeCall PostMainFunctionHookTdx
====================
The TEE function need implement a TEE check function (such as
IsSev, or
IsTdx).

Tdx call CPUID(0x21) to determine if it is tdx guest in the very
beginning of ResetVector. Then 'TDXG' is set in TDX_WORK_AREA. SEV
does
the similar work which call CheckSevFeatures to set SEV_ES_WORK_AREA
to 1.

After that both TDX and SEV read the above WORK_AREA to check if
it is TDX
or SEV or legacy guest.

In Tdx the access to SEV_ES_WORK_AREA will trigger error because
SEV_ES_WORK_AREA is *NOT* initialized by host VMM.
In SEV-SNP I am afraid the access to TDX_WORK_AREA will trigger error
too.

I am wondering if TDX and SEV can use the same memory region (for
example,
TEE_WORK_AREA) as the work area?
So that this work area is guaranteed to be initialized in both TDX
and SEV. Structure of the TEE_WORK_AREA may look like this:
typedef struct {
UINT8 Flag[4]; 'TDXG' or 'SEVG' or all-0
UINT8 Others[];
} TEE_WORK_AREA;

Join devel@edk2.groups.io to automatically receive all group messages.