On 9/30/19 2:12 PM, Laszlo Ersek wrote: > 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 <thomas.lenda...@amd.com> >>>> >>>> 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 :)
No worries, I'll see what I can do in ordering the patches so they make more sense to a reviewer looking at the code for the first time. Thanks, Tom > > I'll try to continue the review of the series this week. > > Thanks! > Laszlo > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#48295): https://edk2.groups.io/g/devel/message/48295 Mute This Topic: https://groups.io/mt/34203543/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
