Re: [PATCH V4 2/3] OvmfPkg/Sec: Update the check logic in SevEsIsEnabled

["Min Xu" <min.m.xu@intel.com>]

On August 4, 2021 3:24 AM, Brijesh Singh wrote:
> Hi Min,
> 
> On 8/2/21 8:18 PM, Min Xu wrote:
> > RFC: https://bugzilla.tianocore.org/show_bug.cgi?id=3429
> >
> > SevEsIsEnabled return TRUE if SevEsWorkArea->SevEsEnabled is non-zero.
> > It is correct when SevEsWorkArea is only used by SEV. After Intel TDX
> > is enabled in Ovmf, the SevEsWorkArea is shared by TDX and SEV. (This
> > is to avoid the waist of memory region in MEMFD). The value of
> > SevEsWorkArea->SevEsEnabled now is :
> >   0 if in Legacy guest
> >   1 if in SEV
> >   2 if in Tdx guest
> > That's why the changes is made.
> >
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Brijesh Singh <brijesh.singh@amd.com>
> > Cc: Erdem Aktas <erdemaktas@google.com>
> > Cc: James Bottomley <jejb@linux.ibm.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Tom Lendacky <thomas.lendacky@amd.com>
> > Signed-off-by: Min Xu <min.m.xu@intel.com>
> > ---
> >   OvmfPkg/Sec/SecMain.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c index
> > 9db67e17b2aa..e166a9389a1a 100644
> > --- a/OvmfPkg/Sec/SecMain.c
> > +++ b/OvmfPkg/Sec/SecMain.c
> > @@ -828,7 +828,7 @@ SevEsIsEnabled (
> >
> >     SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32
> > (PcdSevEsWorkAreaBase);
> >
> > -  return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled !=
> > 0));
> > +  return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled ==
> > + 1));
> >   }
> 
> This is wrong, we need to check the SevEs sub type and not the global Sev
> enable. This also need to be broken into at least two commits
> 
> 1. introduce the updated CcWorkArea structure 2. update the existing code to
> use the CcWorkArea layout
> 
> If you are okay then I can rework and send the patch so that you can add the
> TDX on top of it.
That will be great If you can rework the SEV parts. Thanks much Brijesh! 
> 

Thanks!
Xu, Min