From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web11.9268.1580979842829483029 for ; Thu, 06 Feb 2020 01:04:03 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=i2zdqB30; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580979842; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Z97n13T7qVHiRvUsG+GZRmbYFbwFSDxhgrPgPhNKcx4=; b=i2zdqB30NsROLAoeF3bmKmmqBM7ME+MT7HVpxtyt898RsqdVKJrQB+RFIVHES1QcSKpskB ptgllU1P4azikpWOxdPYvvKTfqoqrPIt5v7tS20MX9LBBCO4ZZG1Abx+lfwyf7H1AQV5yY EgcplF/QwgWOXI9uJvPD9u0lRnvNPpU= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-367-y9oXvks4NMKUJiCRs5ruIg-1; Thu, 06 Feb 2020 04:03:58 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id B6CBF1137841; Thu, 6 Feb 2020 09:03:56 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-20.ams2.redhat.com [10.36.117.20]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7C54910016DA; Thu, 6 Feb 2020 09:03:54 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v4 32/40] OvmfPkg/Sec: Add #VC exception handling for Sec phase To: devel@edk2.groups.io, thomas.lendacky@amd.com Cc: Jordan Justen , Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , Brijesh Singh References: <2af37abfa288fb07caf3bdbfe694278caad203ac.1580857303.git.thomas.lendacky@amd.com> From: "Laszlo Ersek" Message-ID: <57b5b0f4-cee2-4924-bdf0-0e0476090abe@redhat.com> Date: Thu, 6 Feb 2020 10:03:53 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <2af37abfa288fb07caf3bdbfe694278caad203ac.1580857303.git.thomas.lendacky@amd.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-MC-Unique: y9oXvks4NMKUJiCRs5ruIg-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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 > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Signed-off-by: Tom Lendacky > --- > 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 > #include > #include > +#include > > #include > > @@ -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 Thanks, Laszlo