From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id BD23021A08D82 for ; Fri, 26 May 2017 14:05:41 -0700 (PDT) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 26 May 2017 14:05:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,399,1491289200"; d="scan'208";a="91854758" Received: from mvakil-mobl1.amr.corp.intel.com (HELO localhost) ([10.254.191.209]) by orsmga004.jf.intel.com with ESMTP; 26 May 2017 14:05:40 -0700 MIME-Version: 1.0 To: Brijesh Singh , edk2-devel@lists.01.org Message-ID: <149583274037.25973.13062338567511386932@jljusten-skl> From: Jordan Justen In-Reply-To: <1495809845-32472-1-git-send-email-brijesh.singh@amd.com> Cc: Thomas.Lendacky@amd.com, leo.duran@amd.com, Brijesh Singh , Jeff Fan , Liming Gao , Laszlo Ersek , Jiewen Yao References: <1495809845-32472-1-git-send-email-brijesh.singh@amd.com> User-Agent: alot/0.5.1 Date: Fri, 26 May 2017 14:05:40 -0700 Subject: Re: [PATCH v6 00/17] x86: Secure Encrypted Virtualization (AMD) X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 May 2017 21:05:42 -0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable 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 drive= r issue > which was causing #PF when PFLASH was enabled. I have submitted patch = to > fix it in upstream http://marc.info/?l=3Dkvm&m=3D149304930814202&w=3D2 > = > 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 AmdSevIniti= alize() > 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 > Cc: Liming Gao > Cc: Leo Duran > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Leo Duran > Cc: Jiewen Yao > Cc: Tom Lendacky > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Brijesh Singh > = > 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 =3D> QemuFwCfgDxeLib.inf}= | 15 +- > OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgLib.inf =3D> 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 =3D> 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/BaseMemEncryptSe= vLib.inf > create mode 100644 OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuL= ib.inf > copy OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgLib.inf =3D> QemuFwCfgDxeLib= .inf} (71%) > rename OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgLib.inf =3D> QemuFwCfgPeiL= ib.inf} (80%) > create mode 100644 OvmfPkg/Include/Library/MemEncryptSevLib.h > create mode 100644 OvmfPkg/IoMmuDxe/AmdSevIoMmu.h > create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemor= y.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/MemEncryptS= evLib.c > create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLib= Internal.c > create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSe= vLib.c > create mode 100644 OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemor= y.c > create mode 100644 OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuL= ib.c > create mode 100644 OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgDxe.c > rename OvmfPkg/Library/QemuFwCfgLib/{QemuFwCfgPeiDxe.c =3D> QemuFwCfgPei= .c} (61%) > create mode 100644 OvmfPkg/PlatformPei/AmdSev.c > = > -- = > 2.7.4 >=20