Re: [edk2-devel] [PATCH v3 0/3] SEV Page Encryption Bitmap support for OVMF.

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

On 12/04/20 09:10, Ashish Kalra wrote:
> On Fri, Dec 04, 2020 at 04:50:05AM +0100, Laszlo Ersek wrote:
>> On 12/04/20 01:03, Ashish Kalra wrote:
>>> From: Ashish Kalra <ashish.kalra@amd.com>
>>>
>>> By default all the SEV guest memory regions are considered encrypted,
>>> if a guest changes the encryption attribute of the page (e.g mark a
>>> page as decrypted) then notify hypervisor. Hypervisor will need to
>>> track the unencrypted pages. The information will be used during
>>> guest live migration, guest page migration and guest debugging.
>>>
>>> The patch-set also adds a new SEV and SEV-ES hypercall abstraction
>>> library to support SEV Page encryption/decryption status hypercalls
>>> for SEV and SEV-ES guests.
>>>
>>> BaseMemEncryptSevLib invokes hypercalls via this new hypercall library.
>>>
>>> A branch containing these patches is available here:
>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fashkalra%2Fedk2%2Ftree%2Fsev_page_encryption_bitmap_v3&amp;data=04%7C01%7Cashish.kalra%40amd.com%7Cbc3c88f21f1d40b322b408d89807b5c8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637426506192800828%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=VZzP2MVJSECgMhOyuCCASw58g74BiCVAH9JW8hZG3Tw%3D&amp;reserved=0
>>>
>>> Changes since v2:
>>>  - GHCB_BASE setup during reset-vector as decrypted is marked explicitly
>>>    in the hypervisor page encryption bitmap after setting the 
>>>    PcdSevEsIsEnabled PCD.
>>>
>>> Changes since v1:
>>>  - Mark GHCB_BASE setup during reset-vector as decrypted explicitly in
>>>    the hypervisor page encryption bitmap.
>>>  - Resending the series with correct shallow threading.
>>>
>>> Ashish Kalra (2):
>>>   OvmfPkg/MemEncryptHypercallLib: add library to support SEV hypercalls.
>>>   OvmfPkg/PlatformPei: Mark SEC GHCB page in the page encrpytion bitmap.
>>>
>>> Brijesh Singh (1):
>>>   OvmfPkg/BaseMemEncryptLib: Support to issue unencrypted hypercall
>>>
>>>  .../Include/Library/MemEncryptHypercallLib.h  |  37 ++++++
>>>  .../BaseMemEncryptSevLib.inf                  |   1 +
>>>  .../BaseMemEncryptSevLib/X64/VirtualMemory.c  |  18 +++
>>>  .../MemEncryptHypercallLib.c                  | 105 ++++++++++++++++++
>>>  .../MemEncryptHypercallLib.inf                |  39 +++++++
>>>  .../X64/AsmHelperStub.nasm                    |  39 +++++++
>>>  OvmfPkg/OvmfPkgX64.dsc                        |   1 +
>>>  OvmfPkg/PlatformPei/AmdSev.c                  |  10 ++
>>>  8 files changed, 250 insertions(+)
>>>  create mode 100644 OvmfPkg/Include/Library/MemEncryptHypercallLib.h
>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.c
>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/MemEncryptHypercallLib.inf
>>>  create mode 100644 OvmfPkg/Library/MemEncryptHypercallLib/X64/AsmHelperStub.nasm
>>>
>>
>> I'll need some time to get to this series.
>>
>> I'm fairly certain though, from a quick skim, that this series breaks
>> all DSC files under OvmfPkg except X64. Please fix that.
>>
>>
> 
> Ok thanks Laszlo, i will fix this.

Thanks.

I can see a new comment for the series from Dov Murik, and I think
that's awesome. I'd welcome if there were lively exchanges around OVMF
patch sets. I'm selfish of course: I'd like to delegate reviews.

So, on this patch set, I notice it does not add the new
(MemEncryptHypercallLib-related) files to Maintainers.txt, namely
section "OvmfPkg: SEV-related modules".

Please include such a patch in v4 -- if Tom and Brijesh agree, I'd like
to put the new lib explicitly under their reviewership.

Also, I plan to review this series (v4, at this point) only for
formalities. I'd like to receive an R-b from Tom or Brijesh [*], and
another from Dov or a colleague at IBM, for this series; those together
should suffice for merging the library.

[*] Brijesh seems to be the original author of patch#2, so maybe Tom is
a better-poised reviewer for this.

Thanks
Laszlo