From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 57A3F81F6D for ; Wed, 8 Feb 2017 09:51:55 -0800 (PST) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DBEF43D968; Wed, 8 Feb 2017 17:51:55 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-31.phx2.redhat.com [10.3.116.31]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v18Hpr2p029088; Wed, 8 Feb 2017 12:51:53 -0500 To: "Yao, Jiewen" , "Duran, Leo" , "Zeng, Star" , "edk2-devel@ml01.01.org" References: <1486497223-22694-1-git-send-email-leo.duran@amd.com> <1486497223-22694-2-git-send-email-leo.duran@amd.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B8215CE@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A8EB0DA@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A8EB10D@shsmsx102.ccr.corp.intel.com> <74D8A39837DF1E4DA445A8C0B3885C503A8EB122@shsmsx102.ccr.corp.intel.com> Cc: "Tian, Feng" , "Singh, Brijesh" , "Jordan Justen (Intel address)" From: Laszlo Ersek Message-ID: <9b935505-7da8-0b4d-f800-68ae4362476c@redhat.com> Date: Wed, 8 Feb 2017 18:51:51 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A8EB122@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 08 Feb 2017 17:51:55 +0000 (UTC) Subject: Re: [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncryptionAddressOrMask X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Feb 2017 17:51:55 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 02/08/17 18:27, Yao, Jiewen wrote: > I believe PcdDxeIplSwitchtoLongMode == DXE is Long mode. > > > > See DEC description: > > # It is assumed that 64-bit DxeCore is built in firmware if it is > true; otherwise 32-bit DxeCore > > # is built in firmware.

Unfortunately, I have no historical context or background for this PCD; all I can say is that the X64 OVMF platform does not set the PCD. It enters long mode, and sets up page tables for the first 4GB of RAM, in SEC. Then SEC decompresses the flash contents to RAM, which covers both PEIFV and DXEFV. PEI runs from RAM. This is possible because on QEMU/KVM, there's no need to initialize RAM, thus only SEC runs from flash, in-place. Perhaps Jordan can provide more insight. If Brijesh and Leo would like to run the X64 OVMF platform as a SEV guest too, then this should be considered, in my opinion. One more comment below: > > > > And the code MdeModulePkg\Universal\Acpi\S3SaveStateDxe\AcpiS3ContextSave.c: > > > > BOOLEAN > > IsLongModeWakingVectorSupport ( > > IN EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE*Facs > > ) > > { > > if((Facs == NULL) || > > (Facs->Signature != > EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) ) { > > // > > // Something wrong with FACS. > > // > > returnFALSE; > > } > > if((Facs->Version == > EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION) && > > ((Facs->Flags & EFI_ACPI_4_0_64BIT_WAKE_SUPPORTED_F) != 0)) { > > // > > // BIOS supports 64bit waking vector. > > // > > if(FeaturePcdGet (PcdDxeIplSwitchToLongMode)) { > > returnTRUE; > > } > > } > > returnFALSE; > > } In practice, it's okay if the OVMF X64 platform is recognized as "not supporting a 64-bit waking vector for S3 resume". All the 64-bit guest OSes that I've tested with OVMF X64 (Linux, and the Windows 7 / 8 / 10 families, both client and server) use a real mode (16-bit) waking vector, in practice. ... Actually, upon reviewing the above code more carefully, we don't even reach the FeaturePcdGet() call: the FACS that QEMU generates (and OVMF downloads and installs) corresponds to ACPI 1.0, that is, EFI_ACPI_1_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION. Thanks Laszlo > > > > > > Thank you > > Yao Jiewen > > > > *From:*edk2-devel [mailto:edk2-devel-bounces@lists.01.org] *On Behalf Of > *Yao, Jiewen > *Sent:* Wednesday, February 8, 2017 9:18 AM > *To:* Laszlo Ersek ; Duran, Leo ; > Zeng, Star ; edk2-devel@ml01.01.org > *Cc:* Tian, Feng ; Singh, Brijesh > > *Subject:* Re: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD > PcdPteMemoryEncryptionAddressOrMask > > > > Good reminder. I take back my word. > > In this case, we need consume PcdPteMemoryEncryptionAddressOrMask in IA32 mode to build X64 paging. > > We need > > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Wednesday, February 8, 2017 9:11 AM > To: Yao, Jiewen >; Duran, Leo >; Zeng, Star >; edk2-devel@ml01.01.org > > Cc: Tian, Feng >; Singh, Brijesh > > 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. > > Thanks, > Laszlo > >> >> >> >> Thank you >> >> Yao Jiewen >> >> >> >> *From:*Duran, Leo [mailto:leo.duran@amd.com] >> *Sent:* Wednesday, February 8, 2017 9:00 AM >> *To:* Zeng, Star >>; edk2-devel@ml01.01.org > >> *Cc:* Laszlo Ersek >>; Tian, Feng >> >>; Singh, Brijesh >>; Yao, >> Jiewen >> >> *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 >>>; edk2-devel@ml01.01.org > >> >>> Cc: Laszlo Ersek >> >; Tian, Feng >> >; >>> Singh, Brijesh >> >; Zeng, Star >> >; >>> Yao, Jiewen >>> >>> 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 > >>> Cc: Laszlo Ersek >> >; Tian, Feng >> >; >>> Brijesh Singh >> >; Zeng, Star >> >; >>> Leo Duran >>> >>> Subject: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD >>> PcdPteMemoryEncryptionAddressOrMask >>> >>> From: Brijesh Singh >>> >>> >>> 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 >>> >>> Cc: Star Zeng >>> >>> Cc: Laszlo Ersek >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Leo Duran >>> >>> --- >>> 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.
>>> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.
# >>> # 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.
>>> +Copyright (c) 2017, AMD Incorporated. All rights reserved.
>>> + >>> 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.
# >>> Copyright (c) 2016, Linaro Ltd. All rights reserved.
# (C) Copyright 2016 >>> Hewlett Packard Enterprise Development LP
>>> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.
# >>> # 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 > >>> 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 >