Re: [edk2-devel] [RFC PATCH v2 08/44] OvmfPkg: Create GHCB pages for use during Pei and Dxe phase

["Laszlo Ersek" <lersek@redhat.com>]

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.lendacky@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 :)

I'll try to continue the review of the series this week.

Thanks!
Laszlo