Re: [edk2-devel] [PATCH v2 4/7] OvmfPkg/IndustryStandard: Introduce PageTable.h

["Laszlo Ersek" <lersek@redhat.com>]

On 03/25/21 16:47, Anthony PERARD via groups.io wrote:
> We are going to use the page table structure in yet another place,
> collect the types and macro that can be used from another module
> rather that making yet another copy.
> 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2490
> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
> ---
> CC: Tom Lendacky <thomas.lendacky@amd.com>
> CC: Brijesh Singh <brijesh.singh@amd.com>
> ---
> 
> Notes:
>     v2:
>     - new patch
> 
>  .../IndustryStandard/PageTable.h}             | 117 +-------------
>  .../BaseMemEncryptSevLib/X64/VirtualMemory.h  | 143 +-----------------
>  2 files changed, 5 insertions(+), 255 deletions(-)
>  copy OvmfPkg/{Library/BaseMemEncryptSevLib/X64/VirtualMemory.h => Include/IndustryStandard/PageTable.h} (60%)
> 
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Include/IndustryStandard/PageTable.h
> similarity index 60%
> copy from OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> copy to OvmfPkg/Include/IndustryStandard/PageTable.h
> index 996f94f07ebb..e3da4e8cf21c 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> +++ b/OvmfPkg/Include/IndustryStandard/PageTable.h
> @@ -1,6 +1,6 @@
>  /** @file
>  
> -  Virtual Memory Management Services to set or clear the memory encryption bit
> +  x86_64 Page Tables structures
>  
>    Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
>    Copyright (c) 2017 - 2020, AMD Incorporated. All rights reserved.<BR>
> @@ -11,17 +11,10 @@
>  
>  **/
>  
> -#ifndef __VIRTUAL_MEMORY__
> -#define __VIRTUAL_MEMORY__
> +#ifndef __PAGE_TABLE_H__
> +#define __PAGE_TABLE_H__

Should be PAGE_TABLE_H_ nowadays (no leading underscore, and one
trailing underscore suffices).

Thanks
Laszlo

>  
> -#include <Library/BaseLib.h>
> -#include <Library/BaseMemoryLib.h>
> -#include <Library/CacheMaintenanceLib.h>
> -#include <Library/DebugLib.h>
> -#include <Library/MemoryAllocationLib.h>
> -#include <Uefi.h>
> -
> -#define SYS_CODE64_SEL 0x38
> +#include <Base.h>
>  
>  #pragma pack(1)
>  
> @@ -165,106 +158,4 @@ typedef union {
>  #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_ALIGN_MASK  \
> -  (~(EFI_PHYSICAL_ADDRESS)(PAGE_TABLE_POOL_ALIGNMENT - 1))
> -
> -typedef struct {
> -  VOID            *NextPool;
> -  UINTN           Offset;
> -  UINTN           FreePages;
> -} PAGE_TABLE_POOL;
> -
> -/**
> -  Return the pagetable memory encryption mask.
> -
> -  @return  The pagetable memory encryption mask.
> -
> -**/
> -UINT64
> -EFIAPI
> -InternalGetMemEncryptionAddressMask (
> -  VOID
> -  );
> -
> -/**
> -  This function clears memory encryption bit for the memory region specified by
> -  PhysicalAddress and Length from the current page table context.
> -
> -  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
> -                                      current CR3)
> -  @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_INVALID_PARAMETER    Number of pages is zero.
> -  @retval RETURN_UNSUPPORTED          Clearing 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
> -  );
> -
> -/**
> -  This function sets memory encryption bit for the memory region specified by
> -  PhysicalAddress and Length from the current page table context.
> -
> -  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
> -                                      current CR3)
> -  @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 set for the memory
> -                                      region.
> -  @retval RETURN_INVALID_PARAMETER    Number of pages is zero.
> -  @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
> -  );
> -
> -/**
> -  Returns the encryption state of the specified virtual address range.
> -
> -  @param[in]  Cr3BaseAddress          Cr3 Base Address (if zero then use
> -                                      current CR3)
> -  @param[in]  BaseAddress             Base address to check
> -  @param[in]  Length                  Length of virtual address range
> -
> -  @retval MemEncryptSevAddressRangeUnencrypted  Address range is mapped
> -                                                unencrypted
> -  @retval MemEncryptSevAddressRangeEncrypted    Address range is mapped
> -                                                encrypted
> -  @retval MemEncryptSevAddressRangeMixed        Address range is mapped mixed
> -  @retval MemEncryptSevAddressRangeError        Address range is not mapped
> -**/
> -MEM_ENCRYPT_SEV_ADDRESS_RANGE_STATE
> -EFIAPI
> -InternalMemEncryptSevGetAddressRangeState (
> -  IN PHYSICAL_ADDRESS         Cr3BaseAddress,
> -  IN PHYSICAL_ADDRESS         BaseAddress,
> -  IN UINTN                    Length
> -  );
> -
>  #endif
> diff --git a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> index 996f94f07ebb..b621d811ca6f 100644
> --- a/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> +++ b/OvmfPkg/Library/BaseMemEncryptSevLib/X64/VirtualMemory.h
> @@ -20,151 +20,10 @@
>  #include <Library/DebugLib.h>
>  #include <Library/MemoryAllocationLib.h>
>  #include <Uefi.h>
> +#include <IndustryStandard/PageTable.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  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  CacheDisabled:1;          // 0 = Cached, 1=Non-Cached
> -    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  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  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  PAT:1;                    //
> -    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
> -  } 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  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  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  MustBe1:1;                // Must be 1
> -    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
> -  } 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  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  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  MustBe1:1;                // Must be 1
> -    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
> -  } 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  \
>