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

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 8/22/19 9:12 AM, Laszlo Ersek wrote:
> On 08/21/19 23:42, Lendacky, Thomas wrote:
>> On 8/21/19 9:31 AM, Laszlo Ersek wrote:
>>> On 08/19/19 23:35, Lendacky, Thomas wrote:
>>>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>>>
>>>> Allocate memory for the GHCB pages during SEV initialization for use
>>>> during Pei and Dxe phases. Since the GHCB pages must be mapped as shared
>>>> pages, modify CreateIdentityMappingPageTables() so that pagetable entries
>>>> are created without the encryption bit set.
>>>>
>>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>>> ---
>>>>  UefiCpuPkg/UefiCpuPkg.dec                     |  4 ++
>>>>  OvmfPkg/OvmfPkgX64.dsc                        |  4 ++
>>>>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf       |  3 +
>>>>  OvmfPkg/PlatformPei/PlatformPei.inf           |  2 +
>>>>  .../Core/DxeIplPeim/X64/VirtualMemory.h       | 12 +++-
>>>>  .../Core/DxeIplPeim/Ia32/DxeLoadFunc.c        |  4 +-
>>>>  .../Core/DxeIplPeim/X64/DxeLoadFunc.c         | 11 +++-
>>>>  .../Core/DxeIplPeim/X64/VirtualMemory.c       | 49 ++++++++++----
>>>>  .../MemEncryptSevLibInternal.c                |  1 -
>>>>  .../BaseMemEncryptSevLib/X64/VirtualMemory.c  | 33 ++++++++--
>>>>  OvmfPkg/PlatformPei/AmdSev.c                  | 64 +++++++++++++++++++
>>>>  11 files changed, 164 insertions(+), 23 deletions(-)
>>>
>>> Should be split to at least four patches (UefiCpuPkg, MdeModulePkg,
>>> OvmfPkg/BaseMemEncryptSevLib, OvmfPkg/PlatformPei).
>>>
>>> In addition, MdeModulePkg content must not depend on UefiCpuPkg content
>>> -- if modules under both packages need to consume a new PCD, then the
>>> PCD should be declared under MdeModulePkg. The rough dependency order is:
>>>
>>> - MdePkg (must be self-contained)
>>> - MdeModulePkg (may consume MdePkg)
>>> - UefiCpuPkg (may consume everything above, to my knowledge)
>>> - OvmfPkg (may consume everything above)
>>>
>>
>> Ok, thanks for the guidance.
>>
>> Ideally, I just would like to modify the newly created page tables after
>> the call to CreateIdentityMappingPageTables() in MdeModulePkg/Core/
>> DxeIplPeim/Ia32/DxeLoadFunc.c. Is there a preferred way to add a listener
>> or callback or notification service so that the main changes would be
>> limited to the OvmfPkg files and would that be acceptable?
> 
> * https://bugzilla.tianocore.org/show_bug.cgi?id=623
> 
>   Reported on 2017-07-07, resolved as WONTFIX on 2019-07-30 ("no
>   resources").
> 
>   And it's not like patches had not been proposed -- Leo had implemented
>   a notification service --; they were rejected.
> 
> * https://bugzilla.tianocore.org/show_bug.cgi?id=847
> 
>   Reported on 2018-01-11, marked "not high priority" as of 2019-07-23
>   <https://www.mail-archive.com/devel@edk2.groups.io/msg05507.html>.
> 
> I don't know what to tell you. While nobody seems to disagree with the
> necessity of such a service and/or library, core maintainers have
> rejected all the code proposals thus far (= "don't do that"). And I'm
> unaware of any constructive guidance (= "do this instead").

This isn't on the level of a "notify every time something changes" type
of thing. This is more of a "hey, we built some new pagetables and are
about to make them active, but before we do have a look and change what
you think should be changed."

With that, I'd be able to remove the GhcbBase and GhcbSize that is
propogated on the ToSplit and Split functions.

I'll take a look and see what it would look like and go from there.

> 
> I suggest filing a Feature Request BZ for SEV-ES enablement (for
> OvmfPkg), and referencing that as "dependent bug" in both of the
> above-mentioned BZs. It might also help to dial in to the APAC/NAMO
> design / bug triage meeting, and campaign for the feature there.

Yes, I need to file that Feature Request BZ anyway.

Thanks,
Tom

> 
> https://github.com/tianocore/tianocore.github.io/wiki/Bug-Triage
> 
> I have a bad track record at convincing core maintainers to do what they
> don't want to do. And I see escalating such problems from email to phone
> as a work-around, sort of "wear down your opponent by sheer
> persistence". So I avoid that. But, I've seen the approach work for
> others, so you might have better luck.
> 
> (The APAC/NAMO call is also at a bad time for me, in UTC+1 / UTC+2.)
> 
> I think the present RFC patches are a good way to re-raise these topics.
> 
> Laszlo
>