On 02/05/20 00:01, Lendacky, Thomas wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
>
> An SEV-ES guest will generate a #VC exception when it encounters a
> non-automatic exit (NAE) event. It is expected that the #VC exception
> handler will communicate with the hypervisor using the GHCB to handle
> the NAE event.
>
> NAE events can occur during the Sec phase, so initialize exception
> handling early in the OVMF Sec support.
>
> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Signed-off-by: Tom Lendacky <thomas.lenda...@amd.com>
> ---
> OvmfPkg/Sec/SecMain.inf | 2 ++
> OvmfPkg/Sec/SecMain.c | 76 ++++++++++++++++++++++++++++++++++-------
> 2 files changed, 65 insertions(+), 13 deletions(-)
>
> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
> index 63ba4cb555fb..401d06039dd3 100644
> --- a/OvmfPkg/Sec/SecMain.inf
> +++ b/OvmfPkg/Sec/SecMain.inf
> @@ -50,11 +50,13 @@ [LibraryClasses]
> PeCoffExtraActionLib
> ExtractGuidedSectionLib
> LocalApicLib
> + CpuExceptionHandlerLib
>
> [Ppis]
> gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED
>
> [Pcd]
> + gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
> index bae9764577f0..2bab7128ade2 100644
> --- a/OvmfPkg/Sec/SecMain.c
> +++ b/OvmfPkg/Sec/SecMain.c
> @@ -24,6 +24,7 @@
> #include <Library/PeCoffExtraActionLib.h>
> #include <Library/ExtractGuidedSectionLib.h>
> #include <Library/LocalApicLib.h>
> +#include <Library/CpuExceptionHandlerLib.h>
>
> #include <Ppi/TemporaryRamSupport.h>
>
> @@ -34,6 +35,10 @@ typedef struct _SEC_IDT_TABLE {
> IA32_IDT_GATE_DESCRIPTOR IdtTable[SEC_IDT_ENTRY_COUNT];
> } SEC_IDT_TABLE;
>
> +typedef struct _SEC_SEV_ES_WORK_AREA {
> + UINT8 SevEsEnabled;
> +} SEC_SEV_ES_WORK_AREA;
> +
> VOID
> EFIAPI
> SecStartupPhase2 (
> @@ -712,6 +717,19 @@ FindAndReportEntryPoints (
> return;
> }
>
> +STATIC
> +BOOLEAN
> +SevEsIsEnabled (
> + VOID
> + )
> +{
> + SEC_SEV_ES_WORK_AREA *SevEsWorkArea;
> +
> + SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32
> (PcdSevEsWorkAreaBase);
> +
> + return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0));
> +}
> +
Ah so this is how we handle the IA32 build then -- the PCD will be zero,
except in "OvmfPkgX64.fdf". OK.
> VOID
> EFIAPI
> SecCoreStartupWithStack (
> @@ -737,8 +755,53 @@ SecCoreStartupWithStack (
> Table[Index] = 0;
> }
>
> + //
> + // Initialize IDT - Since this is before library constructors are called,
> + // we use a loop rather than CopyMem.
> + //
> + IdtTableInStack.PeiService = NULL;
> + for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
> + UINT8 *Src, *Dst;
> + UINTN Byte;
> +
> + Src = (UINT8 *) &mIdtEntryTemplate;
> + Dst = (UINT8 *) &IdtTableInStack.IdtTable[Index];
> + for (Byte = 0; Byte < sizeof (mIdtEntryTemplate); Byte++) {
> + Dst[Byte] = Src[Byte];
> + }
> + }
> +
> + IdtDescriptor.Base = (UINTN)&IdtTableInStack.IdtTable;
> + IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
> +
> + if (SevEsIsEnabled()) {
> + //
> + // For SEV-ES guests, the exception handler is needed before calling
> + // ProcessLibraryConstructorList() because some of the library
> constructors
> + // perform some functions that result in #VC exceptions being generated.
> + //
> + // Due to this code executing before library constructors, *all* library
> + // API calls are theoretically interface contract violations. However,
> + // because this is SEC (executing in flash), those constructors cannot
> + // write variables with static storage duration anyway. Furthermore, only
> + // a small, restricted set of APIs, such as AsmWriteIdtr() and
> + // InitializeCpuExceptionHandlers(), are called, where we require that
> the
> + // underlying library not require constructors to have been invoked and
> + // that the library instance not trigger any #VC exceptions.
> + //
> + AsmWriteIdtr (&IdtDescriptor);
> + InitializeCpuExceptionHandlers (NULL);
> + }
> +
> ProcessLibraryConstructorList (NULL, NULL);
>
> + if (!SevEsIsEnabled()) {
> + //
> + // For non SEV-ES guests, just load the IDTR.
> + //
> + AsmWriteIdtr (&IdtDescriptor);
> + }
> +
> DEBUG ((EFI_D_INFO,
> "SecCoreStartupWithStack(0x%x, 0x%x)\n",
> (UINT32)(UINTN)BootFv,
> @@ -751,19 +814,6 @@ SecCoreStartupWithStack (
> //
> InitializeFloatingPointUnits ();
>
> - //
> - // Initialize IDT
> - //
> - IdtTableInStack.PeiService = NULL;
> - for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
> - CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof
> (mIdtEntryTemplate));
> - }
> -
> - IdtDescriptor.Base = (UINTN)&IdtTableInStack.IdtTable;
> - IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
> -
> - AsmWriteIdtr (&IdtDescriptor);
> -
> #if defined (MDE_CPU_X64)
> //
> // ASSERT that the Page Tables were set by the reset vector code to
>
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Thanks,
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#53849): https://edk2.groups.io/g/devel/message/53849
Mute This Topic: https://groups.io/mt/70985002/21656
Mute #vc: https://groups.io/mk?hashtag=vc&subid=3846945
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
