Re: [PATCH V12 33/47] OvmfPkg: Update Sec to support Tdx

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 4/15/22 19:13, Xu, Min M wrote:
> On April 16, 2022 4:05 AM, Tom Lendacky wrote:
>>>    #define SEC_IDT_ENTRY_COUNT  34
>>> @@ -738,6 +737,20 @@ SecCoreStartupWithStack (
>>>      UINT32                Index;
>>>      volatile UINT8        *Table;
>>>
>>> + #if defined (TDX_GUEST_SUPPORTED)
>>> +  if (TdIsEnabled ()) {
>>
>> I wish I had caught this earlier, but this patch breaks SEV-ES support.
>> TdIsEnabled() uses the CPUID instruction. At this point, exception handling is
>> not established and a CPUID instruction will generate a #VC and cause the
>> booting guest to crash.
>>
> Sorry for the broken.
>>
>> That is why the SevEsIsEnabled() function checks the work area to determine
>> if SEV-ES is supported. In the early boot code we established a temporary
>> #VC handler to specifically handle CPUID and then set the work area
>> indicator that SEV-ES is enabled.
>>
>> I think you'll need to do something similar for this area. Haven't you already
>> set the workarea from calling InitTdx before this point?
> TDX has set the workarea in ResetVector.
> I am working on a patch-set (now it is v2) which is to fix the issues caused by TdIsEnabled. Please see https://edk2.groups.io/g/devel/message/88916
> This patch-set introduce CcProbe() which checks the Ovmf work area to return the guest type.
> In the next version CcProbe will be called instead of TdIsEnabled in SecMain.c.
> 
> Please help to review the above patch-set so that there will not be more broken in the future.

I'll test out that patchset on Monday. Thanks!

Tom

> 
> Thanks much
> Min