From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web09.22917.1574337997725803447 for ; Thu, 21 Nov 2019 04:06:37 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=Z5R5s3KT; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574337996; 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=dNMvup/xzZLJv44X5cDKIO3xMTlhgr6yj+t4Y+3wIN8=; b=Z5R5s3KT1YLDbyhFimLv4QbT26EycgSUe8U60EDRTGoBqAMDA+Ow3BQkZZhubznlQHlurN TWTlMquQ8o9Ch99XOO4ZljVdrUhGRBLahekNrGIfCTlSIxHpxDjThbQbvYNr2tqD+/CQLy 1RfNc/C45RNWCSUv6sHuBYS1gP1PmLk= 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-286-RH74oyYjNfW6up4_8YY2uQ-1; Thu, 21 Nov 2019 07:06:33 -0500 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8E06918A85B3; Thu, 21 Nov 2019 12:06:31 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-197.ams2.redhat.com [10.36.116.197]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2E37B29346; Thu, 21 Nov 2019 12:06:28 +0000 (UTC) Subject: Re: [edk2-devel] [RFC PATCH v3 30/43] 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: <041cb6f6352c9e0f9982d316cd842c2daf57e7fa.1574280425.git.thomas.lendacky@amd.com> From: "Laszlo Ersek" Message-ID: <12e35ff5-ff14-87d4-f4ac-604448289688@redhat.com> Date: Thu, 21 Nov 2019 13:06:28 +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: <041cb6f6352c9e0f9982d316cd842c2daf57e7fa.1574280425.git.thomas.lendacky@amd.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-MC-Unique: RH74oyYjNfW6up4_8YY2uQ-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable On 11/20/19 21:06, Lendacky, Thomas wrote: > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2198 >=20 > 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. >=20 > NAE events can occur during the Sec phase, so initialize exception > handling early in the OVMF Sec support. >=20 > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Signed-off-by: Tom Lendacky > --- > OvmfPkg/Sec/SecMain.inf | 1 + > OvmfPkg/Sec/SecMain.c | 29 ++++++++++++++++------------- > 2 files changed, 17 insertions(+), 13 deletions(-) >=20 > diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf > index 63ba4cb555fb..7f53845f5436 100644 > --- a/OvmfPkg/Sec/SecMain.inf > +++ b/OvmfPkg/Sec/SecMain.inf > @@ -50,6 +50,7 @@ [LibraryClasses] > PeCoffExtraActionLib > ExtractGuidedSectionLib > LocalApicLib > + CpuExceptionHandlerLib > =20 > [Ppis] > gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED > diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c > index bae9764577f0..db319030ee58 100644 > --- a/OvmfPkg/Sec/SecMain.c > +++ b/OvmfPkg/Sec/SecMain.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > =20 > #include > =20 > @@ -737,6 +738,21 @@ SecCoreStartupWithStack ( > Table[Index] =3D 0; > } > =20 > + // > + // Initialize IDT > + // > + IdtTableInStack.PeiService =3D NULL; > + for (Index =3D 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) { > + CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeo= f (mIdtEntryTemplate)); > + } > + > + IdtDescriptor.Base =3D (UINTN)&IdtTableInStack.IdtTable; > + IdtDescriptor.Limit =3D (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1= ); > + > + AsmWriteIdtr (&IdtDescriptor); > + > + InitializeCpuExceptionHandlers (NULL); > + > ProcessLibraryConstructorList (NULL, NULL); > =20 > DEBUG ((EFI_D_INFO, (1) The problem here is that we call multiple library APIs before calling ProcessLibraryConstructorList() -- namely CopyMem(), AsmWriteIdtr(), and InitializeCpuExceptionHandlers(). (See also the "SetMem" reference in the leading context, in the source file -- it is not quoted in this patch.) Thus, would it be possible to move all the "+" lines, quoted above, just below the ProcessLibraryConstructorList() call? (2) If possible I'd like to restrict the InitializeCpuExceptionHandlers() call to SevEsIsEnabled(). (Unless you're implying that InitializeCpuExceptionHandlers() is useful even without SEV-ES -- but then the commit message should be reworded accordingly.) If you agree to that restriction, then I further suggest reordering this patch against the next one: [edk2-devel] [RFC PATCH v3 31/43] OvmfPkg/Sec: Enable cache early to speed up booting Namely, if you put that patch first, then in the present patch you can extend the already existing SevEsIsEnabled()-dependent scope, with a call to InitializeCpuExceptionHandlers(). The end result would be something like: ProcessLibraryConstructorList(); // // Initialize IDT // ... // if (SevEsIsEnabled()) { // // non-automatic exit events (NAE) can occur during SEC ... // InitializeCpuExceptionHandlers (NULL); // // Under SEV-ES, the hypervisor can't modify CR0 ... // AsmEnableCache (); } What's your opinion? Thanks! Laszlo > @@ -751,19 +767,6 @@ SecCoreStartupWithStack ( > // > InitializeFloatingPointUnits (); > =20 > - // > - // Initialize IDT > - // > - IdtTableInStack.PeiService =3D NULL; > - for (Index =3D 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) { > - CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeo= f (mIdtEntryTemplate)); > - } > - > - IdtDescriptor.Base =3D (UINTN)&IdtTableInStack.IdtTable; > - IdtDescriptor.Limit =3D (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 >=20