Re: [PATCH 3/3] UefiCpuPkg/Universal/Acpi/S3Resume2Pei: Add support for PCD PcdPteMemoryEncryptionAddressOrMask

["Duran, Leo" <leo.duran@amd.com>]

Hi Jeff,
The new PCD is intended to be OR'ed with the address (upper bits).
Leo.

> -----Original Message-----
> From: Fan, Jeff [mailto:jeff.fan@intel.com]
> Sent: Thursday, February 09, 2017 8:23 PM
> To: Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Laszlo Ersek <lersek@redhat.com>; Singh, Brijesh <brijesh.singh@amd.com>
> Subject: RE: [PATCH 3/3] UefiCpuPkg/Universal/Acpi/S3Resume2Pei: Add
> support for PCD PcdPteMemoryEncryptionAddressOrMask
> 
> Hi Leo,
> 
> I want to understand your usage model. What fields are you going to update
> in below Page Table Entry by the new PCD?
> 
> 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;
> 
> I did not see any updating in SMM. Is it un-necessary? Is this feature working
> on POST phase or OS runtime phase?
> 
> Thanks!
> Jeff
> 
> -----Original Message-----
> From: Leo Duran [mailto:leo.duran@amd.com]
> Sent: Thursday, February 09, 2017 5:13 AM
> To: edk2-devel@ml01.01.org
> Cc: Leo Duran; Fan, Jeff; Tian, Feng; Zeng, Star; Laszlo Ersek; Brijesh Singh
> Subject: [PATCH 3/3] UefiCpuPkg/Universal/Acpi/S3Resume2Pei: Add
> support for PCD PcdPteMemoryEncryptionAddressOrMask
> 
> This PCD holds the address mask for page table entries when memory
> encryption is enabled on AMD processors supporting the Secure Encrypted
> Virtualization (SEV) feature.
> 
> The mask is applied when page tables are created (S3Resume.c).
> 
> CC: Jeff Fan <jeff.fan@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> Signed-off-by: Leo Duran <leo.duran@amd.com>
> ---
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c       | 9 +++++----
>  UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf | 2 ++
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> index d306fba..ee1e2cd 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume.c
> @@ -5,6 +5,7 @@
>    control is passed to OS waking up handler.
> 
>    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 @@ -696,7
> +697,7 @@ RestoreS3PageTables (
>        //
>        // Make a PML4 Entry
>        //
> -      PageMapLevel4Entry->Uint64 =
> (UINT64)(UINTN)PageDirectoryPointerEntry;
> +      PageMapLevel4Entry->Uint64 =
> + (UINT64)(UINTN)PageDirectoryPointerEntry | PcdGet64
> + (PcdPteMemoryEncryptionAddressOrMask);
>        PageMapLevel4Entry->Bits.ReadWrite = 1;
>        PageMapLevel4Entry->Bits.Present = 1;
> 
> @@ -707,7 +708,7 @@ RestoreS3PageTables (
>            //
>            // Fill in the Page Directory entries
>            //
> -          PageDirectory1GEntry->Uint64 = (UINT64)PageAddress;
> +          PageDirectory1GEntry->Uint64 = (UINT64)PageAddress | PcdGet64
> + (PcdPteMemoryEncryptionAddressOrMask);
>            PageDirectory1GEntry->Bits.ReadWrite = 1;
>            PageDirectory1GEntry->Bits.Present = 1;
>            PageDirectory1GEntry->Bits.MustBe1 = 1; @@ -724,7 +725,7 @@
> RestoreS3PageTables (
>            //
>            // Fill in a Page Directory Pointer Entries
>            //
> -          PageDirectoryPointerEntry->Uint64 =
> (UINT64)(UINTN)PageDirectoryEntry;
> +          PageDirectoryPointerEntry->Uint64 =
> + (UINT64)(UINTN)PageDirectoryEntry | PcdGet64
> + (PcdPteMemoryEncryptionAddressOrMask);
>            PageDirectoryPointerEntry->Bits.ReadWrite = 1;
>            PageDirectoryPointerEntry->Bits.Present = 1;
> 
> @@ -732,7 +733,7 @@ RestoreS3PageTables (
>              //
>              // Fill in the Page Directory entries
>              //
> -            PageDirectoryEntry->Uint64 = (UINT64)PageAddress;
> +            PageDirectoryEntry->Uint64 = (UINT64)PageAddress | PcdGet64
> + (PcdPteMemoryEncryptionAddressOrMask);
>              PageDirectoryEntry->Bits.ReadWrite = 1;
>              PageDirectoryEntry->Bits.Present = 1;
>              PageDirectoryEntry->Bits.MustBe1 = 1; diff --git
> a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> index 73aeca3..d514523 100644
> --- a/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> +++ b/UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> @@ -6,6 +6,7 @@
>  # control is passed to OS waking up handler.
>  #
>  # Copyright (c) 2010 - 2014, 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 @@ -91,6 +92,7
> @@ [FeaturePcd]
> 
>  [Pcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable  ##
> SOMETIMES_CONSUMES
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrM
> ask    ## CONSUMES
> 
>  [Depex]
>    TRUE
> --
> 1.9.1