On 8/4/21 3:20 PM, Brijesh Singh wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3429 > > Update the SEV support to switch to using the newer work area format. > > Cc: James Bottomley <j...@linux.ibm.com> > Cc: Min Xu <min.m...@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Tom Lendacky <thomas.lenda...@amd.com> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Cc: Ard Biesheuvel <ardb+tianoc...@kernel.org> > Cc: Erdem Aktas <erdemak...@google.com> > Signed-off-by: Brijesh Singh <brijesh.si...@amd.com> > --- > OvmfPkg/ResetVector/ResetVector.inf | 1 + > OvmfPkg/Sec/SecMain.inf | 1 + > OvmfPkg/Sec/SecMain.c | 25 ++++++++++++++++++++++- > OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 ++++++++ > OvmfPkg/ResetVector/Ia32/PageTables64.asm | 4 ++++ > OvmfPkg/ResetVector/ResetVector.nasmb | 1 + > 6 files changed, 39 insertions(+), 1 deletion(-) > > diff --git a/OvmfPkg/ResetVector/ResetVector.inf > b/OvmfPkg/ResetVector/ResetVector.inf > index d028c92d8cfa..6ec9cca40c3a 100644 > --- a/OvmfPkg/ResetVector/ResetVector.inf > +++ b/OvmfPkg/ResetVector/ResetVector.inf > @@ -34,6 +34,7 @@ [BuildOptions] > *_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/ > > [Pcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase
Laszlo was trying to keep things sorted, so you should move this down to the end of the list. > gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize > diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf > index 7f78dcee2772..82910dcbd5c2 100644 > --- a/OvmfPkg/Sec/SecMain.inf > +++ b/OvmfPkg/Sec/SecMain.inf > @@ -56,6 +56,7 @@ [Ppis] > gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED > > [Pcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase Ditto here, even though the list isn't truly sorted to begin with. > gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c > index 9db67e17b2aa..dda572c7ad7d 100644 > --- a/OvmfPkg/Sec/SecMain.c > +++ b/OvmfPkg/Sec/SecMain.c > @@ -807,6 +807,29 @@ SevEsProtocolCheck ( > Ghcb->GhcbUsage = GHCB_STANDARD_USAGE; > } > > +/** > + Determine if the SEV is active. > + > + During the early booting, GuestType is set in the work area. Verify that it > + is an SEV guest. > + > + @retval TRUE SEV is enabled > + @retval FALSE SEV is not enabled > + > +**/ > +STATIC > +BOOLEAN > +IsSevGuest ( > + VOID > + ) > +{ > + OVMF_WORK_AREA *WorkArea; > + > + WorkArea = (OVMF_WORK_AREA *) FixedPcdGet32 (PcdOvmfWorkAreaBase); > + > + return ((WorkArea != NULL) && (WorkArea->GuestType == GUEST_TYPE_AMD_SEV)); > +} > + > /** > Determine if SEV-ES is active. > > @@ -828,7 +851,7 @@ SevEsIsEnabled ( > > SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 > (PcdSevEsWorkAreaBase); > > - return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0)); > + return (((IsSevGuest()) && SevEsWorkArea != NULL) && > (SevEsWorkArea->SevEsEnabled != 0)); The IsSevGuest() function checks for a NULL work area, so there's no need to check for SevEsWorkArea being non-NULL now. I think it would read better, though, to do: if (!IsSevGuest ()) { return FALSE; } SevEsWorkArea = ... return (SevEsWorkArea->SevEsEnabled != 0); > } > > VOID > diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm > b/OvmfPkg/ResetVector/Ia32/AmdSev.asm > index aa95d06eaddb..87d81b01e263 100644 > --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm > +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm > @@ -171,6 +171,9 @@ CheckSevFeatures: > bt eax, 0 > jnc NoSev > > + ; Set the work area header to indicate that the SEV is enabled s/the SEV/SEV/ > + mov byte[WORK_AREA_GUEST_TYPE], 1 The "1" should probably be defined in ResetVector.nasmb as a %define. > + > ; Check for SEV-ES memory encryption feature: > ; CPUID Fn8000_001F[EAX] - Bit 3 > ; CPUID raises a #VC exception if running as an SEV-ES guest > @@ -257,6 +260,11 @@ SevExit: > IsSevEsEnabled: > xor eax, eax > > + ; During CheckSevFeatures, the WORK_AREA_GUEST_TYPE is set > + ; to 1 if SEV is enabled. > + cmp byte[WORK_AREA_GUEST_TYPE], 1 > + jne SevEsDisabled > + > ; During CheckSevFeatures, the SEV_ES_WORK_AREA was set to 1 if > ; SEV-ES is enabled. > cmp byte[SEV_ES_WORK_AREA], 1 > diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm > b/OvmfPkg/ResetVector/Ia32/PageTables64.asm > index eacdb69ddb9f..f688909f1c7d 100644 > --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm > +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm > @@ -42,6 +42,10 @@ BITS 32 > ; > SetCr3ForPageTables64: > > + ; Clear the WorkArea header. The SEV probe routines will populate the How about: ; Initialize the WorkArea header to indicate a legacy guest. The ... > + ; work area when detected. > + mov byte[WORK_AREA_GUEST_TYPE], 0 And then use a %define here for the '0' > + > OneTimeCall CheckSevFeatures > xor edx, edx > test eax, eax > diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb > b/OvmfPkg/ResetVector/ResetVector.nasmb > index acec46a32450..d1d800c56745 100644 > --- a/OvmfPkg/ResetVector/ResetVector.nasmb > +++ b/OvmfPkg/ResetVector/ResetVector.nasmb > @@ -72,6 +72,7 @@ > %define GHCB_PT_ADDR (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase)) > %define GHCB_BASE (FixedPcdGet32 (PcdOvmfSecGhcbBase)) > %define GHCB_SIZE (FixedPcdGet32 (PcdOvmfSecGhcbSize)) > + %define WORK_AREA_GUEST_TYPE (FixedPcdGet32 (PcdOvmfWorkAreaBase)) Create %defines for each of the defined enum types from the first patch. Thanks, Tom > %define SEV_ES_WORK_AREA (FixedPcdGet32 (PcdSevEsWorkAreaBase)) > %define SEV_ES_WORK_AREA_RDRAND (FixedPcdGet32 (PcdSevEsWorkAreaBase) + 8) > %define SEV_ES_WORK_AREA_ENC_MASK (FixedPcdGet32 (PcdSevEsWorkAreaBase) + > 16) > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#78979): https://edk2.groups.io/g/devel/message/78979 Mute This Topic: https://groups.io/mt/84670985/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-