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

Min Xu posted 3 patches 4 years, 6 months ago
There is a newer version of this series
[edk2-devel] [PATCH V4 2/3] OvmfPkg/Sec: Update the check logic in SevEsIsEnabled
Posted by Min Xu 4 years, 6 months ago
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));
 }
 
 VOID
-- 
2.29.2.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78577): https://edk2.groups.io/g/devel/message/78577
Mute This Topic: https://groups.io/mt/84631105/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH V4 2/3] OvmfPkg/Sec: Update the check logic in SevEsIsEnabled
Posted by Brijesh Singh via groups.io 4 years, 6 months ago
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.

thanks

>   
>   VOID
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78617): https://edk2.groups.io/g/devel/message/78617
Mute This Topic: https://groups.io/mt/84631105/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH V4 2/3] OvmfPkg/Sec: Update the check logic in SevEsIsEnabled
Posted by Min Xu 4 years, 6 months ago
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


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78620): https://edk2.groups.io/g/devel/message/78620
Mute This Topic: https://groups.io/mt/84631105/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-