From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 30 Sep 2019 12:13:02 -0700 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 mx1.redhat.com (Postfix) with ESMTPS id 6C19D10DCC96; Mon, 30 Sep 2019 19:13:01 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-111.rdu2.redhat.com [10.10.121.111]) by smtp.corp.redhat.com (Postfix) with ESMTP id 362241001B08; Mon, 30 Sep 2019 19:12:59 +0000 (UTC) Subject: Re: [edk2-devel] [RFC PATCH v2 08/44] OvmfPkg: Create GHCB pages for use during Pei and Dxe phase To: "Lendacky, Thomas" , "devel@edk2.groups.io" Cc: Jordan Justen , Ard Biesheuvel , Michael D Kinney , Liming Gao , Eric Dong , Ray Ni , "Singh, Brijesh" References: <9799d415f652618c8a960cdb0040918185588652.1568922728.git.thomas.lendacky@amd.com> <8779b242-a38c-3bf7-ce85-469197fee287@amd.com> From: "Laszlo Ersek" Message-ID: <4c42d99e-dff5-2b34-2e6f-25b3c6f0cef7@redhat.com> Date: Mon, 30 Sep 2019 21:12:58 +0200 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: <8779b242-a38c-3bf7-ce85-469197fee287@amd.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.6.2 (mx1.redhat.com [10.5.110.64]); Mon, 30 Sep 2019 19:13:01 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/26/19 16:00, Lendacky, Thomas wrote: > On 9/26/19 3:00 AM, Laszlo Ersek wrote: >> Hi Tom, >> >> On 09/19/19 21:52, Lendacky, Thomas wrote: >>> From: Tom Lendacky >>> >>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198 >>> >>> Allocate memory for the GHCB pages during SEV initialization for use >>> during Pei and Dxe phases. The GHCB page(s) must be shared pages, so >>> clear the encryption mask from the current page table entries. Upon >>> successful allocation, set the GHCB PCDs (PcdGhcbBase and PcdGhcbSize). >> >> skimming this patch and the next two ones for OvmfPkg (#10, #11), I'm a >> bit lost. I'm missing a parallel between the "early X64 page tables" and >> the GHCB-related pages. >> >> The former are set up (in X64 OVMF) in SEC, and are used throughout PEI >> until the DXE IPL builds new ones for the DXE phase. The latter also >> *seemed* to be set up in SEC, and I thought they'd be used throughout >> PEI -- I assumed the next place we'd need to massage GHCB pages would be >> similarly in the DXE IPL, or thereabouts. >> >> However, in this patch, we seem to allocate new pages for GHCB, and the >> commit message implies they are supposed to be used during PEI. That >> diverges from how long the "early X64 page tables" are used. > > At this stage, we need a GHCB page for every (v)CPU. So a new allocation > is done and then the pages are marked unencrypted. Once the new GHCB > pages are allocated, the original GHCB page for SEC is no longer needed > because the new allocation replaces it in the BSP. But the early page > table is still required in order to access all of the memory from the 2MB > range (0x800000 to 0x9fffff). > >> >> I guess this difference could be justified, especially because we do MP >> stuff in PEI. (And we need separate GHCB stuff per VCPU -- in SEC we >> only consider the BSP.) >> >> But then, the question becomes: what exactly do we need the GHCB page >> allocated in SEC for? From the blurb, it seems that the GHCB allows the > > There are lots of different ways to cause a #VC. A #VC is generated for > debug statements that use port I/O, MMIO, intercept-able MSR accesses, > CPUID instructions, WBINVD instructions, etc. Many of these things happen > during SEC. With the debug serial output enabled, over 8,000 #VC > exceptions occur before allocating the new GHCB pages in > AmdSevEsInitialize(). > >> guest to selectively (actively) share information with the hypervisor -- >> such as (parts of?) the register file, which the hypervisor cannot >> directly access, for a SEV-ES guest. >> >> But, we never seem to place such information at PcdOvmfSecGhcbBase (aka >> GHCB_BASE) in SEC. We program the GHCB's base address, and then we clear >> the GHCB, but that seems to be it. Do we write anything non-zero to that >> block, ever? > > Yes, that happens in the SEC exception handler. When the #VC occurs, the > GHCB information is filled in and a VMGEXIT instruction is issued to exit > to the hypervisor. The hypervisor then accesses the GHCB in order to > perform the requested function. (Sorry about sending this in a separate email.) So... Where is the #VC handler that implements this logic in SEC? ... Ah wait, now I understand why I got confused. The patch series thus far has modified SEC code that is specific to OVMF, and then it advances to OvmfPkg/PlatformPei. I thought we were done with SEC changes in OVMF, and I didn't understand where the #VC handler you refer to above was. But, looking a bit ahead in the series, I see the exception handler being built gradually, in UefiCpuPkg, and (I guess) also enabled in OVMF's SEC somewhere / sometime. I wonder what the best ordering would be, for the patches in the series. The middle seems to be alternating between UefiCpuPkg and OvmfPkg. That's quite unusual. I don't have a clear understanding of the feature yet, so I can't authoritatively suggest a "better" structure. As a first guess, I would suggest constructing the building blocks in MdePkg and UefiCpuPkg (libraries, primarily), then utilizing them in MdeModulePkg (core drivers), then adding platform code in OvmfPkg. Also, I would suggest copious cross-references between the patches (identified by subjects). I hope this is not too annoying, just trying to ask for crutches :) I'll try to continue the review of the series this week. Thanks! Laszlo