[edk2-devel] [PATCH v14 00/46] SEV-ES guest support
Liming Gao
liming.gao at intel.com
Tue Aug 11 01:12:48 UTC 2020
Tom:
I run ECC plugin (https://edk2.groups.io/g/devel/message/63271) in my local machine. It reports below issues. Can you help update the patches to fix them?
I use the standalone EccCheck from https://github.com/shenglei10/edk2/tree/ecc_script.
EFI coding style error
*Error code: 8005
*Variable name does not follow the rules: 1. First character should be upper case 2. Must contain lower case characters 3. No white space characters 4. Global variable name must start with a 'g'
*file: D:\AllPkg\edk2\OvmfPkg\Sec\SecMain.c
*Line number: 867
*The variable name [*Ds] does not follow the rules
EFI coding style error
*Error code: 9003
*The first line of text in a comment block should be a brief description of the element being documented and the brief description must end with a period.
*file: D:\AllPkg\edk2\OvmfPkg\Library\BaseMemEncryptSevLib\MemEncryptSevLibInternal.c
*Line number: 72
*Comment description should end with period '.'
EFI coding style error
*Error code: 3002
*Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=)
*file: D:\AllPkg\edk2\OvmfPkg\Library\VmgExitLib\VmgExitVcHandler.c
*Line number: 1280
*Predicate Expression: OpCount
EFI coding style error
*Error code: 5007
*There should be no initialization of a variable as part of its declaration
*file: D:\AllPkg\edk2\OvmfPkg\Library\VmgExitLib\VmgExitVcHandler.c
*Line number: 845
*Variable Name: Data
EFI coding style error
*Error code: 5007
*There should be no initialization of a variable as part of its declaration
*file: D:\AllPkg\edk2\OvmfPkg\Library\VmgExitLib\VmgExitVcHandler.c
*Line number: 849
*Variable Name: Data
Thanks
Liming
-----Original Message-----
From: Laszlo Ersek <lersek at redhat.com>
Sent: 2020年8月11日 3:36
To: devel at edk2.groups.io; thomas.lendacky at amd.com
Cc: Brijesh Singh <brijesh.singh at amd.com>; Ard Biesheuvel <ard.biesheuvel at arm.com>; Dong, Eric <eric.dong at intel.com>; Justen, Jordan L <jordan.l.justen at intel.com>; Gao, Liming <liming.gao at intel.com>; Kinney, Michael D <michael.d.kinney at intel.com>; Ni, Ray <ray.ni at intel.com>; Andrew Fish <afish at apple.com>; Anthony Perard <anthony.perard at citrix.com>; You, Benjamin <benjamin.you at intel.com>; Bi, Dandan <dandan.bi at intel.com>; Dong, Guo <guo.dong at intel.com>; Wu, Hao A <hao.a.wu at intel.com>; Wang, Jian J <jian.j.wang at intel.com>; Julien Grall <julien at xen.org>; Leif Lindholm <leif at nuviainc.com>; Ma, Maurice <maurice.ma at intel.com>
Subject: Re: [edk2-devel] [PATCH v14 00/46] SEV-ES guest support
On 08/07/20 21:38, Lendacky, Thomas wrote:
> From: Tom Lendacky <thomas.lendacky at amd.com>
>
> This patch series provides support for running EDK2/OVMF under SEV-ES.
>
> Secure Encrypted Virtualization - Encrypted State (SEV-ES) expands on
> the SEV support to protect the guest register state from the
> hypervisor. See
> "AMD64 Architecture Programmer's Manual Volume 2: System Programming",
> section "15.35 Encrypted State (SEV-ES)" [1].
>
> In order to allow a hypervisor to perform functions on behalf of a
> guest, there is architectural support for notifying a guest's
> operating system when certain types of VMEXITs are about to occur.
> This allows the guest to selectively share information with the
> hypervisor to satisfy the requested function. The notification is
> performed using a new exception, the VMM Communication exception
> (#VC). The information is shared through the Guest-Hypervisor Communication Block (GHCB) using the VMGEXIT instruction.
> The GHCB format and the protocol for using it is documented in "SEV-ES
> Guest-Hypervisor Communication Block Standardization" [2].
>
> The main areas of the EDK2 code that are updated to support SEV-ES are
> around the exception handling support and the AP boot support.
>
> Exception support is required starting in Sec, continuing through Pei
> and into Dxe in order to handle #VC exceptions that are generated.
> Each AP requires it's own GHCB page as well as a page to hold values
> specific to that AP.
>
> AP booting poses some interesting challenges. The INIT-SIPI-SIPI
> sequence is typically used to boot the APs. However, the hypervisor is
> not allowed to update the guest registers. The GHCB document [2] talks
> about how SMP booting under SEV-ES is performed.
>
> Since the GHCB page must be a shared (unencrypted) page, the processor
> must be running in long mode in order for the guest and hypervisor to
> communicate with each other. As a result, SEV-ES is only supported
> under the X64 architecture.
>
> This series adds a new library requirement for the VmgExitLib library
> against the UefiCpuPkg CpuExceptionHandlerLib library and the
> UefiCpuPkg MpInitLib library. The edk2-platforms repo requires
> updates/patches to add the new library requirement. To accomodate
> that, this series could be split between:
>
> patch number 10:
> UefiPayloadPkg: Prepare UefiPayloadPkg to use the VmgExitLib library
>
> and patch number 11:
> UefiCpuPkg/CpuExceptionHandler: Add base support for the #VC
> exception
>
> The updates to edk2-platforms can be applied at the split.
>
> [1] https://www.amd.com/system/files/TechDocs/24593.pdf
> [2] https://developer.amd.com/wp-content/resources/56421.pdf
>
> ---
>
> These patches are based on commit:
> 9565ab67c209 ("ShellPkg: smbiosview - Change some type 17 field values
> format")
>
> A version of the tree can be found at:
> https://github.com/AMDESE/ovmf/tree/sev-es-v22
>
> Cc: Andrew Fish <afish at apple.com>
> Cc: Anthony Perard <anthony.perard at citrix.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
> Cc: Benjamin You <benjamin.you at intel.com>
> Cc: Dandan Bi <dandan.bi at intel.com>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Guo Dong <guo.dong at intel.com>
> Cc: Hao A Wu <hao.a.wu at intel.com>
> Cc: Jian J Wang <jian.j.wang at intel.com>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Julien Grall <julien at xen.org>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Leif Lindholm <leif at nuviainc.com>
> Cc: Liming Gao <liming.gao at intel.com>
> Cc: Maurice Ma <maurice.ma at intel.com>
> Cc: Michael D Kinney <michael.d.kinney at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
>
> Changes since v13:
> - Fixup the AsmRelocateApLoop() call site so IA32 successfully boots APs.
> Do this by appending the three new parameters without altering the
> original parameter passing order.
> - Minor updates to description text and help text to expand the GHCB
> acronym.
>
> Changes since v12:
> - Change IA32 VMGEXIT .nasm file to issue an int 3. Depending on the
> version of NASM, the "BITS 64" trick to get NASM to recognize the
> VMMCALL instruction (VMGEXIT is a REP VMMCALL) caused an error. Since
> SEV-ES is X64 only, VMGEXIT should never be called in IA32.
>
> Changes since v11:
> - Make the XGETBV and VMGEXIT .nasm files buildable for all environments
> and remove the updates that add these instructions to GccInline.c
>
> Changes since v10:
> - Fix conflicts around GccInline.c file after moving to latest commit
> - Fix conflicts with OVMF PCD values after moving to latest commit
>
> Changes since v9:
> - Fixed bit field declarations in the GHCB structure to use UINT32
> and not UINT64.
> - Fixed a warning produced by VS2019 in the instruction parsing code
> by expliciting casting a bit shift to an INT64.
> - Sorted section entries in the OVMF VmgExitLib INF file.
> - Moved the new Maintainers.txt entry so entries remain sorted.
> - Documentation style fixes for return values.
> - Miscellaneous code style fixes.
>
> Changes since v8:
> - Move IOIO exit info definitions into Ghcb.h file
> - Add a macro for calculating IO instruction bytes (IOIO_DATA_BYTES)
> - Exception handler support for debug registers
> - Moved the DRx register saving changes into the UefiCpuPkg patch for
> base #VC support in CpuExceptionHandlerLib.
> - OvmfPkg VmgExitLib
> - Remove the .uni file
> - Update .inf file:
> - New file location for VmgExitVcHandler.c
> - Add additional Packages and LibraryClasses
> - Introduce a header file to hold the #VC instruction parsing related
> definitions
> - Include additional #defines for instruction decoding to replace
> hard coded values for things like instruction prefixes and escapes.
> - Replace hardcoded CPUID values with values from existing header files
> and use existing CR4 definition for accessing CR4 data.
> - Change the type used for obtaining data addresses in the instruction
> parsing
> - Switch from INTN to UINT64 and use compiler conversions and casting
> to perform the correct address calculation
> - ResetVector code:
> - Revert some inadvertant changes introduced in v7 for reserving the
> SEV-ES work area memory and for checking the status of SEV-ES.
> - AP Booting
> - Provide support for non-broadcast INIT-SIPI-SIPI AP boot (minimize
> code duplication by creating a function to set the AP jump table
> vector address).
> - Fix file/directory entry in maintainer changes.
> - Various coding style fixes
> - Commenting, if statements, etc.
> - Various documentation style fixes
>
> Changes since v7:
> - Reserve the SEV-ES workarea when S3 is enabled
> - Fix warnings issued by the Visual Studio compiler
> - Create a NULL VmgExitLib instance that is used for VMGEXIT
> related operations as well as #VC handling. Then create the full
> VmgExitLib support only in OvmfPkg - where it will be used. This
> removes a bunch of implementation code from platforms that will
> not be using the functionality.
> - Remove single use interfaces from the VmgExitLib (VmgMmioWrite
> and VmgSetApJumpTable)
>
> Changes since v6:
> - Add function comments to all functions, including local functions
> - Add function parameter direction to all functions (in/out)
> - Add support for MMIO MOVZX/MOVSX instructions
> - Ensure the per-CPU variable page remains encrypted
> - Coding-style fixes as identified by Ecc
>
> Changes since v5:
> - Remove extraneous VmgExitLib usage
> - Miscellaneous changes to address feedback (coding style, etc.)
>
> Changes since v4:
> - Move the SEV-ES protocol negotiation out of the SEC exception handler
> and into the SecMain.c file. As a result:
> - Move the SecGhcb related PCDs out of UefiCpuPkg and into OvmfPkg
> - Combine SecAMDSevVcHandler.c and PeiDxeAMDSevVcHandler.c into a
> single AMDSevVcHandler.c
> - Consolidate VmgExitLib usage into common LibraryClasses sections
> - Add documentation comments to the VmgExitLib functions
>
> Changes since v3:
> - Remove the need for the MP library finalization routine. The AP
> jump table address will be held by the hypervisor rather than
> communicated via the GHCB MSR. This removes some fragility around
> the UEFI to OS transition.
> - Rename the SEV-ES RIP reset area to SEV-ES workarea and use it to
> communicate the SEV-ES status, so that SEC CPU exception handling is
> only established for an SEV-ES guest.
> - Fix SMM build breakageAdd around QemuFlashPtrWrite().
> - Fix SMM build breakage by adding VC exception support the SMM CPU
> exception handling.
> - Add memory fencing around the invocation of AsmVmgExit().
> - Clarify comments around the SEV-ES AP reset RIP values and usage.
> - Move some PCD definitions from MdeModulePkg to UefiCpuPkg.
> - Remove the 16-bit code selector definition from MdeModulePkg
>
> Changes since v2:
> - Added a way to locate the SEV-ES fixed AP RIP address for starting
> AP's to avoid updating the actual flash image (build time location
> that is identified with a GUID value).
> - Create a VmgExit library to replace static inline functions.
> - Move some PCDs to the appropriate packages
> - Add support for writing to QEMU flash under SEV-ES
> - Add additional MMIO opcode support
> - Cleaned up the GHCB MSR CPUID protocol support
>
> Changes since v1:
> - Patches reworked to be more specific to the component/area being updated
> and order of definition/usage
> - Created a library for VMGEXIT-related functions to replace use of inline
> functions
> - Allocation method for GDT changed from AllocatePool to AllocatePages
> - Early caching only enabled for SEV-ES guests
> - Ensure AP loop mode set to halt loop mode for SEV-ES guests
> - Reserved SEC GHCB-related memory areas when S3 is enabled
>
> Tom Lendacky (46):
> MdeModulePkg: Create PCDs to be used in support of SEV-ES
> UefiCpuPkg: Create PCD to be used in support of SEV-ES
> MdePkg: Add the MSR definition for the GHCB register
> MdePkg: Add a structure definition for the GHCB
> MdeModulePkg/DxeIplPeim: Support GHCB pages when creating page tables
> MdePkg/BaseLib: Add support for the XGETBV instruction
> MdePkg/BaseLib: Add support for the VMGEXIT instruction
> UefiCpuPkg: Implement library support for VMGEXIT
> OvmfPkg: Prepare OvmfPkg to use the VmgExitLib library
> UefiPayloadPkg: Prepare UefiPayloadPkg to use the VmgExitLib library
> UefiCpuPkg/CpuExceptionHandler: Add base support for the #VC exception
> OvmfPkg/VmgExitLib: Implement library support for VmgExitLib in OVMF
> OvmfPkg/VmgExitLib: Add support for IOIO_PROT NAE events
> OvmfPkg/VmgExitLib: Support string IO for IOIO_PROT NAE events
> OvmfPkg/VmgExitLib: Add support for CPUID NAE events
> OvmfPkg/VmgExitLib: Add support for MSR_PROT NAE events
> OvmfPkg/VmgExitLib: Add support for NPF NAE events (MMIO)
> OvmfPkg/VmgExitLib: Add support for WBINVD NAE events
> OvmfPkg/VmgExitLib: Add support for RDTSC NAE events
> OvmfPkg/VmgExitLib: Add support for RDPMC NAE events
> OvmfPkg/VmgExitLib: Add support for INVD NAE events
> OvmfPkg/VmgExitLib: Add support for VMMCALL NAE events
> OvmfPkg/VmgExitLib: Add support for RDTSCP NAE events
> OvmfPkg/VmgExitLib: Add support for MONITOR/MONITORX NAE events
> OvmfPkg/VmgExitLib: Add support for MWAIT/MWAITX NAE events
> OvmfPkg/VmgExitLib: Add support for DR7 Read/Write NAE events
> OvmfPkg/MemEncryptSevLib: Add an SEV-ES guest indicator function
> OvmfPkg: Add support to perform SEV-ES initialization
> OvmfPkg: Create a GHCB page for use during Sec phase
> OvmfPkg/PlatformPei: Reserve GHCB-related areas if S3 is supported
> OvmfPkg: Create GHCB pages for use during Pei and Dxe phase
> OvmfPkg/PlatformPei: Move early GDT into ram when SEV-ES is enabled
> UefiCpuPkg: Create an SEV-ES workarea PCD
> OvmfPkg: Reserve a page in memory for the SEV-ES usage
> OvmfPkg/PlatformPei: Reserve SEV-ES work area if S3 is supported
> OvmfPkg/ResetVector: Add support for a 32-bit SEV check
> OvmfPkg/Sec: Add #VC exception handling for Sec phase
> OvmfPkg/Sec: Enable cache early to speed up booting
> OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Bypass flash detection with
> SEV-ES
> UefiCpuPkg: Add a 16-bit protected mode code segment descriptor
> UefiCpuPkg/MpInitLib: Add CPU MP data flag to indicate if SEV-ES is
> enabled
> UefiCpuPkg: Allow AP booting under SEV-ES
> OvmfPkg: Use the SEV-ES work area for the SEV-ES AP reset vector
> OvmfPkg: Move the GHCB allocations into reserved memory
> UefiCpuPkg/MpInitLib: Prepare SEV-ES guest APs for OS use
> Maintainers.txt: Add reviewers for the OvmfPkg SEV-related files
>
> MdeModulePkg/MdeModulePkg.dec | 9 +
> OvmfPkg/OvmfPkg.dec | 9 +
> UefiCpuPkg/UefiCpuPkg.dec | 17 +
> OvmfPkg/OvmfPkgIa32.dsc | 6 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 6 +
> OvmfPkg/OvmfPkgX64.dsc | 6 +
> OvmfPkg/OvmfXen.dsc | 1 +
> UefiCpuPkg/UefiCpuPkg.dsc | 2 +
> UefiPayloadPkg/UefiPayloadPkgIa32.dsc | 2 +
> UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc | 2 +
> OvmfPkg/OvmfPkgX64.fdf | 9 +
> MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 +
> MdePkg/Library/BaseLib/BaseLib.inf | 4 +
> OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 36 +
> OvmfPkg/PlatformPei/PlatformPei.inf | 9 +
> .../FvbServicesRuntimeDxe.inf | 2 +
> OvmfPkg/ResetVector/ResetVector.inf | 8 +
> OvmfPkg/Sec/SecMain.inf | 4 +
> .../DxeCpuExceptionHandlerLib.inf | 1 +
> .../PeiCpuExceptionHandlerLib.inf | 1 +
> .../SecPeiCpuExceptionHandlerLib.inf | 1 +
> .../SmmCpuExceptionHandlerLib.inf | 1 +
> .../Xcode5SecPeiCpuExceptionHandlerLib.inf | 1 +
> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 4 +
> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 4 +
> .../Library/VmgExitLibNull/VmgExitLibNull.inf | 27 +
> .../Core/DxeIplPeim/X64/VirtualMemory.h | 12 +-
> MdePkg/Include/Library/BaseLib.h | 31 +
> MdePkg/Include/Register/Amd/Fam17Msr.h | 46 +
> MdePkg/Include/Register/Amd/Ghcb.h | 166 ++
> .../IndustryStandard/InstructionParsing.h | 83 +
> OvmfPkg/Include/Library/MemEncryptSevLib.h | 12 +
> .../QemuFlash.h | 13 +
> UefiCpuPkg/CpuDxe/CpuGdt.h | 4 +-
> UefiCpuPkg/Include/Library/VmgExitLib.h | 103 +
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 68 +-
> .../Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 +-
> .../Core/DxeIplPeim/X64/DxeLoadFunc.c | 11 +-
> .../Core/DxeIplPeim/X64/VirtualMemory.c | 57 +-
> .../MemEncryptSevLibInternal.c | 75 +-
> OvmfPkg/Library/VmgExitLib/VmgExitLib.c | 159 ++
> OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 1716 +++++++++++++++++
> OvmfPkg/PlatformPei/AmdSev.c | 89 +
> OvmfPkg/PlatformPei/MemDetect.c | 43 +
> .../QemuFlash.c | 23 +-
> .../QemuFlashDxe.c | 40 +
> .../QemuFlashSmm.c | 16 +
> OvmfPkg/Sec/SecMain.c | 188 +-
> UefiCpuPkg/CpuDxe/CpuGdt.c | 8 +-
> .../CpuExceptionCommon.c | 10 +-
> .../PeiDxeSmmCpuException.c | 20 +-
> .../SecPeiCpuException.c | 19 +
> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 120 +-
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 337 +++-
> UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 19 +
> .../Library/VmgExitLibNull/VmgExitLibNull.c | 121 ++
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/SmmFuncsArch.c | 2 +-
> Maintainers.txt | 10 +
> MdeModulePkg/MdeModulePkg.uni | 8 +
> MdePkg/Library/BaseLib/Ia32/VmgExit.nasm | 38 +
> MdePkg/Library/BaseLib/Ia32/XGetBv.nasm | 31 +
> MdePkg/Library/BaseLib/X64/VmgExit.nasm | 32 +
> MdePkg/Library/BaseLib/X64/XGetBv.nasm | 34 +
> OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 100 +
> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 351 +++-
> OvmfPkg/ResetVector/ResetVector.nasmb | 20 +
> .../X64/ExceptionHandlerAsm.nasm | 17 +
> .../X64/Xcode5ExceptionHandlerAsm.nasm | 17 +
> UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 2 +-
> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 20 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 4 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 369 +++-
> .../Library/VmgExitLibNull/VmgExitLibNull.uni | 15 +
> .../ResetVector/Vtf0/Ia16/Real16ToFlat32.asm | 9 +
> UefiCpuPkg/UefiCpuPkg.uni | 11 +
> 75 files changed, 4777 insertions(+), 100 deletions(-) create mode
> 100644 OvmfPkg/Library/VmgExitLib/VmgExitLib.inf
> create mode 100644
> UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
> create mode 100644 MdePkg/Include/Register/Amd/Ghcb.h
> create mode 100644
> OvmfPkg/Include/IndustryStandard/InstructionParsing.h
> create mode 100644 UefiCpuPkg/Include/Library/VmgExitLib.h
> create mode 100644 OvmfPkg/Library/VmgExitLib/VmgExitLib.c
> create mode 100644 OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
> create mode 100644 UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.c
> create mode 100644 MdePkg/Library/BaseLib/Ia32/VmgExit.nasm
> create mode 100644 MdePkg/Library/BaseLib/Ia32/XGetBv.nasm
> create mode 100644 MdePkg/Library/BaseLib/X64/VmgExit.nasm
> create mode 100644 MdePkg/Library/BaseLib/X64/XGetBv.nasm
> create mode 100644 OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm
> create mode 100644
> UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.uni
>
For all patches except #10 ("UefiPayloadPkg: Prepare UefiPayloadPkg to use the VmgExitLib library") and #46 ("Maintainers.txt: Add reviewers for the OvmfPkg SEV-related files"):
Regression-tested-by: Laszlo Ersek <lersek at redhat.com>
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#63937): https://edk2.groups.io/g/devel/message/63937
Mute This Topic: https://groups.io/mt/76056465/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list