Re: [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncryptionAddressOrMask

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

Please see below.
Leo.

From: Yao, Jiewen [mailto:jiewen.yao@intel.com]
Sent: Wednesday, February 08, 2017 11:05 AM
To: Duran, Leo <leo.duran@amd.com>; Zeng, Star <star.zeng@intel.com>; edk2-devel@ml01.01.org
Cc: Laszlo Ersek <lersek@redhat.com>; Tian, Feng <feng.tian@intel.com>; Singh, Brijesh <brijesh.singh@amd.com>
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncryptionAddressOrMask

HI Leo
Thanks to clarify that.

If that is the case, do you think it will be better to limit this PCD to X64 only in DEC file. Such as [PcdsDynamic.X64, PcdsDynamicEx.X64]
[Duran, Leo] I think we need to check the PCD from 32-bit PEI.

Thank you
Yao Jiewen

From: Duran, Leo [mailto:leo.duran@amd.com]
Sent: Wednesday, February 8, 2017 9:00 AM
To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>
Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Subject: RE: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncryptionAddressOrMask

Pease see reply below.
Leo

> -----Original Message-----
> From: Zeng, Star [mailto:star.zeng@intel.com]
> Sent: Tuesday, February 07, 2017 8:27 PM
> To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>
> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>;
> Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>;
> Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> Subject: RE: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD
> PcdPteMemoryEncryptionAddressOrMask
>
> Does Create4GPageTablesIa32Pae() also need to be updated?
>
> Thanks,
> Star
[Duran, Leo]
Hi Star,
No, I do not think Create4GPageTablesIa32Pae() is in the execution path.

The SEV feature requires 64-bit LongMode, so the PcdDxeIplSwitchtoLongMode *must* set to TRUE at build-time,
in which case Create4GPageTablesIa32Pae() would *not* be called by HandOffToDxeCore().

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Leo Duran
> Sent: Wednesday, February 8, 2017 3:54 AM
> To: edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>
> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>;
> Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>;
> Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>>
> Subject: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD
> PcdPteMemoryEncryptionAddressOrMask
>
> From: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>
>
> This dynamic PCD holds the address mask for page table entries when
> memory encryption is enabled on AMD processors supporting the Secure
> Encrypted Virtualization (SEV) feature.
>
> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com>>
> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>>
> ---
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf          |  5 ++++-
>  MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 18 ++++++++++--
> ------
>  MdeModulePkg/MdeModulePkg.dec                    |  8 ++++++++
>  3 files changed, 22 insertions(+), 9 deletions(-)
>
> diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> index 2bc41be..d62bd9b 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
> @@ -6,6 +6,8 @@
>  #  needed to run the DXE Foundation.
>  #
>  #  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
> @@ -111,7 +113,8 @@ [FeaturePcd]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSupportUefiDecompress ##
> CONSUMES
>
>  [Pcd.IA32,Pcd.X64]
> -  gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable              ##
> SOMETIMES_CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable                      ##
> SOMETIMES_CONSUMES
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrM
> ask    ## CONSUMES
>
>  [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack               ##
> SOMETIMES_CONSUMES
> diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> index 790f6ab..2c52389 100644
> --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
> @@ -16,6 +16,8 @@
>      3) IA-32 Intel(R) Architecture Software Developer's Manual Volume
> 3:System Programmer's Guide, Intel
>
>  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
> @@ -71,14 +73,14 @@ Split2MPageTo4K (
>    //
>    // Fill in 2M page entry.
>    //
> -  *PageEntry2M = (UINT64) (UINTN) PageTableEntry | IA32_PG_P |
> IA32_PG_RW;
> +  *PageEntry2M = (UINT64) (UINTN) PageTableEntry | PcdGet64
> + (PcdPteMemoryEncryptionAddressOrMask) | IA32_PG_P | IA32_PG_RW;
>
>    PhysicalAddress4K = PhysicalAddress;
>    for (IndexOfPageTableEntries = 0; IndexOfPageTableEntries < 512;
> IndexOfPageTableEntries++, PageTableEntry++, PhysicalAddress4K +=
> SIZE_4KB) {
>      //
>      // Fill in the Page Table entries
>      //
> -    PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K;
> +    PageTableEntry->Uint64 = (UINT64) PhysicalAddress4K | PcdGet64
> + (PcdPteMemoryEncryptionAddressOrMask);
>      PageTableEntry->Bits.ReadWrite = 1;
>      PageTableEntry->Bits.Present = 1;
>      if ((PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase +
> StackSize)) { @@ -116,7 +118,7 @@ Split1GPageTo2M (
>    //
>    // Fill in 1G page entry.
>    //
> -  *PageEntry1G = (UINT64) (UINTN) PageDirectoryEntry | IA32_PG_P |
> IA32_PG_RW;
> +  *PageEntry1G = (UINT64) (UINTN) PageDirectoryEntry | PcdGet64
> + (PcdPteMemoryEncryptionAddressOrMask) | IA32_PG_P | IA32_PG_RW;
>
>    PhysicalAddress2M = PhysicalAddress;
>    for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512;
> IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M
> += SIZE_2MB) { @@ -129,7 +131,7 @@ Split1GPageTo2M (
>        //
>        // Fill in the Page Directory entries
>        //
> -      PageDirectoryEntry->Uint64 = (UINT64) PhysicalAddress2M;
> +      PageDirectoryEntry->Uint64 = (UINT64) PhysicalAddress2M |
> + PcdGet64 (PcdPteMemoryEncryptionAddressOrMask);
>        PageDirectoryEntry->Bits.ReadWrite = 1;
>        PageDirectoryEntry->Bits.Present = 1;
>        PageDirectoryEntry->Bits.MustBe1 = 1; @@ -248,7 +250,7 @@
> CreateIdentityMappingPageTables (
>      //
>      // Make a PML4 Entry
>      //
> -    PageMapLevel4Entry->Uint64 =
> (UINT64)(UINTN)PageDirectoryPointerEntry;
> +    PageMapLevel4Entry->Uint64 =
> + (UINT64)(UINTN)PageDirectoryPointerEntry | PcdGet64
> + (PcdPteMemoryEncryptionAddressOrMask);
>      PageMapLevel4Entry->Bits.ReadWrite = 1;
>      PageMapLevel4Entry->Bits.Present = 1;
>
> @@ -262,7 +264,7 @@ CreateIdentityMappingPageTables (
>            //
>            // 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; @@ -280,7 +282,7 @@
> CreateIdentityMappingPageTables (
>          //
>          // 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;
>
> @@ -294,7 +296,7 @@ CreateIdentityMappingPageTables (
>              //
>              // 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/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec index 273cd7e..207384f 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -6,6 +6,8 @@
>  # Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.<BR>  #
> Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>  # (C) Copyright 2016
> Hewlett Packard Enterprise Development LP<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 that
> accompanies this distribution.
>  # The full text of the license may be found at @@ -1738,5 +1740,11 @@
> [PcdsDynamic, PcdsDynamicEx]
>    # @Prompt If there is any test key used by the platform.
>
> gEfiMdeModulePkgTokenSpaceGuid.PcdTestKeyUsed|FALSE|BOOLEAN|0x0
> 0030003
>
> +  ## This dynamic PCD holds the address mask for page table entries
> + when memory encryption is  #  enabled on AMD processors supporting the
> Secure Encrypted Virtualization (SEV) feature.
> +  #  This mask should be applied when creating 1:1 virtual to physical
> mapping tables.
> +  #
> +
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrM
> ask|0x0
> + |UINT64|0x00030004
> +
>  [UserExtensions.TianoCore."ExtraFiles"]
>    MdeModulePkgExtra.uni
> --
> 1.9.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel