[edk2-devel] [PATCH 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format
Lendacky, Thomas via groups.io
thomas.lendacky=amd.com at groups.io
Mon Aug 9 16:54:34 UTC 2021
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 <jejb at linux.ibm.com>
> Cc: Min Xu <min.m.xu at intel.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Cc: Tom Lendacky <thomas.lendacky at amd.com>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Erdem Aktas <erdemaktas at google.com>
> Signed-off-by: Brijesh Singh <brijesh.singh at 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/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list