Re: [PATCH 00/20] OvmfPkg: SEV: decrypt the initial SMRAM save state map for SMBASE relocation

[Brijesh Singh <brijesh.singh@amd.com>]

On 3/1/18 6:03 PM, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: sev_smram_save_state_map
>
> This patch series replaces the following patch from Brijesh:
>
>   [PATCH v2 1/2] OvmfPkg/AmdSevDxe: Clear the C-bit from SMM Saved State
>   http://mid.mail-archive.com/20180228161415.28723-2-brijesh.singh@amd.com
>   https://lists.01.org/pipermail/edk2-devel/2018-February/022016.html
>
> The series modifies four modules under OvmfPkg:
> - MemEncryptSevLib
> - PlatformPei
> - SmmCpuFeaturesLib
> - AmdSevDxe
>
> Patches 01 through 16 are cleanups for these modules, without change in
> functionality. The series is formatted with 20 lines of context, for
> simplifying the review of these patches.
>
> Patches 17 through 20 (one patch per module listed above) implement
> $SUBJECT:
>
> - MemEncryptSevLib gets a new helper function used by the other three
>   modules.
>
> - PlatformPei makes sure that DXE stays out of the page(s) where the
>   initial SMRAM save state map lives.
>
> - SmmCpuFeaturesLib and AmdSevDxe basically do what they did in
>   Brijesh's patch, just better separated, and with minor tweaks.
>
> The series is bisectable.
>
> I regression-tested my usual non-SEV guests (with Brijesh's v2 2/2 patch
> applied on top, from the above-referenced series), which covers i440fx
> (no SMM, X64), q35 (SMM, IA32 and IA32X64), Fedora and Windows, normal
> boot and S3.
>
> I also tried to test the series with SEV guests (again with Brijesh's v2
> 2/2 patch applied on top). Unfortunately, I didn't get good results with
> or without SMM. Without SMM, the guest OS boots to a point, but then it
> gets stuck with the CPU spinning. With SMM, OVMF gets stuck in SMBASE
> relocation.

To boot the SEV guest with SMM support we need this KVM patch, without
this we will get either #UD or some undefined behavior.

https://git.kernel.org/pub/scm/virt/kvm/kvm.git/commit/?id=7607b7174405aec7441ff6c970833c463114040a

It's strange that you are having trouble booting SEV guest without SMM
support. It's possible that we might have some mismatch kernel kvm +
qemu + ovmf patches.
>
> I should mention however that my SEV host setup dates back to November
> 2017, including host kernel, QEMU and guest OS. I suppose all those
> components have seen many changes since, on the respective upstream
> lists. I'll have to work with Brijesh to update my SEV host to the
> latest bits.
>
> Until then, Brijesh, can you please test this series? Thank you!


Sure, I will try the series tomorrow morning. thank you so much for the
cleanup and remaining SMM work.


>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
>
> Cheers
> Laszlo
>
> Laszlo Ersek (20):
>   OvmfPkg/MemEncryptSevLib: rewrap to 79 characters width
>   OvmfPkg/MemEncryptSevLib: clean up MemEncryptSevIsEnabled() decl
>   OvmfPkg/MemEncryptSevLib: clean up MemEncryptSevClearPageEncMask()
>     decl
>   OvmfPkg/MemEncryptSevLib: clean up MemEncryptSevSetPageEncMask() decl
>   OvmfPkg/MemEncryptSevLib: clean up SetMemoryEncDec() comment block
>   OvmfPkg/MemEncryptSevLib: clean up
>     InternalMemEncryptSevSetMemoryDecrypted() decl
>   OvmfPkg/MemEncryptSevLib: clean up
>     InternalMemEncryptSevSetMemoryEncrypted() decl
>   OvmfPkg/MemEncryptSevLib: sort #includes, and entries in INF file
>     sections
>   OvmfPkg/PlatformPei: sort #includes in "AmdSev.c"
>   OvmfPkg/SmmCpuFeaturesLib: rewrap to 79 columns
>   OvmfPkg/SmmCpuFeaturesLib: upper-case the "static" keyword
>   OvmfPkg/SmmCpuFeaturesLib: sort #includes, and entries in INF file
>     sections
>   OvmfPkg/SmmCpuFeaturesLib: remove unneeded #includes and
>     LibraryClasses
>   OvmfPkg/AmdSevDxe: rewrap to 79 characters width
>   OvmfPkg/AmdSevDxe: sort #includes, and entries in INF file sections
>   OvmfPkg/AmdSevDxe: refresh #includes and LibraryClasses
>   OvmfPkg/MemEncryptSevLib: find pages of initial SMRAM save state map
>   OvmfPkg/PlatformPei: SEV: allocate pages of initial SMRAM save state
>     map
>   OvmfPkg/SmmCpuFeaturesLib: SEV: encrypt+free pages of init. save state
>     map
>   OvmfPkg/AmdSevDxe: decrypt the pages of the initial SMRAM save state
>     map
>
>  OvmfPkg/AmdSevDxe/AmdSevDxe.c                                   |  88 ++-
>  OvmfPkg/AmdSevDxe/AmdSevDxe.inf                                 |  25 +-
>  OvmfPkg/Include/Library/MemEncryptSevLib.h                      |  89 ++-
>  OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf   |  17 +-
>  OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c    |  72 ++-
>  OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLibInternal.c |  66 +-
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c     |  74 ++-
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c        | 172 ++++--
>  OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h        | 137 +++--
>  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c           | 650 ++++++++++++++++----
>  OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf         |  13 +-
>  OvmfPkg/PlatformPei/AmdSev.c                                    |  36 +-
>  12 files changed, 1067 insertions(+), 372 deletions(-)
>