public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Kinney, Michael D" <michael.d.kinney@intel.com>
To: Laszlo Ersek <lersek@redhat.com>,
	edk2-devel-01 <edk2-devel@lists.01.org>,
	 "Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "Justen, Jordan L" <jordan.l.justen@intel.com>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [PATCH 01/20] OvmfPkg/MemEncryptSevLib: rewrap to 79 characters width
Date: Fri, 2 Mar 2018 00:33:50 +0000	[thread overview]
Message-ID: <E92EE9817A31E24EB0585FDF735412F5B896E408@ORSMSX113.amr.corp.intel.com> (raw)
In-Reply-To: <20180302000408.14201-2-lersek@redhat.com>

Laszlo,

Sorting #includes looks strange to me.

We usually include the top level environment include
first (e.g. <PiPei.h>) and then the libs, protocols,
ppis, GUIDs grouped together.

If it is a lib module, the produced libs are listed
first followed by the consumed libs.

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Thursday, March 1, 2018 4:04 PM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Brijesh Singh <brijesh.singh@amd.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>
> Subject: [edk2] [PATCH 01/20] OvmfPkg/MemEncryptSevLib:
> rewrap to 79 characters width
> 
> There are many overlong lines; it's hard to work with
> the library like
> this. Rewrap all files to 79 columns.
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Brijesh Singh <brijesh.singh@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSevL
> ib.inf   |   7 +-
>  OvmfPkg/Include/Library/MemEncryptSevLib.h
> |  20 ++-
> 
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.
> h        | 111 ++++++++------
> 
> OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptSev
> Lib.c    |  34 +++--
> 
> OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLibIn
> ternal.c |   8 +-
> 
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSevL
> ib.c     |  58 ++++---
> 
> OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.
> c        | 158 +++++++++++++-------
>  7 files changed, 253 insertions(+), 143 deletions(-)
> 
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSe
> vLib.inf
> b/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSe
> vLib.inf
> index 3cfd80a28c1d..81b075194ace 100644
> ---
> a/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSe
> vLib.inf
> +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/BaseMemEncryptSe
> vLib.inf
> @@ -1,45 +1,48 @@
>  ## @file
>  #  Library provides the helper functions for SEV guest
>  #
>  # Copyright (c) 2017 Advanced Micro Devices. All
> rights reserved.<BR>
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and
> conditions of the BSD
>  #  License which accompanies this distribution. The
> full text of the license
>  #  may be found at http://opensource.org/licenses/bsd-
> license.php
> +#
>  #  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS,
> -#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> +#  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR
> +#  IMPLIED.
>  #
>  #
>  ##
> 
>  [Defines]
>    INF_VERSION                    = 1.25
>    BASE_NAME                      = MemEncryptSevLib
>    FILE_GUID                      = c1594631-3888-4be4-
> 949f-9c630dbc842b
>    MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  =
> MemEncryptSevLib|PEIM DXE_DRIVER DXE_RUNTIME_DRIVER
> DXE_SMM_DRIVER UEFI_DRIVER
> 
>  #
> -# The following information is for reference only and
> not required by the build tools.
> +# The following information is for reference only and
> not required by the build
> +# tools.
>  #
>  # VALID_ARCHITECTURES           = IA32 X64
>  #
> 
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
>    OvmfPkg/OvmfPkg.dec
>    UefiCpuPkg/UefiCpuPkg.dec
> 
>  [Sources.X64]
>    MemEncryptSevLibInternal.c
>    X64/MemEncryptSevLib.c
>    X64/VirtualMemory.c
> 
>  [Sources.IA32]
>    MemEncryptSevLibInternal.c
>    Ia32/MemEncryptSevLib.c
> 
>  [LibraryClasses]
> diff --git a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> index b6753762423e..4f3ba9f22cb4 100644
> --- a/OvmfPkg/Include/Library/MemEncryptSevLib.h
> +++ b/OvmfPkg/Include/Library/MemEncryptSevLib.h
> @@ -18,64 +18,68 @@
>  #define _MEM_ENCRYPT_SEV_LIB_H_
> 
>  #include <Base.h>
> 
>  /**
>    Returns a boolean to indicate whether SEV is enabled
> 
>    @retval TRUE           SEV is active
>    @retval FALSE          SEV is not enabled
>    **/
>  BOOLEAN
>  EFIAPI
>  MemEncryptSevIsEnabled (
>    VOID
>    );
> 
>  /**
>    This function clears memory encryption bit for the
> memory region specified
>    by BaseAddress and Number of pages from the current
> page table context.
> 
> -  @param[in]  BaseAddress           The physical
> address that is the start address
> -                                    of a memory
> region.
> -  @param[in]  NumberOfPages         The number of
> pages from start memory region.
> +  @param[in]  BaseAddress           The physical
> address that is the start
> +                                    address of a
> memory region.
> +  @param[in]  NumberOfPages         The number of
> pages from start memory
> +                                    region.
>    @param[in]  Flush                 Flush the caches
> before clearing the bit
>                                      (mostly TRUE
> except MMIO addresses)
> 
> -  @retval RETURN_SUCCESS            The attributes
> were cleared for the memory region.
> +  @retval RETURN_SUCCESS            The attributes
> were cleared for the memory
> +                                    region.
>    @retval RETURN_INVALID_PARAMETER  Number of pages is
> zero.
>    @retval RETURN_UNSUPPORTED        Clearing memory
> encryption attribute is not
>                                      supported
>    **/
>  RETURN_STATUS
>  EFIAPI
>  MemEncryptSevClearPageEncMask (
>    IN PHYSICAL_ADDRESS         Cr3BaseAddress,
>    IN PHYSICAL_ADDRESS         BaseAddress,
>    IN UINTN                    NumberOfPages,
>    IN BOOLEAN                  CacheFlush
>    );
> 
>  /**
>    This function sets memory encryption bit for the
> memory region specified by
>    BaseAddress and Number of pages from the current
> page table context.
> 
> -  @param[in]  BaseAddress           The physical
> address that is the start address
> -                                    of a memory
> region.
> -  @param[in]  NumberOfPages         The number of
> pages from start memory region.
> +  @param[in]  BaseAddress           The physical
> address that is the start
> +                                    address of a
> memory region.
> +  @param[in]  NumberOfPages         The number of
> pages from start memory
> +                                    region.
>    @param[in]  Flush                 Flush the caches
> before clearing the bit
>                                      (mostly TRUE
> except MMIO addresses)
> 
> -  @retval RETURN_SUCCESS            The attributes
> were set for the memory region.
> +  @retval RETURN_SUCCESS            The attributes
> were set for the memory
> +                                    region.
>    @retval RETURN_INVALID_PARAMETER  Number of pages is
> zero.
>    @retval RETURN_UNSUPPORTED        Clearing memory
> encryption attribute is not
>                                      supported
>    **/
>  RETURN_STATUS
>  EFIAPI
>  MemEncryptSevSetPageEncMask (
>    IN PHYSICAL_ADDRESS         Cr3BaseAddress,
>    IN PHYSICAL_ADDRESS         BaseAddress,
>    IN UINTN                    NumberOfPages,
>    IN BOOLEAN                  CacheFlush
>    );
>  #endif // _MEM_ENCRYPT_SEV_LIB_H_
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemor
> y.h
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemor
> y.h
> index e7b5634b45c1..7dd1bbe0eb26 100644
> ---
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemor
> y.h
> +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemor
> y.h
> @@ -1,212 +1,239 @@
>  /** @file
> 
>    Virtual Memory Management Services to set or clear
> the memory encryption bit
> 
> -Copyright (c) 2006 - 2016, Intel Corporation. All
> rights reserved.<BR>
> -Copyright (c) 2017, AMD Incorporated. All rights
> reserved.<BR>
> +  Copyright (c) 2006 - 2016, Intel Corporation. All
> rights reserved.<BR>
> +  Copyright (c) 2017, AMD Incorporated. All rights
> reserved.<BR>
> 
> -This program and the accompanying materials
> -are licensed and made available under the terms and
> conditions of the BSD License
> -which accompanies this distribution.  The full text of
> the license may be found at
> -http://opensource.org/licenses/bsd-license.php
> +  This program and the accompanying materials are
> licensed and made available
> +  under the terms and conditions of the BSD License
> which accompanies this
> +  distribution.  The full text of the license may be
> found at
> +  http://opensource.org/licenses/bsd-license.php
> 
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN
> "AS IS" BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS, WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> 
> -Code is derived from
> MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
> +  Code is derived from
> MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h
> 
>  **/
> 
>  #ifndef __VIRTUAL_MEMORY__
>  #define __VIRTUAL_MEMORY__
> 
>  #include <Uefi.h>
>  #include <Library/BaseLib.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>
> 
>  #include <Library/CacheMaintenanceLib.h>
>  #define SYS_CODE64_SEL 0x38
> 
>  #pragma pack(1)
> 
>  //
>  // Page-Map Level-4 Offset (PML4) and
>  // Page-Directory-Pointer Offset (PDPE) entries 4K &
> 2MB
>  //
> 
>  typedef union {
>    struct {
> -    UINT64  Present:1;                // 0 = Not
> present in memory, 1 = Present in memory
> +    UINT64  Present:1;                // 0 = Not
> present in memory,
> +                                      //   1 = Present
> in memory
>      UINT64  ReadWrite:1;              // 0 = Read-
> Only, 1= Read/Write
>      UINT64  UserSupervisor:1;         // 0 =
> Supervisor, 1=User
> -    UINT64  WriteThrough:1;           // 0 = Write-
> Back caching, 1=Write-Through caching
> +    UINT64  WriteThrough:1;           // 0 = Write-
> Back caching,
> +                                      //   1 = Write-
> Through caching
>      UINT64  CacheDisabled:1;          // 0 = Cached,
> 1=Non-Cached
> -    UINT64  Accessed:1;               // 0 = Not
> accessed, 1 = Accessed (set by CPU)
> +    UINT64  Accessed:1;               // 0 = Not
> accessed,
> +                                      //   1 =
> Accessed (set by CPU)
>      UINT64  Reserved:1;               // Reserved
>      UINT64  MustBeZero:2;             // Must Be Zero
>      UINT64  Available:3;              // Available for
> use by system software
>      UINT64  PageTableBaseAddress:40;  // Page Table
> Base Address
>      UINT64  AvabilableHigh:11;        // Available for
> use by system software
>      UINT64  Nx:1;                     // No Execute
> bit
>    } Bits;
>    UINT64    Uint64;
>  } PAGE_MAP_AND_DIRECTORY_POINTER;
> 
>  //
>  // Page Table Entry 4KB
>  //
>  typedef union {
>    struct {
> -    UINT64  Present:1;                // 0 = Not
> present in memory, 1 = Present in memory
> +    UINT64  Present:1;                // 0 = Not
> present in memory,
> +                                      //   1 = Present
> in memory
>      UINT64  ReadWrite:1;              // 0 = Read-
> Only, 1= Read/Write
>      UINT64  UserSupervisor:1;         // 0 =
> Supervisor, 1=User
> -    UINT64  WriteThrough:1;           // 0 = Write-
> Back caching, 1=Write-Through caching
> +    UINT64  WriteThrough:1;           // 0 = Write-
> Back caching,
> +                                      //   1 = Write-
> Through caching
>      UINT64  CacheDisabled:1;          // 0 = Cached,
> 1=Non-Cached
> -    UINT64  Accessed:1;               // 0 = Not
> accessed, 1 = Accessed (set by CPU)
> -    UINT64  Dirty:1;                  // 0 = Not
> Dirty, 1 = written by processor on access to page
> +    UINT64  Accessed:1;               // 0 = Not
> accessed,
> +                                      //   1 =
> Accessed (set by CPU)
> +    UINT64  Dirty:1;                  // 0 = Not
> Dirty, 1 = written by
> +                                      //   processor
> on access to page
>      UINT64  PAT:1;                    //
> -    UINT64  Global:1;                 // 0 = Not
> global page, 1 = global page TLB not cleared on CR3
> write
> +    UINT64  Global:1;                 // 0 = Not
> global page, 1 = global page
> +                                      //   TLB not
> cleared on CR3 write
>      UINT64  Available:3;              // Available for
> use by system software
>      UINT64  PageTableBaseAddress:40;  // Page Table
> Base Address
>      UINT64  AvabilableHigh:11;        // Available for
> use by system software
> -    UINT64  Nx:1;                     // 0 = Execute
> Code, 1 = No Code Execution
> +    UINT64  Nx:1;                     // 0 = Execute
> Code,
> +                                      //   1 = No Code
> Execution
>    } Bits;
>    UINT64    Uint64;
>  } PAGE_TABLE_4K_ENTRY;
> 
>  //
>  // Page Table Entry 2MB
>  //
>  typedef union {
>    struct {
> -    UINT64  Present:1;                // 0 = Not
> present in memory, 1 = Present in memory
> +    UINT64  Present:1;                // 0 = Not
> present in memory,
> +                                      //   1 = Present
> in memory
>      UINT64  ReadWrite:1;              // 0 = Read-
> Only, 1= Read/Write
>      UINT64  UserSupervisor:1;         // 0 =
> Supervisor, 1=User
> -    UINT64  WriteThrough:1;           // 0 = Write-
> Back caching, 1=Write-Through caching
> +    UINT64  WriteThrough:1;           // 0 = Write-
> Back caching,
> +                                      //   1=Write-
> Through caching
>      UINT64  CacheDisabled:1;          // 0 = Cached,
> 1=Non-Cached
> -    UINT64  Accessed:1;               // 0 = Not
> accessed, 1 = Accessed (set by CPU)
> -    UINT64  Dirty:1;                  // 0 = Not
> Dirty, 1 = written by processor on access to page
> +    UINT64  Accessed:1;               // 0 = Not
> accessed,
> +                                      //   1 =
> Accessed (set by CPU)
> +    UINT64  Dirty:1;                  // 0 = Not
> Dirty, 1 = written by
> +                                      //   processor
> on access to page
>      UINT64  MustBe1:1;                // Must be 1
> -    UINT64  Global:1;                 // 0 = Not
> global page, 1 = global page TLB not cleared on CR3
> write
> +    UINT64  Global:1;                 // 0 = Not
> global page, 1 = global page
> +                                      //   TLB not
> cleared on CR3 write
>      UINT64  Available:3;              // Available for
> use by system software
>      UINT64  PAT:1;                    //
>      UINT64  MustBeZero:8;             // Must be zero;
>      UINT64  PageTableBaseAddress:31;  // Page Table
> Base Address
>      UINT64  AvabilableHigh:11;        // Available for
> use by system software
> -    UINT64  Nx:1;                     // 0 = Execute
> Code, 1 = No Code Execution
> +    UINT64  Nx:1;                     // 0 = Execute
> Code,
> +                                      //   1 = No Code
> Execution
>    } Bits;
>    UINT64    Uint64;
>  } PAGE_TABLE_ENTRY;
> 
>  //
>  // Page Table Entry 1GB
>  //
>  typedef union {
>    struct {
> -    UINT64  Present:1;                // 0 = Not
> present in memory, 1 = Present in memory
> +    UINT64  Present:1;                // 0 = Not
> present in memory,
> +                                      //   1 = Present
> in memory
>      UINT64  ReadWrite:1;              // 0 = Read-
> Only, 1= Read/Write
>      UINT64  UserSupervisor:1;         // 0 =
> Supervisor, 1=User
> -    UINT64  WriteThrough:1;           // 0 = Write-
> Back caching, 1=Write-Through caching
> +    UINT64  WriteThrough:1;           // 0 = Write-
> Back caching,
> +                                      //   1 = Write-
> Through caching
>      UINT64  CacheDisabled:1;          // 0 = Cached,
> 1=Non-Cached
> -    UINT64  Accessed:1;               // 0 = Not
> accessed, 1 = Accessed (set by CPU)
> -    UINT64  Dirty:1;                  // 0 = Not
> Dirty, 1 = written by processor on access to page
> +    UINT64  Accessed:1;               // 0 = Not
> accessed,
> +                                      //   1 =
> Accessed (set by CPU)
> +    UINT64  Dirty:1;                  // 0 = Not
> Dirty, 1 = written by
> +                                      //   processor
> on access to page
>      UINT64  MustBe1:1;                // Must be 1
> -    UINT64  Global:1;                 // 0 = Not
> global page, 1 = global page TLB not cleared on CR3
> write
> +    UINT64  Global:1;                 // 0 = Not
> global page, 1 = global page
> +                                      //   TLB not
> cleared on CR3 write
>      UINT64  Available:3;              // Available for
> use by system software
>      UINT64  PAT:1;                    //
>      UINT64  MustBeZero:17;            // Must be zero;
>      UINT64  PageTableBaseAddress:22;  // Page Table
> Base Address
>      UINT64  AvabilableHigh:11;        // Available for
> use by system software
> -    UINT64  Nx:1;                     // 0 = Execute
> Code, 1 = No Code Execution
> +    UINT64  Nx:1;                     // 0 = Execute
> Code,
> +                                      //   1 = No Code
> Execution
>    } Bits;
>    UINT64    Uint64;
>  } PAGE_TABLE_1G_ENTRY;
> 
>  #pragma pack()
> 
>  #define IA32_PG_P                   BIT0
>  #define IA32_PG_RW                  BIT1
>  #define IA32_PG_PS                  BIT7
> 
>  #define PAGING_PAE_INDEX_MASK       0x1FF
> 
>  #define PAGING_4K_ADDRESS_MASK_64
> 0x000FFFFFFFFFF000ull
>  #define PAGING_2M_ADDRESS_MASK_64
> 0x000FFFFFFFE00000ull
>  #define PAGING_1G_ADDRESS_MASK_64
> 0x000FFFFFC0000000ull
> 
>  #define PAGING_L1_ADDRESS_SHIFT     12
>  #define PAGING_L2_ADDRESS_SHIFT     21
>  #define PAGING_L3_ADDRESS_SHIFT     30
>  #define PAGING_L4_ADDRESS_SHIFT     39
> 
>  #define PAGING_PML4E_NUMBER         4
> 
>  #define PAGETABLE_ENTRY_MASK        ((1UL << 9) - 1)
>  #define PML4_OFFSET(x)              ( (x >> 39) &
> PAGETABLE_ENTRY_MASK)
>  #define PDP_OFFSET(x)               ( (x >> 30) &
> PAGETABLE_ENTRY_MASK)
>  #define PDE_OFFSET(x)               ( (x >> 21) &
> PAGETABLE_ENTRY_MASK)
>  #define PTE_OFFSET(x)               ( (x >> 12) &
> PAGETABLE_ENTRY_MASK)
>  #define PAGING_1G_ADDRESS_MASK_64
> 0x000FFFFFC0000000ull
> 
>  #define PAGE_TABLE_POOL_ALIGNMENT   BASE_2MB
>  #define PAGE_TABLE_POOL_UNIT_SIZE   SIZE_2MB
> -#define PAGE_TABLE_POOL_UNIT_PAGES  EFI_SIZE_TO_PAGES
> (PAGE_TABLE_POOL_UNIT_SIZE)
> +#define PAGE_TABLE_POOL_UNIT_PAGES  \
> +  EFI_SIZE_TO_PAGES (PAGE_TABLE_POOL_UNIT_SIZE)
>  #define PAGE_TABLE_POOL_ALIGN_MASK  \
>    (~(EFI_PHYSICAL_ADDRESS)(PAGE_TABLE_POOL_ALIGNMENT -
> 1))
> 
>  typedef struct {
>    VOID            *NextPool;
>    UINTN           Offset;
>    UINTN           FreePages;
>  } PAGE_TABLE_POOL;
> 
> 
> 
>  /**
> -  This function clears memory encryption bit for the
> memory region specified by PhysicalAddress
> -  and length from the current page table context.
> +  This function clears memory encryption bit for the
> memory region specified by
> +  PhysicalAddress and length from the current page
> table context.
> 
> -  @param[in]  PhysicalAddress         The physical
> address that is the start address of a memory region.
> +  @param[in]  PhysicalAddress         The physical
> address that is the start
> +                                      address of a
> memory region.
>    @param[in]  Length                  The length of
> memory region
> -  @param[in]  Flush                   Flush the caches
> before applying the encryption mask
> +  @param[in]  Flush                   Flush the caches
> before applying the
> +                                      encryption mask
> 
> -  @retval RETURN_SUCCESS              The attributes
> were cleared for the memory region.
> +  @retval RETURN_SUCCESS              The attributes
> were cleared for the
> +                                      memory region.
>    @retval RETURN_INVALID_PARAMETER    Number of pages
> is zero.
> -  @retval RETURN_UNSUPPORTED          Setting the
> memory encyrption attribute is not supported
> +  @retval RETURN_UNSUPPORTED          Setting the
> memory encyrption attribute
> +                                      is not supported
>  **/
>  RETURN_STATUS
>  EFIAPI
>  InternalMemEncryptSevSetMemoryDecrypted (
>    IN  PHYSICAL_ADDRESS     Cr3BaseAddress,
>    IN  PHYSICAL_ADDRESS     PhysicalAddress,
>    IN  UINT64               Length,
>    IN  BOOLEAN              CacheFlush
>    );
> 
>  /**
>    This function sets memory encryption bit for the
> memory region specified by
>    PhysicalAddress and length from the current page
> table context.
> 
> -  @param[in]  PhysicalAddress         The physical
> address that is the start address
> -                                      of a memory
> region.
> +  @param[in]  PhysicalAddress         The physical
> address that is the start
> +                                      address of a
> memory region.
>    @param[in]  Length                  The length of
> memory region
>    @param[in]  Flush                   Flush the caches
> before applying the
>                                        encryption mask
> 
> -  @retval RETURN_SUCCESS              The attributes
> were cleared for the memory region.
> +  @retval RETURN_SUCCESS              The attributes
> were cleared for the
> +                                      memory region.
>    @retval RETURN_INVALID_PARAMETER    Number of pages
> is zero.
> -  @retval RETURN_UNSUPPORTED          Setting the
> memory encyrption attribute is
> -                                      not supported
> +  @retval RETURN_UNSUPPORTED          Setting the
> memory encyrption attribute
> +                                      is not supported
>  **/
>  RETURN_STATUS
>  EFIAPI
>  InternalMemEncryptSevSetMemoryEncrypted (
>    IN  PHYSICAL_ADDRESS     Cr3BaseAddress,
>    IN  PHYSICAL_ADDRESS     PhysicalAddress,
>    IN  UINT64               Length,
>    IN  BOOLEAN              CacheFlush
>    );
> 
>  #endif
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptS
> evLib.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptS
> evLib.c
> index a2ea99019917..d1130df2d0e7 100644
> ---
> a/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptS
> evLib.c
> +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/Ia32/MemEncryptS
> evLib.c
> @@ -1,84 +1,90 @@
>  /** @file
> 
>    Secure Encrypted Virtualization (SEV) library helper
> function
> 
>    Copyright (c) 2017, AMD Incorporated. All rights
> reserved.<BR>
> 
> -  This program and the accompanying materials
> -  are licensed and made available under the terms and
> conditions of the BSD
> -  License which accompanies this distribution.  The
> full text of the license may
> -  be found at http://opensource.org/licenses/bsd-
> license.php
> +  This program and the accompanying materials are
> licensed and made available
> +  under the terms and conditions of the BSD License
> which accompanies this
> +  distribution.  The full text of the license may be
> found at
> +  http://opensource.org/licenses/bsd-license.php
> 
>    THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS,
>    WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> 
>  **/
> 
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Register/Cpuid.h>
>  #include <Register/Amd/Cpuid.h>
>  #include <Register/Amd/Msr.h>
>  #include <Library/MemEncryptSevLib.h>
> 
>  /**
>    This function clears memory encryption bit for the
> memory region specified
>    by BaseAddress and Number of pages from the current
> page table context.
> 
> -  @param[in]  Cr3BaseAddress        Cr3 Base Address
> (if zero then use current CR3)
> -  @param[in]  BaseAddress           The physical
> address that is the start address
> -                                    of a memory
> region.
> -  @param[in]  NumberOfPages         The number of
> pages from start memory region.
> +  @param[in]  Cr3BaseAddress        Cr3 Base Address
> (if zero then use current
> +                                    CR3)
> +  @param[in]  BaseAddress           The physical
> address that is the start
> +                                    address of a
> memory region.
> +  @param[in]  NumberOfPages         The number of
> pages from start memory
> +                                    region.
>    @param[in]  Flush                 Flush the caches
> before clearing the bit
>                                      (mostly TRUE
> except MMIO addresses)
> 
> -  @retval RETURN_SUCCESS            The attributes
> were cleared for the memory region.
> +  @retval RETURN_SUCCESS            The attributes
> were cleared for the memory
> +                                    region.
>    @retval RETURN_INVALID_PARAMETER  Number of pages is
> zero.
>    @retval RETURN_UNSUPPORTED        Clearing memory
> encryption attribute is not
>                                      supported
>    **/
>  RETURN_STATUS
>  EFIAPI
>  MemEncryptSevClearPageEncMask (
>    IN PHYSICAL_ADDRESS         Cr3BaseAddress,
>    IN PHYSICAL_ADDRESS         BaseAddress,
>    IN UINTN                    NumberOfPages,
>    IN BOOLEAN                  Flush
>    )
>  {
>    //
>    // Memory encryption bit is not accessible in 32-bit
> mode
>    //
>    return RETURN_UNSUPPORTED;
>  }
> 
>  /**
>    This function sets memory encryption bit for the
> memory region specified by
>    BaseAddress and Number of pages from the current
> page table context.
> 
> -  @param[in]  Cr3BaseAddress        Cr3 Base Address
> (if zero then use current CR3)
> -  @param[in]  BaseAddress           The physical
> address that is the start address
> -                                    of a memory
> region.
> -  @param[in]  NumberOfPages         The number of
> pages from start memory region.
> +  @param[in]  Cr3BaseAddress        Cr3 Base Address
> (if zero then use current
> +                                    CR3)
> +  @param[in]  BaseAddress           The physical
> address that is the start
> +                                    address of a
> memory region.
> +  @param[in]  NumberOfPages         The number of
> pages from start memory
> +                                    region.
>    @param[in]  Flush                 Flush the caches
> before clearing the bit
>                                      (mostly TRUE
> except MMIO addresses)
> 
> -  @retval RETURN_SUCCESS            The attributes
> were set for the memory region.
> +  @retval RETURN_SUCCESS            The attributes
> were set for the memory
> +                                    region.
>    @retval RETURN_INVALID_PARAMETER  Number of pages is
> zero.
>    @retval RETURN_UNSUPPORTED        Clearing memory
> encryption attribute is not
>                                      supported
>    **/
>  RETURN_STATUS
>  EFIAPI
>  MemEncryptSevSetPageEncMask (
>    IN PHYSICAL_ADDRESS         Cr3BaseAddress,
>    IN PHYSICAL_ADDRESS         BaseAddress,
>    IN UINTN                    NumberOfPages,
>    IN BOOLEAN                  Flush
>    )
>  {
>    //
>    // Memory encryption bit is not accessible in 32-bit
> mode
>    //
>    return RETURN_UNSUPPORTED;
>  }
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLib
> Internal.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLib
> Internal.c
> index 002f079c7eb3..ff561236d819 100644
> ---
> a/OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLib
> Internal.c
> +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/MemEncryptSevLib
> Internal.c
> @@ -1,30 +1,30 @@
>  /** @file
> 
>    Secure Encrypted Virtualization (SEV) library helper
> function
> 
>    Copyright (c) 2017, AMD Incorporated. All rights
> reserved.<BR>
> 
> -  This program and the accompanying materials
> -  are licensed and made available under the terms and
> conditions of the BSD
> -  License which accompanies this distribution.  The
> full text of the license may
> -  be found at http://opensource.org/licenses/bsd-
> license.php
> +  This program and the accompanying materials are
> licensed and made available
> +  under the terms and conditions of the BSD License
> which accompanies this
> +  distribution.  The full text of the license may be
> found at
> +  http://opensource.org/licenses/bsd-license.php
> 
>    THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS,
>    WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> 
>  **/
> 
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Register/Cpuid.h>
>  #include <Register/Amd/Cpuid.h>
>  #include <Register/Amd/Msr.h>
>  #include <Library/MemEncryptSevLib.h>
> 
>  STATIC BOOLEAN mSevStatus = FALSE;
>  STATIC BOOLEAN mSevStatusChecked = FALSE;
> 
>  /**
> 
>    Returns a boolean to indicate whether SEV is enabled
> 
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSe
> vLib.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSe
> vLib.c
> index 9ec76708bd7b..4b7fdf7d044d 100644
> ---
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSe
> vLib.c
> +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/MemEncryptSe
> vLib.c
> @@ -1,84 +1,98 @@
>  /** @file
> 
>    Secure Encrypted Virtualization (SEV) library helper
> function
> 
>    Copyright (c) 2017, AMD Incorporated. All rights
> reserved.<BR>
> 
> -  This program and the accompanying materials
> -  are licensed and made available under the terms and
> conditions of the BSD
> -  License which accompanies this distribution.  The
> full text of the license may
> -  be found at http://opensource.org/licenses/bsd-
> license.php
> +  This program and the accompanying materials are
> licensed and made available
> +  under the terms and conditions of the BSD License
> which accompanies this
> +  distribution.  The full text of the license may be
> found at
> +  http://opensource.org/licenses/bsd-license.php
> 
>    THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS,
>    WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> 
>  **/
> 
>  #include <Library/BaseLib.h>
>  #include <Library/DebugLib.h>
>  #include <Register/Cpuid.h>
>  #include <Register/Amd/Cpuid.h>
>  #include <Register/Amd/Msr.h>
>  #include <Library/MemEncryptSevLib.h>
> 
>  #include "VirtualMemory.h"
> 
>  /**
> 
>    This function clears memory encryption bit for the
> memory region specified by
>    BaseAddress and Number of pages from the current
> page table context.
> 
> -  @param[in]  Cr3BaseAddress          Cr3 Base Address
> (if zero then use current CR3)
> -  @param[in]  BaseAddress             The physical
> address that is the start address
> -                                      of a memory
> region.
> -  @param[in]  NumberOfPages           The number of
> pages from start memory region.
> +  @param[in]  Cr3BaseAddress          Cr3 Base Address
> (if zero then use
> +                                      current CR3)
> +  @param[in]  BaseAddress             The physical
> address that is the start
> +                                      address of a
> memory region.
> +  @param[in]  NumberOfPages           The number of
> pages from start memory
> +                                      region.
>    @param[in]  Flush                   Flush the caches
> before clearing the bit
>                                        (mostly TRUE
> except MMIO addresses)
> 
> -  @retval RETURN_SUCCESS              The attributes
> were cleared for the memory
> -                                      region.
> +  @retval RETURN_SUCCESS              The attributes
> were cleared for the
> +                                      memory region.
>    @retval RETURN_INVALID_PARAMETER    Number of pages
> is zero.
> -  @retval RETURN_UNSUPPORTED          Clearing the
> memory encryption attribute is
> -                                      not supported
> +  @retval RETURN_UNSUPPORTED          Clearing the
> memory encryption attribute
> +                                      is not supported
>    **/
>  RETURN_STATUS
>  EFIAPI
>  MemEncryptSevClearPageEncMask (
>    IN PHYSICAL_ADDRESS         Cr3BaseAddress,
>    IN PHYSICAL_ADDRESS         BaseAddress,
>    IN UINTN                    NumPages,
>    IN BOOLEAN                  Flush
>    )
>  {
> -  return InternalMemEncryptSevSetMemoryDecrypted
> (Cr3BaseAddress, BaseAddress,
> EFI_PAGES_TO_SIZE(NumPages), Flush);
> +  return InternalMemEncryptSevSetMemoryDecrypted (
> +           Cr3BaseAddress,
> +           BaseAddress,
> +           EFI_PAGES_TO_SIZE (NumPages),
> +           Flush
> +           );
>  }
> 
>  /**
> 
>    This function clears memory encryption bit for the
> memory region specified by
>    BaseAddress and Number of pages from the current
> page table context.
> 
> -  @param[in]  Cr3BaseAddress          Cr3 Base Address
> (if zero then use current CR3)
> -  @param[in]  BaseAddress             The physical
> address that is the start address
> -                                      of a memory
> region.
> -  @param[in]  NumberOfPages           The number of
> pages from start memory region.
> +  @param[in]  Cr3BaseAddress          Cr3 Base Address
> (if zero then use
> +                                      current CR3)
> +  @param[in]  BaseAddress             The physical
> address that is the start
> +                                      address of a
> memory region.
> +  @param[in]  NumberOfPages           The number of
> pages from start memory
> +                                      region.
>    @param[in]  Flush                   Flush the caches
> before clearing the bit
>                                        (mostly TRUE
> except MMIO addresses)
> 
> -  @retval RETURN_SUCCESS              The attributes
> were cleared for the memory
> -                                      region.
> +  @retval RETURN_SUCCESS              The attributes
> were cleared for the
> +                                      memory region.
>    @retval RETURN_INVALID_PARAMETER    Number of pages
> is zero.
> -  @retval RETURN_UNSUPPORTED          Clearing the
> memory encryption attribute is
> -                                      not supported
> +  @retval RETURN_UNSUPPORTED          Clearing the
> memory encryption attribute
> +                                      is not supported
>    **/
>  RETURN_STATUS
>  EFIAPI
>  MemEncryptSevSetPageEncMask (
>    IN PHYSICAL_ADDRESS         Cr3BaseAddress,
>    IN PHYSICAL_ADDRESS         BaseAddress,
>    IN UINTN                    NumPages,
>    IN BOOLEAN                  Flush
>    )
>  {
> -  return InternalMemEncryptSevSetMemoryEncrypted
> (Cr3BaseAddress, BaseAddress,
> EFI_PAGES_TO_SIZE(NumPages), Flush);
> +  return InternalMemEncryptSevSetMemoryEncrypted (
> +           Cr3BaseAddress,
> +           BaseAddress,
> +           EFI_PAGES_TO_SIZE (NumPages),
> +           Flush
> +           );
>  }
> diff --git
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemor
> y.c
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemor
> y.c
> index 4185874c99b8..65b8babaac44 100644
> ---
> a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemor
> y.c
> +++
> b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemor
> y.c
> @@ -1,36 +1,36 @@
>  /** @file
> 
>    Virtual Memory Management Services to set or clear
> the memory encryption bit
> 
> -Copyright (c) 2006 - 2016, Intel Corporation. All
> rights reserved.<BR>
> -Copyright (c) 2017, AMD Incorporated. All rights
> reserved.<BR>
> +  Copyright (c) 2006 - 2016, Intel Corporation. All
> rights reserved.<BR>
> +  Copyright (c) 2017, AMD Incorporated. All rights
> reserved.<BR>
> 
> -This program and the accompanying materials
> -are licensed and made available under the terms and
> conditions of the BSD License
> -which accompanies this distribution.  The full text of
> the license may be found at
> -http://opensource.org/licenses/bsd-license.php
> +  This program and the accompanying materials are
> licensed and made available
> +  under the terms and conditions of the BSD License
> which accompanies this
> +  distribution.  The full text of the license may be
> found at
> +  http://opensource.org/licenses/bsd-license.php
> 
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN
> "AS IS" BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> EITHER EXPRESS OR IMPLIED.
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON
> AN "AS IS" BASIS, WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> 
> -Code is derived from
> MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> +  Code is derived from
> MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> 
>  **/
> 
>  #include <Library/CpuLib.h>
>  #include <Register/Cpuid.h>
>  #include <Register/Amd/Cpuid.h>
> 
>  #include "VirtualMemory.h"
> 
>  STATIC BOOLEAN mAddressEncMaskChecked = FALSE;
>  STATIC UINT64  mAddressEncMask;
>  STATIC PAGE_TABLE_POOL   *mPageTablePool = NULL;
> 
>  typedef enum {
>     SetCBit,
>     ClearCBit
>  } MAP_RANGE_MODE;
> 
>  /**
>    Get the memory encryption mask
> @@ -52,45 +52,46 @@ GetMemEncryptionAddressMask (
>    }
> 
>    //
>    // CPUID Fn8000_001F[EBX] Bit 0:5 (memory encryption
> bit position)
>    //
>    AsmCpuid (CPUID_MEMORY_ENCRYPTION_INFO, NULL,
> &Ebx.Uint32, NULL, NULL);
>    EncryptionMask = LShiftU64 (1, Ebx.Bits.PtePosBits);
> 
>    mAddressEncMask = EncryptionMask &
> PAGING_1G_ADDRESS_MASK_64;
>    mAddressEncMaskChecked = TRUE;
> 
>    return mAddressEncMask;
>  }
> 
>  /**
>    Initialize a buffer pool for page table use only.
> 
>    To reduce the potential split operation on page
> table, the pages reserved for
>    page table should be allocated in the times of
> PAGE_TABLE_POOL_UNIT_PAGES and
>    at the boundary of PAGE_TABLE_POOL_ALIGNMENT. So the
> page pool is always
> -  initialized with number of pages greater than or
> equal to the given PoolPages.
> +  initialized with number of pages greater than or
> equal to the given
> +  PoolPages.
> 
>    Once the pages in the pool are used up, this method
> should be called again to
> -  reserve at least another PAGE_TABLE_POOL_UNIT_PAGES.
> Usually this won't happen
> -  often in practice.
> +  reserve at least another PAGE_TABLE_POOL_UNIT_PAGES.
> Usually this won't
> +  happen often in practice.
> 
>    @param[in] PoolPages      The least page number of
> the pool to be created.
> 
>    @retval TRUE    The pool is initialized
> successfully.
>    @retval FALSE   The memory is out of resource.
>  **/
>  STATIC
>  BOOLEAN
>  InitializePageTablePool (
>    IN  UINTN                           PoolPages
>    )
>  {
>    VOID                      *Buffer;
> 
>    //
>    // Always reserve at least
> PAGE_TABLE_POOL_UNIT_PAGES, including one page for
>    // header.
>    //
>    PoolPages += 1;   // Add one page for header.
>    PoolPages = ((PoolPages - 1) /
> PAGE_TABLE_POOL_UNIT_PAGES + 1) *
> @@ -166,89 +167,96 @@ AllocatePageTableMemory (
> 
>    mPageTablePool->Offset     += EFI_PAGES_TO_SIZE
> (Pages);
>    mPageTablePool->FreePages  -= Pages;
> 
>    DEBUG ((
>      DEBUG_VERBOSE,
>      "%a:%a: Buffer=0x%Lx Pages=%ld\n",
>      gEfiCallerBaseName,
>      __FUNCTION__,
>      Buffer,
>      Pages
>      ));
> 
>    return Buffer;
>  }
> 
> 
>  /**
>    Split 2M page to 4K.
> 
> -  @param[in]      PhysicalAddress       Start physical
> address the 2M page covered.
> +  @param[in]      PhysicalAddress       Start physical
> address the 2M page
> +                                        covered.
>    @param[in, out] PageEntry2M           Pointer to 2M
> page entry.
>    @param[in]      StackBase             Stack base
> address.
>    @param[in]      StackSize             Stack size.
> 
>  **/
>  STATIC
>  VOID
>  Split2MPageTo4K (
>    IN        PHYSICAL_ADDRESS
> PhysicalAddress,
>    IN  OUT   UINT64
> *PageEntry2M,
>    IN        PHYSICAL_ADDRESS               StackBase,
>    IN        UINTN                          StackSize
>    )
>  {
>    PHYSICAL_ADDRESS                  PhysicalAddress4K;
>    UINTN
> IndexOfPageTableEntries;
>    PAGE_TABLE_4K_ENTRY               *PageTableEntry,
> *PageTableEntry1;
>    UINT64                            AddressEncMask;
> 
>    PageTableEntry = AllocatePageTableMemory(1);
> 
>    PageTableEntry1 = PageTableEntry;
> 
>    AddressEncMask = GetMemEncryptionAddressMask ();
> 
>    ASSERT (PageTableEntry != NULL);
>    ASSERT (*PageEntry2M & AddressEncMask);
> 
>    PhysicalAddress4K = PhysicalAddress;
> -  for (IndexOfPageTableEntries = 0;
> IndexOfPageTableEntries < 512;
> IndexOfPageTableEntries++, PageTableEntry++,
> PhysicalAddress4K += SIZE_4KB) {
> +  for (IndexOfPageTableEntries = 0;
> +       IndexOfPageTableEntries < 512;
> +       (IndexOfPageTableEntries++,
> +        PageTableEntry++,
> +        PhysicalAddress4K += SIZE_4KB)) {
>      //
>      // Fill in the Page Table entries
>      //
>      PageTableEntry->Uint64 = (UINT64)
> PhysicalAddress4K | AddressEncMask;
>      PageTableEntry->Bits.ReadWrite = 1;
>      PageTableEntry->Bits.Present = 1;
> -    if ((PhysicalAddress4K >= StackBase) &&
> (PhysicalAddress4K < StackBase + StackSize)) {
> +    if ((PhysicalAddress4K >= StackBase) &&
> +        (PhysicalAddress4K < StackBase + StackSize)) {
>        //
>        // Set Nx bit for stack.
>        //
>        PageTableEntry->Bits.Nx = 1;
>      }
>    }
> 
>    //
>    // Fill in 2M page entry.
>    //
> -  *PageEntry2M = (UINT64) (UINTN) PageTableEntry1 |
> IA32_PG_P | IA32_PG_RW | AddressEncMask;
> +  *PageEntry2M = ((UINT64)(UINTN)PageTableEntry1 |
> +                  IA32_PG_P | IA32_PG_RW |
> AddressEncMask);
>  }
> 
>  /**
>    Set one page of page table pool memory to be read-
> only.
> 
>    @param[in] PageTableBase    Base address of page
> table (CR3).
>    @param[in] Address          Start address of a page
> to be set as read-only.
>    @param[in] Level4Paging     Level 4 paging flag.
> 
>  **/
>  STATIC
>  VOID
>  SetPageTablePoolReadOnly (
>    IN  UINTN                             PageTableBase,
>    IN  EFI_PHYSICAL_ADDRESS              Address,
>    IN  BOOLEAN                           Level4Paging
>    )
>  {
>    UINTN                 Index;
>    UINTN                 EntryIndex;
> @@ -374,96 +382,108 @@ EnablePageTableProtection (
>    PAGE_TABLE_POOL         *HeadPool;
>    PAGE_TABLE_POOL         *Pool;
>    UINT64                  PoolSize;
>    EFI_PHYSICAL_ADDRESS    Address;
> 
>    if (mPageTablePool == NULL) {
>      return;
>    }
> 
>    //
>    // SetPageTablePoolReadOnly might update
> mPageTablePool. It's safer to
>    // remember original one in advance.
>    //
>    HeadPool = mPageTablePool;
>    Pool = HeadPool;
>    do {
>      Address  = (EFI_PHYSICAL_ADDRESS)(UINTN)Pool;
>      PoolSize = Pool->Offset + EFI_PAGES_TO_SIZE (Pool-
> >FreePages);
> 
>      //
> -    // The size of one pool must be multiple of
> PAGE_TABLE_POOL_UNIT_SIZE, which
> -    // is one of page size of the processor (2MB by
> default). Let's apply the
> -    // protection to them one by one.
> +    // The size of one pool must be multiple of
> PAGE_TABLE_POOL_UNIT_SIZE,
> +    // which is one of page size of the processor (2MB
> by default). Let's apply
> +    // the protection to them one by one.
>      //
>      while (PoolSize > 0) {
>        SetPageTablePoolReadOnly(PageTableBase, Address,
> Level4Paging);
>        Address   += PAGE_TABLE_POOL_UNIT_SIZE;
>        PoolSize  -= PAGE_TABLE_POOL_UNIT_SIZE;
>      }
> 
>      Pool = Pool->NextPool;
>    } while (Pool != HeadPool);
> 
>  }
> 
> 
>  /**
>    Split 1G page to 2M.
> 
> -  @param[in]      PhysicalAddress       Start physical
> address the 1G page covered.
> +  @param[in]      PhysicalAddress       Start physical
> address the 1G page
> +                                        covered.
>    @param[in, out] PageEntry1G           Pointer to 1G
> page entry.
>    @param[in]      StackBase             Stack base
> address.
>    @param[in]      StackSize             Stack size.
> 
>  **/
>  STATIC
>  VOID
>  Split1GPageTo2M (
>    IN          PHYSICAL_ADDRESS
> PhysicalAddress,
>    IN  OUT     UINT64
> *PageEntry1G,
>    IN          PHYSICAL_ADDRESS
> StackBase,
>    IN          UINTN                          StackSize
>    )
>  {
>    PHYSICAL_ADDRESS                  PhysicalAddress2M;
>    UINTN
> IndexOfPageDirectoryEntries;
>    PAGE_TABLE_ENTRY
> *PageDirectoryEntry;
>    UINT64                            AddressEncMask;
> 
>    PageDirectoryEntry = AllocatePageTableMemory(1);
> 
>    AddressEncMask = GetMemEncryptionAddressMask ();
>    ASSERT (PageDirectoryEntry != NULL);
>    ASSERT (*PageEntry1G & GetMemEncryptionAddressMask
> ());
>    //
>    // Fill in 1G page entry.
>    //
> -  *PageEntry1G = (UINT64) (UINTN) PageDirectoryEntry |
> IA32_PG_P | IA32_PG_RW | AddressEncMask;
> +  *PageEntry1G = ((UINT64)(UINTN)PageDirectoryEntry |
> +                  IA32_PG_P | IA32_PG_RW |
> AddressEncMask);
> 
>    PhysicalAddress2M = PhysicalAddress;
> -  for (IndexOfPageDirectoryEntries = 0;
> IndexOfPageDirectoryEntries < 512;
> IndexOfPageDirectoryEntries++, PageDirectoryEntry++,
> PhysicalAddress2M += SIZE_2MB) {
> -    if ((PhysicalAddress2M < StackBase + StackSize) &&
> ((PhysicalAddress2M + SIZE_2MB) > StackBase)) {
> +  for (IndexOfPageDirectoryEntries = 0;
> +       IndexOfPageDirectoryEntries < 512;
> +       (IndexOfPageDirectoryEntries++,
> +        PageDirectoryEntry++,
> +        PhysicalAddress2M += SIZE_2MB)) {
> +    if ((PhysicalAddress2M < StackBase + StackSize) &&
> +        ((PhysicalAddress2M + SIZE_2MB) > StackBase))
> {
>        //
>        // Need to split this 2M page that covers stack
> range.
>        //
> -      Split2MPageTo4K (PhysicalAddress2M, (UINT64 *)
> PageDirectoryEntry, StackBase, StackSize);
> +      Split2MPageTo4K (
> +        PhysicalAddress2M,
> +        (UINT64 *)PageDirectoryEntry,
> +        StackBase,
> +        StackSize
> +        );
>      } else {
>        //
>        // Fill in the Page Directory entries
>        //
>        PageDirectoryEntry->Uint64 = (UINT64)
> PhysicalAddress2M | AddressEncMask;
>        PageDirectoryEntry->Bits.ReadWrite = 1;
>        PageDirectoryEntry->Bits.Present = 1;
>        PageDirectoryEntry->Bits.MustBe1 = 1;
>      }
>    }
>  }
> 
> 
>  /**
>    Set or Clear the memory encryption bit
> 
>    @param[in]      PagetablePoint        Page table
> entry pointer (PTE).
>    @param[in]      Mode                  Set or Clear
> encryption bit
> 
>  **/
> @@ -510,62 +530,63 @@ VOID
>  DisableReadOnlyPageWriteProtect (
>    VOID
>    )
>  {
>    AsmWriteCr0 (AsmReadCr0() & ~BIT16);
>  }
> 
>  /**
>   Enable Write Protect on pages marked as read-only.
>  **/
>  VOID
>  EnableReadOnlyPageWriteProtect (
>    VOID
>    )
>  {
>    AsmWriteCr0 (AsmReadCr0() | BIT16);
>  }
> 
> 
>  /**
> -  This function either sets or clears memory
> encryption bit for the memory region
> -  specified by PhysicalAddress and length from the
> current page table context.
> +  This function either sets or clears memory
> encryption bit for the memory
> +  region specified by PhysicalAddress and length from
> the current page table
> +  context.
> 
>    The function iterates through the physicalAddress
> one page at a time, and set
>    or clears the memory encryption mask in the page
> table. If it encounters
>    that a given physical address range is part of large
> page then it attempts to
>    change the attribute at one go (based on size),
> otherwise it splits the
>    large pages into smaller (e.g 2M page into 4K pages)
> and then try to set or
>    clear the encryption bit on the smallest page size.
> 
>    @param[in]  PhysicalAddress         The physical
> address that is the start
>                                        address of a
> memory region.
>    @param[in]  Length                  The length of
> memory region
>    @param[in]  Mode                    Set or Clear
> mode
>    @param[in]  Flush                   Flush the caches
> before applying the
>                                        encryption mask
> 
> -  @retval RETURN_SUCCESS              The attributes
> were cleared for the memory
> -                                      region.
> +  @retval RETURN_SUCCESS              The attributes
> were cleared for the
> +                                      memory region.
>    @retval RETURN_INVALID_PARAMETER    Number of pages
> is zero.
> -  @retval RETURN_UNSUPPORTED          Setting the
> memory encyrption attribute is
> -                                      not supported
> +  @retval RETURN_UNSUPPORTED          Setting the
> memory encyrption attribute
> +                                      is not supported
>  **/
> 
>  STATIC
>  RETURN_STATUS
>  EFIAPI
>  SetMemoryEncDec (
>    IN    PHYSICAL_ADDRESS         Cr3BaseAddress,
>    IN    PHYSICAL_ADDRESS         PhysicalAddress,
>    IN    UINTN                    Length,
>    IN    MAP_RANGE_MODE           Mode,
>    IN    BOOLEAN                  CacheFlush
>    )
>  {
>    PAGE_MAP_AND_DIRECTORY_POINTER *PageMapLevel4Entry;
>    PAGE_MAP_AND_DIRECTORY_POINTER
> *PageUpperDirectoryPointerEntry;
>    PAGE_MAP_AND_DIRECTORY_POINTER
> *PageDirectoryPointerEntry;
>    PAGE_TABLE_1G_ENTRY
> *PageDirectory1GEntry;
>    PAGE_TABLE_ENTRY
> *PageDirectory2MEntry;
>    PAGE_TABLE_4K_ENTRY            *PageTableEntry;
>    UINT64                         PgTableMask;
> @@ -584,81 +605,84 @@ SetMemoryEncDec (
>      (Mode == SetCBit) ? "Encrypt" : "Decrypt",
>      (UINT32)CacheFlush
>      ));
> 
>    //
>    // Check if we have a valid memory encryption mask
>    //
>    AddressEncMask = GetMemEncryptionAddressMask ();
>    if (!AddressEncMask) {
>      return RETURN_ACCESS_DENIED;
>    }
> 
>    PgTableMask = AddressEncMask | EFI_PAGE_MASK;
> 
>    if (Length == 0) {
>      return RETURN_INVALID_PARAMETER;
>    }
> 
>    //
>    // We are going to change the memory encryption
> attribute from C=0 -> C=1 or
> -  // vice versa Flush the caches to ensure that data
> is written into memory with
> -  // correct C-bit
> +  // vice versa Flush the caches to ensure that data
> is written into memory
> +  // with correct C-bit
>    //
>    if (CacheFlush) {
>      WriteBackInvalidateDataCacheRange((VOID*)
> (UINTN)PhysicalAddress, Length);
>    }
> 
>    //
>    // Make sure that the page table is changeable.
>    //
>    IsWpEnabled = IsReadOnlyPageWriteProtected ();
>    if (IsWpEnabled) {
>      DisableReadOnlyPageWriteProtect ();
>    }
> 
>    Status = EFI_SUCCESS;
> 
>    while (Length)
>    {
>      //
>      // If Cr3BaseAddress is not specified then read
> the current CR3
>      //
>      if (Cr3BaseAddress == 0) {
>        Cr3BaseAddress = AsmReadCr3();
>      }
> 
>      PageMapLevel4Entry = (VOID*) (Cr3BaseAddress &
> ~PgTableMask);
>      PageMapLevel4Entry +=
> PML4_OFFSET(PhysicalAddress);
>      if (!PageMapLevel4Entry->Bits.Present) {
>        DEBUG ((
>          DEBUG_ERROR,
>          "%a:%a: bad PML4 for Physical=0x%Lx\n",
>          gEfiCallerBaseName,
>          __FUNCTION__,
>          PhysicalAddress
>          ));
>        Status = RETURN_NO_MAPPING;
>        goto Done;
>      }
> 
> -    PageDirectory1GEntry = (VOID*)
> ((PageMapLevel4Entry->Bits.PageTableBaseAddress<<12) &
> ~PgTableMask);
> +    PageDirectory1GEntry = (VOID *)(
> +                             (PageMapLevel4Entry-
> >Bits.PageTableBaseAddress <<
> +                              12) & ~PgTableMask
> +                             );
>      PageDirectory1GEntry +=
> PDP_OFFSET(PhysicalAddress);
>      if (!PageDirectory1GEntry->Bits.Present) {
>        DEBUG ((
>          DEBUG_ERROR,
>          "%a:%a: bad PDPE for Physical=0x%Lx\n",
>          gEfiCallerBaseName,
>          __FUNCTION__,
>          PhysicalAddress
>          ));
>        Status = RETURN_NO_MAPPING;
>        goto Done;
>      }
> 
>      //
>      // If the MustBe1 bit is not 1, it's not actually
> a 1GB entry
>      //
>      if (PageDirectory1GEntry->Bits.MustBe1) {
>        //
>        // Valid 1GB page
>        // If we have at least 1GB to go, we can just
> update this entry
> @@ -668,90 +692,110 @@ SetMemoryEncDec (
>          DEBUG ((
>            DEBUG_VERBOSE,
>            "%a:%a: updated 1GB entry for
> Physical=0x%Lx\n",
>            gEfiCallerBaseName,
>            __FUNCTION__,
>            PhysicalAddress
>            ));
>          PhysicalAddress += BIT30;
>          Length -= BIT30;
>        } else {
>          //
>          // We must split the page
>          //
>          DEBUG ((
>            DEBUG_VERBOSE,
>            "%a:%a: splitting 1GB page for
> Physical=0x%Lx\n",
>            gEfiCallerBaseName,
>            __FUNCTION__,
>            PhysicalAddress
>            ));
> -        Split1GPageTo2M(((UINT64)PageDirectory1GEntry-
> >Bits.PageTableBaseAddress)<<30, (UINT64*)
> PageDirectory1GEntry, 0, 0);
> +        Split1GPageTo2M (
> +          (UINT64)PageDirectory1GEntry-
> >Bits.PageTableBaseAddress << 30,
> +          (UINT64 *)PageDirectory1GEntry,
> +          0,
> +          0
> +          );
>          continue;
>        }
>      } else {
>        //
>        // Actually a PDP
>        //
> -      PageUpperDirectoryPointerEntry =
> (PAGE_MAP_AND_DIRECTORY_POINTER*) PageDirectory1GEntry;
> -      PageDirectory2MEntry = (VOID*)
> ((PageUpperDirectoryPointerEntry-
> >Bits.PageTableBaseAddress<<12) & ~PgTableMask);
> +      PageUpperDirectoryPointerEntry =
> +        (PAGE_MAP_AND_DIRECTORY_POINTER
> *)PageDirectory1GEntry;
> +      PageDirectory2MEntry =
> +        (VOID *)(
> +          (PageUpperDirectoryPointerEntry-
> >Bits.PageTableBaseAddress <<
> +           12) & ~PgTableMask
> +          );
>        PageDirectory2MEntry +=
> PDE_OFFSET(PhysicalAddress);
>        if (!PageDirectory2MEntry->Bits.Present) {
>          DEBUG ((
>            DEBUG_ERROR,
>            "%a:%a: bad PDE for Physical=0x%Lx\n",
>            gEfiCallerBaseName,
>            __FUNCTION__,
>            PhysicalAddress
>            ));
>          Status = RETURN_NO_MAPPING;
>          goto Done;
>        }
>        //
>        // If the MustBe1 bit is not a 1, it's not a 2MB
> entry
>        //
>        if (PageDirectory2MEntry->Bits.MustBe1) {
>          //
>          // Valid 2MB page
>          // If we have at least 2MB left to go, we can
> just update this entry
>          //
>          if (!(PhysicalAddress & (BIT21-1)) && Length
> >= BIT21) {
>            SetOrClearCBit (&PageDirectory2MEntry-
> >Uint64, Mode);
>            PhysicalAddress += BIT21;
>            Length -= BIT21;
>          } else {
>            //
>            // We must split up this page into 4K pages
>            //
>            DEBUG ((
>              DEBUG_VERBOSE,
>              "%a:%a: splitting 2MB page for
> Physical=0x%Lx\n",
>              gEfiCallerBaseName,
>              __FUNCTION__,
>              PhysicalAddress
>              ));
> -          Split2MPageTo4K
> (((UINT64)PageDirectory2MEntry-
> >Bits.PageTableBaseAddress) << 21, (UINT64*)
> PageDirectory2MEntry, 0, 0);
> +          Split2MPageTo4K (
> +            (UINT64)PageDirectory2MEntry-
> >Bits.PageTableBaseAddress << 21,
> +            (UINT64 *)PageDirectory2MEntry,
> +            0,
> +            0
> +            );
>            continue;
>          }
>        } else {
> -        PageDirectoryPointerEntry =
> (PAGE_MAP_AND_DIRECTORY_POINTER*) PageDirectory2MEntry;
> -        PageTableEntry = (VOID*)
> (PageDirectoryPointerEntry-
> >Bits.PageTableBaseAddress<<12 & ~PgTableMask);
> +        PageDirectoryPointerEntry =
> +          (PAGE_MAP_AND_DIRECTORY_POINTER
> *)PageDirectory2MEntry;
> +        PageTableEntry =
> +          (VOID *)(
> +            (PageDirectoryPointerEntry-
> >Bits.PageTableBaseAddress <<
> +             12) & ~PgTableMask
> +            );
>          PageTableEntry += PTE_OFFSET(PhysicalAddress);
>          if (!PageTableEntry->Bits.Present) {
>            DEBUG ((
>              DEBUG_ERROR,
>              "%a:%a: bad PTE for Physical=0x%Lx\n",
>              gEfiCallerBaseName,
>              __FUNCTION__,
>              PhysicalAddress
>              ));
>            Status = RETURN_NO_MAPPING;
>            goto Done;
>          }
>          SetOrClearCBit (&PageTableEntry->Uint64,
> Mode);
>          PhysicalAddress += EFI_PAGE_SIZE;
>          Length -= EFI_PAGE_SIZE;
>        }
>      }
>    }
> 
>    //
> @@ -771,66 +815,78 @@ Done:
>    //
>    // Restore page table write protection, if any.
>    //
>    if (IsWpEnabled) {
>      EnableReadOnlyPageWriteProtect ();
>    }
> 
>    return Status;
>  }
> 
>  /**
>    This function clears memory encryption bit for the
> memory region specified by
>    PhysicalAddress and length from the current page
> table context.
> 
>    @param[in]  PhysicalAddress         The physical
> address that is the start
>                                        address of a
> memory region.
>    @param[in]  Length                  The length of
> memory region
>    @param[in]  Flush                   Flush the caches
> before applying the
>                                        encryption mask
> 
> -  @retval RETURN_SUCCESS              The attributes
> were cleared for the memory
> -                                      region.
> +  @retval RETURN_SUCCESS              The attributes
> were cleared for the
> +                                      memory region.
>    @retval RETURN_INVALID_PARAMETER    Number of pages
> is zero.
> -  @retval RETURN_UNSUPPORTED          Setting the
> memory encyrption attribute is
> -                                      not supported
> +  @retval RETURN_UNSUPPORTED          Setting the
> memory encyrption attribute
> +                                      is not supported
>  **/
>  RETURN_STATUS
>  EFIAPI
>  InternalMemEncryptSevSetMemoryDecrypted (
>    IN  PHYSICAL_ADDRESS        Cr3BaseAddress,
>    IN  PHYSICAL_ADDRESS        PhysicalAddress,
>    IN  UINTN                   Length,
>    IN  BOOLEAN                 Flush
>    )
>  {
> 
> -  return SetMemoryEncDec (Cr3BaseAddress,
> PhysicalAddress, Length, ClearCBit, Flush);
> +  return SetMemoryEncDec (
> +           Cr3BaseAddress,
> +           PhysicalAddress,
> +           Length,
> +           ClearCBit,
> +           Flush
> +           );
>  }
> 
>  /**
>    This function sets memory encryption bit for the
> memory region specified by
>    PhysicalAddress and length from the current page
> table context.
> 
> -  @param[in]  PhysicalAddress         The physical
> address that is the start address
> -                                      of a memory
> region.
> +  @param[in]  PhysicalAddress         The physical
> address that is the start
> +                                      address of a
> memory region.
>    @param[in]  Length                  The length of
> memory region
>    @param[in]  Flush                   Flush the caches
> before applying the
>                                        encryption mask
> 
> -  @retval RETURN_SUCCESS              The attributes
> were cleared for the memory
> -                                      region.
> +  @retval RETURN_SUCCESS              The attributes
> were cleared for the
> +                                      memory region.
>    @retval RETURN_INVALID_PARAMETER    Number of pages
> is zero.
> -  @retval RETURN_UNSUPPORTED          Setting the
> memory encyrption attribute is
> -                                      not supported
> +  @retval RETURN_UNSUPPORTED          Setting the
> memory encyrption attribute
> +                                      is not supported
>  **/
>  RETURN_STATUS
>  EFIAPI
>  InternalMemEncryptSevSetMemoryEncrypted (
>    IN  PHYSICAL_ADDRESS        Cr3BaseAddress,
>    IN  PHYSICAL_ADDRESS        PhysicalAddress,
>    IN  UINTN                   Length,
>    IN  BOOLEAN                 Flush
>    )
>  {
> -  return SetMemoryEncDec (Cr3BaseAddress,
> PhysicalAddress, Length, SetCBit, Flush);
> +  return SetMemoryEncDec (
> +           Cr3BaseAddress,
> +           PhysicalAddress,
> +           Length,
> +           SetCBit,
> +           Flush
> +           );
>  }
> --
> 2.14.1.3.gb7cf6e02401b
> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


  reply	other threads:[~2018-03-02  0:27 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-02  0:03 [PATCH 00/20] OvmfPkg: SEV: decrypt the initial SMRAM save state map for SMBASE relocation Laszlo Ersek
2018-03-02  0:03 ` [PATCH 01/20] OvmfPkg/MemEncryptSevLib: rewrap to 79 characters width Laszlo Ersek
2018-03-02  0:33   ` Kinney, Michael D [this message]
2018-03-02 11:25     ` Laszlo Ersek
2018-03-02  0:03 ` [PATCH 02/20] OvmfPkg/MemEncryptSevLib: clean up MemEncryptSevIsEnabled() decl Laszlo Ersek
2018-03-02  0:03 ` [PATCH 03/20] OvmfPkg/MemEncryptSevLib: clean up MemEncryptSevClearPageEncMask() decl Laszlo Ersek
2018-03-02  0:03 ` [PATCH 04/20] OvmfPkg/MemEncryptSevLib: clean up MemEncryptSevSetPageEncMask() decl Laszlo Ersek
2018-03-02  0:03 ` [PATCH 05/20] OvmfPkg/MemEncryptSevLib: clean up SetMemoryEncDec() comment block Laszlo Ersek
2018-03-02  0:03 ` [PATCH 06/20] OvmfPkg/MemEncryptSevLib: clean up InternalMemEncryptSevSetMemoryDecrypted() decl Laszlo Ersek
2018-03-02  0:03 ` [PATCH 07/20] OvmfPkg/MemEncryptSevLib: clean up InternalMemEncryptSevSetMemoryEncrypted() decl Laszlo Ersek
2018-03-02  0:03 ` [PATCH 08/20] OvmfPkg/MemEncryptSevLib: sort #includes, and entries in INF file sections Laszlo Ersek
2018-03-02  0:03 ` [PATCH 09/20] OvmfPkg/PlatformPei: sort #includes in "AmdSev.c" Laszlo Ersek
2018-03-02  0:03 ` [PATCH 10/20] OvmfPkg/SmmCpuFeaturesLib: rewrap to 79 columns Laszlo Ersek
2018-03-02  0:03 ` [PATCH 11/20] OvmfPkg/SmmCpuFeaturesLib: upper-case the "static" keyword Laszlo Ersek
2018-03-02  0:04 ` [PATCH 12/20] OvmfPkg/SmmCpuFeaturesLib: sort #includes, and entries in INF file sections Laszlo Ersek
2018-03-02  0:04 ` [PATCH 13/20] OvmfPkg/SmmCpuFeaturesLib: remove unneeded #includes and LibraryClasses Laszlo Ersek
2018-03-02  0:04 ` [PATCH 14/20] OvmfPkg/AmdSevDxe: rewrap to 79 characters width Laszlo Ersek
2018-03-02  0:04 ` [PATCH 15/20] OvmfPkg/AmdSevDxe: sort #includes, and entries in INF file sections Laszlo Ersek
2018-03-02  0:04 ` [PATCH 16/20] OvmfPkg/AmdSevDxe: refresh #includes and LibraryClasses Laszlo Ersek
2018-03-02  0:04 ` [PATCH 17/20] OvmfPkg/MemEncryptSevLib: find pages of initial SMRAM save state map Laszlo Ersek
2018-03-02  0:04 ` [PATCH 18/20] OvmfPkg/PlatformPei: SEV: allocate " Laszlo Ersek
2018-03-02  0:04 ` [PATCH 19/20] OvmfPkg/SmmCpuFeaturesLib: SEV: encrypt+free pages of init. " Laszlo Ersek
2018-03-02  0:04 ` [PATCH 20/20] OvmfPkg/AmdSevDxe: decrypt the pages of the initial SMRAM " Laszlo Ersek
2018-03-02  1:16 ` [PATCH 00/20] OvmfPkg: SEV: decrypt the initial SMRAM save state map for SMBASE relocation Brijesh Singh
2018-03-02 11:53   ` Laszlo Ersek
2018-03-02 13:17     ` Brijesh Singh
2018-03-05  9:55       ` Laszlo Ersek
2018-03-05 14:00       ` Laszlo Ersek
2018-03-05 14:44         ` Brijesh Singh
2018-03-05 14:47           ` Brijesh Singh
2018-03-05 21:06           ` Laszlo Ersek
2018-03-02 15:21 ` Brijesh Singh
2018-03-06 12:59   ` Laszlo Ersek

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E92EE9817A31E24EB0585FDF735412F5B896E408@ORSMSX113.amr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox