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

[Jordan Justen <jordan.l.justen@intel.com>]

On 2017-05-26 07:43:48, Brijesh Singh wrote:
> 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.

I think you split IoMmuDxe out from AmdSevDxe based on my feedback
regarding APRIORI, but I don't think this helped.

Ideally I would like to see one driver named IoMmuDxe that is *not* in
APRIORI.

I think because of QemuFlashFvbServicesRuntimeDxe in APRIORI, we also
need IoMmuDxe in the APRIORI. Hopefully we can figure our how to
remove QemuFlashFvbServicesRuntimeDxe and then be able to remove
IoMmuDxe.

-Jordan

>  - 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
> 
> -- 
> 2.7.4
>