From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web12.4466.1574457052238156207 for ; Fri, 22 Nov 2019 13:10:52 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=gJq2H8vK; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574457051; 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=Fif5xxiRd6bKoCfjR1QZejA3ORDomYe0AI5EKwsGMpU=; b=gJq2H8vKNis7TpJtCahJaqjWaUQRl0uqEYUz7MA+Xs5gqSIYLLPwupB4OI1o/UggJ2W4HU 1UddLT1SiilOtfDC4OJ4NgaIjKxe2t3I7sWtTwKMYY/WSHNrNTUQ35M3tj1VkX1V1G/bbR h7qiv5vtCGDDeBdzpqXPiO/UMHTWG74= 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-12-y2FzBhcqOK2nEL83ZFdQ2A-1; Fri, 22 Nov 2019 16:10:49 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 2D3E7800A02; Fri, 22 Nov 2019 21:10:48 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (unknown [10.36.118.23]) by smtp.corp.redhat.com (Postfix) with ESMTP id EAAE25D6A3; Fri, 22 Nov 2019 21:10:45 +0000 (UTC) Subject: Re: [edk2-devel] [RFC PATCH v3 30/43] OvmfPkg/Sec: Add #VC exception handling for Sec phase To: Tom Lendacky , devel@edk2.groups.io Cc: Jordan Justen , Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , Brijesh Singh References: <041cb6f6352c9e0f9982d316cd842c2daf57e7fa.1574280425.git.thomas.lendacky@amd.com> <12e35ff5-ff14-87d4-f4ac-604448289688@redhat.com> From: "Laszlo Ersek" Message-ID: <268877ff-8593-845c-e6e6-fe763557c623@redhat.com> Date: Fri, 22 Nov 2019 22:10:44 +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: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-MC-Unique: y2FzBhcqOK2nEL83ZFdQ2A-1 X-Mimecast-Spam-Score: 0 Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable On 11/22/19 17:30, Tom Lendacky wrote: > On 11/22/19 6:52 AM, Laszlo Ersek wrote: >> On 11/21/19 21:46, Tom Lendacky wrote: >>> On 11/21/19 6:06 AM, Laszlo Ersek wrote: >>>> On 11/20/19 21:06, Lendacky, Thomas wrote: >>>>> @@ -737,6 +738,21 @@ SecCoreStartupWithStack ( >>>>> =A0=A0=A0=A0=A0 Table[Index] =3D 0; >>>>> =A0=A0=A0 } >>>>> >>>>> +=A0 // >>>>> +=A0 // Initialize IDT >>>>> +=A0 // >>>>> +=A0 IdtTableInStack.PeiService =3D NULL; >>>>> +=A0 for (Index =3D 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) { >>>>> +=A0=A0=A0 CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTempl= ate, >>>>> sizeof (mIdtEntryTemplate)); >>>>> +=A0 } >>>>> + >>>>> +=A0 IdtDescriptor.Base=A0 =3D (UINTN)&IdtTableInStack.IdtTable; >>>>> +=A0 IdtDescriptor.Limit =3D (UINT16)(sizeof (IdtTableInStack.IdtTabl= e) >>>>> - 1); >>>>> + >>>>> +=A0 AsmWriteIdtr (&IdtDescriptor); >>>>> + >>>>> +=A0 InitializeCpuExceptionHandlers (NULL); >>>>> + >>>>> =A0=A0=A0 ProcessLibraryConstructorList (NULL, NULL); >>>>> >>>>> =A0=A0=A0 DEBUG ((EFI_D_INFO, >>>> >>>> (1) The problem here is that we call multiple library APIs before >>>> calling ProcessLibraryConstructorList() -- namely CopyMem(), >>>> AsmWriteIdtr(), and InitializeCpuExceptionHandlers(). >=20 > We can reduce this exposure a bit and replace the CopyMem() call with > something similar to the loop above it. That would be nice, if you're not too annoyed by the extra busywork. > I could also use assembler code directly in here to load the IDTR. I think it would be enough to copy MdePkg/Library/BaseLib/Ia32/WriteIdtr.nasm MdePkg/Library/BaseLib/X64/WriteIdtr.nasm under OvmfPkg/Sec, and use a new function name. > That would leave just InitializeCpuExceptionHandlers(). Is there > something that can added so as to warn when a library has a > CONSTRUCTOR added to/part of the definition? Nothing comes to my mind :( [...] >> (1) Append a UINT8 ("BOOLEAN") field to the SEV_ES_AP_JMP_FAR structure >> (and possibly rename the structure), >> >> (2) in the OvmfPkgX64 reset vector, where you determine SEV-ES anyway, >> explicitly set this field to zero if SEV-ES is disabled, and set it to >> one, if SEV-ES is enabled, >> >> (3) In OvmfPkg/Sec, introduce a new (local) header file declaring the >> function SevEsIsEnabled(), >> >> (4) Provide two C-language implementations (under the Ia32 and X64 >> directories): in the 32-bit version, return constant FALSE; in the >> 64-bit version, return the value of the new field. Something like: >> >> =A0=A0 return ((SEV_ES_AP_JMP_FAR *)FixedPcdGet32 >> (PcdSevEsResetRipBase))->SevEsEnabled; >> >> FixedPcdGet32() is explicitly safe to use without library constructors >> having run. >> >> Does this look viable? (It might require you to reshuffle patch 37 vs. >> patch 30.) >=20 > I think this does. Since this is SEC and the reset vector page isn't > needed until PEI and later we could even just use the first byte (make a > union with an SEC usage field) and make this even simpler. Then we don't > have to worry about positioning it. Ah, nice. > Let me work on that and see where I > get. Anything after the #VC is established would use the current method > of determine SEV-ES status. Thanks! Laszlo