Re: [PATCH v6 00/17] x86: Secure Encrypted Virtualization (AMD)

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

Hi Jordan and Laszlo,

Ping.

It has been a while, Do you have any further feedbacks on this series ?
If you want then I can rebase the patches before you commit into upstream repos.

-Brijesh

On 05/26/2017 09:43 AM, Brijesh Singh wrote:
> The patch series provides support for AMD's new Secure Encrypted
> Virtualization (SEV) feature.
> 
> SEV is an extension to the AMD-V architecture which supports running
> multiple VMs under the control of a hypervisor. The SEV feature allows
> the memory contents of a virtual machine (VM) to be transparently encrypted
> with a key unique to the guest VM. The memory controller contains a
> high performance encryption engine which can be programmed with multiple
> keys for use by a different VMs in the system. The programming and
> management of these keys is handled by the AMD Secure Processor firmware
> which exposes a commands for these tasks.
> 
> SEV guest VMs have the concept of private and shared memory.  Private memory is
> encrypted with the guest-specific key, while shared memory may be encrypted
> with hypervisor key.  Certain types of memory (namely instruction pages and
> guest page tables) are always treated as private memory by the hardware.
> For data memory, SEV guest VMs can choose which pages they would like to be
> private. The choice is done using the standard CPU page tables using the C-bit,
> and is fully controlled by the guest. Due to security reasons all the DMA
> operations inside the  guest must be performed on shared pages (C-bit clear).
> Note that since C-bit is only controllable by the guest OS when it is operating
> in 64-bit or 32-bit PAE mode, in all other modes the SEV hardware forces the
> C-bit to a 1.
> 
> The following links provide additional details:
> 
> AMD Memory Encryption whitepaper:
> http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf
> 
> AMD64 Architecture Programmer's Manual:
>      http://support.amd.com/TechDocs/24593.pdf
>      SME is section 7.10
>      SEV is section 15.34
> 
> Secure Encrypted Virutualization Key Management:
> http://support.amd.com/TechDocs/55766_SEV-KM API_Specification.pdf
> 
> KVM Forum Presentation:
> http://www.linux-kvm.org/images/7/74/02x08A-Thomas_Lendacky-AMDs_Virtualizatoin_Memory_Encryption_Technology.pdf
> 
> [1] http://marc.info/?l=linux-mm&m=148846752931115&w=2
> 
> ---
> 
> Patch series is based on commit aff463c825a3
>   (Vlv2TbltDevicePkg/FvbRuntimeDxe: correct NumOfLba vararg type in EraseBlocks())
> 
> https://github.com/codomania/edk2/tree/v6
> 
> The patch series is tested with OvmfIa32.dsc, OvmfIa32X64.dsc and OvmfX64.dsc.
> Since memory encryption bit is not accessiable when processor is in 32-bit mode
> hence any DMA access in this mode would cause assert. I have also tested the
> suspend and resume path, it seems to be working fine. I still need to work to
> finish adding the SEV Dma support in QemuFwCfgS3Lib package (see TODO).
> 
> Changes since v5:
>   - add placeholder gIoMmuAbsentProtocolGuid
>   - add PlatformHasIoMmuLib
>   - fix indentation
> 
> Changes since v4:
>   - decouple IoMmu protocol implementation from AmdSevDxe into a seperate
>     IoMmuDxe driver. And introduce a placeholder protocol to provide the
>     dependency support for the dependent modules.
>   - update debug messages to use gEfiCallerBaseName where applicable.
>   - fix QemuFwCfgSecLib build errors and simplify SEV support
>   - update QemuFwCfgDxeLib to assert when failed to locate IOMMU
>   - update comments "host buffer" to " host buffer"
> 
> Changes since v3:
>   - update AmdSevDxe driver to produce IOMMU protocol
>   - remove BmDmaLib dependency
>   - update QemuFwCfgLib to use IOMMU protocol to allocate SEV DMA buffer
> 
> Changes since v2:
>   - move memory encryption CPUID and MSR definition into UefiCpuPkg
>   - fix the argument order for SUB instruction in ResetVector and add more
>     comments
>   - update PlatformPei to use BaseMemEncryptSevLib
>   - break the overlong comment lines to 79 chars
>   - variable aligment and other formating fixes
>   - split the SEV DMA support patch for QemuFwCfgLib into multiple patches as
>     recommended by Laszlo
>   - add AmdSevDxe driver which runs very early in DXE phase and clear the C-bit
>     from MMIO memory region
>   - drop 'QemuVideoDxe: Clear C-bit from framebuffer' patch since AmdSevDxe
>     driver takes care of clearing the C-bit from MMIO region
>   - Verified that Qemu PFLASH works fine with SEV guest, Found a KVM driver issue
>     which was causing #PF when PFLASH was enabled. I have submitted patch to
>     fix it in upstream http://marc.info/?l=kvm&m=149304930814202&w=2
> 
> Changes since v1:
>   - bug fixes in OvmfPkg/ResetVector (pointed by Tom Lendacky)
>   - add SEV CPUID and MSR register definition in standard include file
>   - remove the MemEncryptLib dependency from PlatformPei. Move AmdSevInitialize()
>     implementation in local file inside the PlatformPei package
>   - rename MemCryptSevLib to MemEncryptSevLib and add functions to set or
>     clear memory encryption attribute on memory region
>   - integerate SEV support in BmDmaLib
>   - split QemuFwCfgDxePei.c into QemuFwCfgDxe.c and QemuFwCfgPei.c to
>     allow building seperate QemuFwCfgLib for Dxe and Pei phase
>     (recommended by Laszlo Ersek)
>   - add SEV support in QemuFwCfgLib
>   - clear the memory encryption attribute from framebuffer memory region
> 
> 
> TODO:
> (Will add these features after basic SEV support patches are accepted in upstream)
>   - add support for DMA operation in QemuFwCfgS3Lib when SEV is enabled
>   - investigate SMM/SMI support
> 
> Cc: Jeff Fan <jeff.fan@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Leo Duran <leo.duran@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Leo Duran <leo.duran@amd.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <Thomas.Lendacky@amd.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> 
> Brijesh Singh (17):
>    UefiCpuPkg: Define AMD Memory Encryption specific CPUID and MSR
>    OvmfPkg/ResetVector: Set C-bit when building initial page table
>    OvmfPkg: Update dsc to use IoLib from BaseIoLibIntrinsicSev.inf
>    OvmfPkg/BaseMemcryptSevLib: Add SEV helper library
>    OvmfPkg/PlatformPei: Set memory encryption PCD when SEV is enabled
>    OvmfPkg: Add AmdSevDxe driver
>    OvmfPkg: Introduce IoMmuAbsent Protocol GUID
>    OvmfPkg: Add PlatformHasIoMmuLib
>    OvmfPkg: Add IoMmuDxe driver
>    OvmfPkg/QemuFwCfgLib: Provide Pei and Dxe specific library
>    OvmfPkg/QemuFwCfgLib: Prepare for SEV support
>    OvmfPkg/QemuFwCfgLib: Implement SEV internal function for SEC phase
>    OvmfPkg/QemuFwCfgLib: Implement SEV internal functions for PEI phase
>    OvmfPkg/QemuFwCfgLib: Implement SEV internal function for Dxe phase
>    OvmfPkg/QemuFwCfgLib: Add option to dynamic alloc FW_CFG_DMA Access
>    OvmfPkg/QemuFwCfgLib: Add SEV support
>    OvmfPkg: update PciHostBridgeDxe to use PlatformHasIoMmuLib
> 
>   OvmfPkg/OvmfPkg.dec                                                    |   1 +
>   OvmfPkg/OvmfPkgIa32.dsc                                                |  11 +-
>   OvmfPkg/OvmfPkgIa32X64.dsc                                             |  12 +-
>   OvmfPkg/OvmfPkgX64.dsc                                                 |  12 +-
>   OvmfPkg/OvmfPkgIa32.fdf                                                |   1 +
>   OvmfPkg/OvmfPkgIa32X64.fdf                                             |   3 +
>   OvmfPkg/OvmfPkgX64.fdf                                                 |   3 +
>   OvmfPkg/AmdSevDxe/AmdSevDxe.inf                                        |  43 ++
>   OvmfPkg/IoMmuDxe/IoMmuDxe.inf                                          |  49 +++
>   OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf          |  50 +++
>   OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf            |  37 ++
>   OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgLib.inf => QemuFwCfgDxeLib.inf} |  15 +-
>   OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgLib.inf => QemuFwCfgPeiLib.inf} |   9 +-
>   OvmfPkg/PlatformPei/PlatformPei.inf                                    |   3 +
>   OvmfPkg/Include/Library/MemEncryptSevLib.h                             |  81 ++++
>   OvmfPkg/IoMmuDxe/AmdSevIoMmu.h                                         |  43 ++
>   OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h               | 184 ++++++++
>   OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibInternal.h                    |  37 ++
>   OvmfPkg/PlatformPei/Platform.h                                         |   5 +
>   UefiCpuPkg/Include/Register/Amd/Cpuid.h                                | 162 +++++++
>   UefiCpuPkg/Include/Register/Amd/Fam17Msr.h                             |  62 +++
>   UefiCpuPkg/Include/Register/Amd/Msr.h                                  |  29 ++
>   OvmfPkg/AmdSevDxe/AmdSevDxe.c                                          |  75 ++++
>   OvmfPkg/IoMmuDxe/AmdSevIoMmu.c                                         | 459 ++++++++++++++++++++
>   OvmfPkg/IoMmuDxe/IoMmuDxe.c                                            |  53 +++
>   OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c           |  84 ++++
>   OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLibInternal.c        |  90 ++++
>   OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c            |  84 ++++
>   OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c               | 439 +++++++++++++++++++
>   OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.c              |  32 ++
>   OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c                            | 230 ++++++++++
>   OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c                            |  67 ++-
>   OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgPeiDxe.c => QemuFwCfgPei.c}     |  72 ++-
>   OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgSec.c                            |  57 +++
>   OvmfPkg/PlatformPei/AmdSev.c                                           |  62 +++
>   OvmfPkg/PlatformPei/Platform.c                                         |   1 +
>   OvmfPkg/ResetVector/Ia32/PageTables64.asm                              |  70 ++-
>   37 files changed, 2703 insertions(+), 24 deletions(-)
>   create mode 100644 OvmfPkg/AmdSevDxe/AmdSevDxe.inf
>   create mode 100644 OvmfPkg/IoMmuDxe/IoMmuDxe.inf
>   create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevLib.inf
>   create mode 100644 OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
>   copy OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgLib.inf => QemuFwCfgDxeLib.inf} (71%)
>   rename OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgLib.inf => QemuFwCfgPeiLib.inf} (80%)
>   create mode 100644 OvmfPkg/Include/Library/MemEncryptSevLib.h
>   create mode 100644 OvmfPkg/IoMmuDxe/AmdSevIoMmu.h
>   create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
>   create mode 100644 UefiCpuPkg/Include/Register/Amd/Cpuid.h
>   create mode 100644 UefiCpuPkg/Include/Register/Amd/Fam17Msr.h
>   create mode 100644 UefiCpuPkg/Include/Register/Amd/Msr.h
>   create mode 100644 OvmfPkg/AmdSevDxe/AmdSevDxe.c
>   create mode 100644 OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
>   create mode 100644 OvmfPkg/IoMmuDxe/IoMmuDxe.c
>   create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSevLib.c
>   create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLibInternal.c
>   create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevLib.c
>   create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.c
>   create mode 100644 OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.c
>   create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c
>   rename OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgPeiDxe.c => QemuFwCfgPei.c} (61%)
>   create mode 100644 OvmfPkg/PlatformPei/AmdSev.c
>