Lazlo, et al, Please see reply below. Lleo > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Wednesday, February 08, 2017 11:11 AM > To: Yao, Jiewen <jiewen....@intel.com>; Duran, Leo > <leo.du...@amd.com>; Zeng, Star <star.z...@intel.com>; edk2- > de...@ml01.01.org > Cc: Tian, Feng <feng.t...@intel.com>; Singh, Brijesh > <brijesh.si...@amd.com> > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD > PcdPteMemoryEncryptionAddressOrMask > > On 02/08/17 18:05, Yao, Jiewen wrote: > > 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] > > Not sure if this is the best place to raise the following observation, but it > should do: > > please everyone remember that PcdDxeIplSwitchToLongMode is only TRUE > if PEI is 32-bit and DXE is 64-bit. It is FALSE in *two* cases: > - both PEI and DXE are 32-bit, and > - both PEI and DXE are 64-bit. > > This doesn't necessarily invalidate anything said thus fair in the thread, but > the following statement from Leo: > > The SEV feature requires 64-bit LongMode, so the > PcdDxeIplSwitchtoLongMode *must* set to TRUE at build-time > > does not follow. The PCD is FALSE in OvmfPkgX64.dsc. [Duran, Leo] Good points... I should have provided more context. 1) I had referred the "PEI is 32-bit and DXE is 64-bit." 2) If both PEI and DXE are 64-bit, then you would be executing the X64 of HandOffToDxe(), which does *not* call Create4GPageTables().
That is, Create4GPageTables() only gets called in the "PEI is 32-bit" case. > > Thanks, > Laszlo > > > > > > > > > Thank you > > > > Yao Jiewen > > > > > > > > *From:*Duran, Leo [mailto:leo.du...@amd.com] > > *Sent:* Wednesday, February 8, 2017 9:00 AM > > *To:* Zeng, Star <star.z...@intel.com>; edk2-de...@ml01.01.org > > *Cc:* Laszlo Ersek <ler...@redhat.com>; Tian, Feng > > <feng.t...@intel.com>; Singh, Brijesh <brijesh.si...@amd.com>; Yao, > > Jiewen <jiewen....@intel.com> > > *Subject:* RE: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD > > PcdPteMemoryEncryptionAddressOrMask > > > > > > > > Pease see reply below. > > Leo > > > >> -----Original Message----- > >> From: Zeng, Star [mailto:star.z...@intel.com] > >> Sent: Tuesday, February 07, 2017 8:27 PM > >> To: Duran, Leo <leo.du...@amd.com <mailto:leo.du...@amd.com>>; > >> edk2-de...@ml01.01.org > > <mailto:edk2-de...@ml01.01.org> > >> Cc: Laszlo Ersek <ler...@redhat.com > > <mailto:ler...@redhat.com>>; Tian, Feng <feng.t...@intel.com > > <mailto:feng.t...@intel.com>>; > >> Singh, Brijesh <brijesh.si...@amd.com > > <mailto:brijesh.si...@amd.com>>; Zeng, Star <star.z...@intel.com > > <mailto:star.z...@intel.com>>; > >> Yao, Jiewen <jiewen....@intel.com <mailto:jiewen....@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-boun...@lists.01.org] On Behalf > >> Of Leo Duran > >> Sent: Wednesday, February 8, 2017 3:54 AM > >> To: edk2-de...@ml01.01.org <mailto:edk2-de...@ml01.01.org> > >> Cc: Laszlo Ersek <ler...@redhat.com > > <mailto:ler...@redhat.com>>; Tian, Feng <feng.t...@intel.com > > <mailto:feng.t...@intel.com>>; > >> Brijesh Singh <brijesh.si...@amd.com > > <mailto:brijesh.si...@amd.com>>; Zeng, Star <star.z...@intel.com > > <mailto:star.z...@intel.com>>; > >> Leo Duran <leo.du...@amd.com <mailto:leo.du...@amd.com>> > >> Subject: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD > >> PcdPteMemoryEncryptionAddressOrMask > >> > >> From: Brijesh Singh <brijesh.si...@amd.com > >> <mailto:brijesh.si...@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.t...@intel.com <mailto:feng.t...@intel.com>> > >> Cc: Star Zeng <star.z...@intel.com <mailto:star.z...@intel.com>> > >> Cc: Laszlo Ersek <ler...@redhat.com <mailto:ler...@redhat.com>> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Leo Duran <leo.du...@amd.com > >> <mailto:leo.du...@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++, > >> IndexOfPageDirectoryEntries++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 > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
