From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (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 E0F87820D4 for ; Wed, 8 Feb 2017 10:20:34 -0800 (PST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga103.jf.intel.com with ESMTP; 08 Feb 2017 10:20:34 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,348,1484035200"; d="scan'208,217";a="56651231" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga004.jf.intel.com with ESMTP; 08 Feb 2017 10:20:34 -0800 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 8 Feb 2017 10:20:33 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.248.2; Wed, 8 Feb 2017 10:20:32 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.88]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.177]) with mapi id 14.03.0248.002; Thu, 9 Feb 2017 02:20:30 +0800 From: "Yao, Jiewen" To: Laszlo Ersek , "Duran, Leo" , "Zeng, Star" , "edk2-devel@ml01.01.org" CC: "Tian, Feng" , "Singh, Brijesh" , "Justen, Jordan L" , "Yao, Jiewen" Thread-Topic: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncryptionAddressOrMask Thread-Index: AQHSgXv4au8E8mWrIUOyXbiZF/4SjKFd3LCAgADzuwCAAIajAP//fGiAgACHCICAAAKn8P//gdSAgACNnFA= Date: Wed, 8 Feb 2017 18:20:30 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503A8EB219@shsmsx102.ccr.corp.intel.com> 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> <9b935505-7da8-0b4d-f800-68ae4362476c@redhat.com> In-Reply-To: <9b935505-7da8-0b4d-f800-68ae4362476c@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.21 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 18:20:35 -0000 Content-Language: en-US Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Got it. If the means of PcdDxeIplSwitchtoLongMode is unclear, we may add more desc= ription to make it clear. If we believe "PcdDxeIplSwitchtoLongMode =3D=3D DXE is Long mode" as final= conclusion, can we treat that as a bug and fix OVMF X64? Thank you Yao Jiewen From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Wednesday, February 8, 2017 9:52 AM To: Yao, Jiewen ; Duran, Leo ; Zen= g, Star ; edk2-devel@ml01.01.org Cc: Tian, Feng ; Singh, Brijesh ; Justen, Jordan L Subject: Re: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEncry= ptionAddressOrMask On 02/08/17 18:27, Yao, Jiewen wrote: > I believe PcdDxeIplSwitchtoLongMode =3D=3D 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 =3D=3D NULL) || > > (Facs->Signature !=3D > EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) ) { > > // > > // Something wrong with FACS. > > // > > returnFALSE; > > } > > if((Facs->Version =3D=3D > EFI_ACPI_4_0_FIRMWARE_ACPI_CONTROL_STRUCTURE_VERSION) && > > ((Facs->Flags & EFI_ACPI_4_0_64BIT_WAKE_SUPPORTED_F) !=3D 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 IA3= 2 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, Le= o > >; Zeng, Star > >; edk2-devel@= ml01.01.org > > Cc: Tian, Feng > >; Singh, Brij= esh > > > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add dynamic PCD PcdPteMemoryEnc= ryptionAddressOrMask > > 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, Sta= r > > >> >; >>> 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 PcdDxeIplSwitchtoLongMo= de *must* set to TRUE at build-time, >> in which case Create4GPageTablesIa32Pae() would *not* be called by HandO= ffToDxeCore(). >> >>> -----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, Sta= r > > >> >; >>> 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.<= BR> >>> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.
# >>> # This program and the accompanying materials # are licensed and ma= de >>> available under the terms and conditions of the BSD License # which >>> accompanies this distribution. The full text of the license may be fou= nd 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 fou= nd at >>> @@ -71,14 +73,14 @@ Split2MPageTo4K ( >>> // >>> // Fill in 2M page entry. >>> // >>> - *PageEntry2M =3D (UINT64) (UINTN) PageTableEntry | IA32_PG_P | >>> IA32_PG_RW; >>> + *PageEntry2M =3D (UINT64) (UINTN) PageTableEntry | PcdGet64 >>> + (PcdPteMemoryEncryptionAddressOrMask) | IA32_PG_P | IA32_PG_RW; >>> >>> PhysicalAddress4K =3D PhysicalAddress; >>> for (IndexOfPageTableEntries =3D 0; IndexOfPageTableEntries < 512; >>> IndexOfPageTableEntries++, PageTableEntry++, PhysicalAddress4K +=3D >>> SIZE_4KB) { >>> // >>> // Fill in the Page Table entries >>> // >>> - PageTableEntry->Uint64 =3D (UINT64) PhysicalAddress4K; >>> + PageTableEntry->Uint64 =3D (UINT64) PhysicalAddress4K | PcdGet64 >>> + (PcdPteMemoryEncryptionAddressOrMask); >>> PageTableEntry->Bits.ReadWrite =3D 1; >>> PageTableEntry->Bits.Present =3D 1; >>> if ((PhysicalAddress4K >=3D StackBase) && (PhysicalAddress4K < Sta= ckBase + >>> StackSize)) { @@ -116,7 +118,7 @@ Split1GPageTo2M ( >>> // >>> // Fill in 1G page entry. >>> // >>> - *PageEntry1G =3D (UINT64) (UINTN) PageDirectoryEntry | IA32_PG_P | >>> IA32_PG_RW; >>> + *PageEntry1G =3D (UINT64) (UINTN) PageDirectoryEntry | PcdGet64 >>> + (PcdPteMemoryEncryptionAddressOrMask) | IA32_PG_P | IA32_PG_RW; >>> >>> PhysicalAddress2M =3D PhysicalAddress; >>> for (IndexOfPageDirectoryEntries =3D 0; IndexOfPageDirectoryEntries = < 512; >>> IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress2M >>> +=3D SIZE_2MB) { @@ -129,7 +131,7 @@ Split1GPageTo2M ( >>> // >>> // Fill in the Page Directory entries >>> // >>> - PageDirectoryEntry->Uint64 =3D (UINT64) PhysicalAddress2M; >>> + PageDirectoryEntry->Uint64 =3D (UINT64) PhysicalAddress2M | >>> + PcdGet64 (PcdPteMemoryEncryptionAddressOrMask); >>> PageDirectoryEntry->Bits.ReadWrite =3D 1; >>> PageDirectoryEntry->Bits.Present =3D 1; >>> PageDirectoryEntry->Bits.MustBe1 =3D 1; @@ -248,7 +250,7 @@ >>> CreateIdentityMappingPageTables ( >>> // >>> // Make a PML4 Entry >>> // >>> - PageMapLevel4Entry->Uint64 =3D >>> (UINT64)(UINTN)PageDirectoryPointerEntry; >>> + PageMapLevel4Entry->Uint64 =3D >>> + (UINT64)(UINTN)PageDirectoryPointerEntry | PcdGet64 >>> + (PcdPteMemoryEncryptionAddressOrMask); >>> PageMapLevel4Entry->Bits.ReadWrite =3D 1; >>> PageMapLevel4Entry->Bits.Present =3D 1; >>> >>> @@ -262,7 +264,7 @@ CreateIdentityMappingPageTables ( >>> // >>> // Fill in the Page Directory entries >>> // >>> - PageDirectory1GEntry->Uint64 =3D (UINT64)PageAddress; >>> + PageDirectory1GEntry->Uint64 =3D (UINT64)PageAddress | PcdGe= t64 >>> + (PcdPteMemoryEncryptionAddressOrMask); >>> PageDirectory1GEntry->Bits.ReadWrite =3D 1; >>> PageDirectory1GEntry->Bits.Present =3D 1; >>> PageDirectory1GEntry->Bits.MustBe1 =3D 1; @@ -280,7 +282,7 @= @ >>> CreateIdentityMappingPageTables ( >>> // >>> // Fill in a Page Directory Pointer Entries >>> // >>> - PageDirectoryPointerEntry->Uint64 =3D >>> (UINT64)(UINTN)PageDirectoryEntry; >>> + PageDirectoryPointerEntry->Uint64 =3D >>> + (UINT64)(UINTN)PageDirectoryEntry | PcdGet64 >>> + (PcdPteMemoryEncryptionAddressOrMask); >>> PageDirectoryPointerEntry->Bits.ReadWrite =3D 1; >>> PageDirectoryPointerEntry->Bits.Present =3D 1; >>> >>> @@ -294,7 +296,7 @@ CreateIdentityMappingPageTables ( >>> // >>> // Fill in the Page Directory entries >>> // >>> - PageDirectoryEntry->Uint64 =3D (UINT64)PageAddress; >>> + PageDirectoryEntry->Uint64 =3D (UINT64)PageAddress | PcdGe= t64 >>> + (PcdPteMemoryEncryptionAddressOrMask); >>> PageDirectoryEntry->Bits.ReadWrite =3D 1; >>> PageDirectoryEntry->Bits.Present =3D 1; >>> PageDirectoryEntry->Bits.MustBe1 =3D 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) Copyrig= ht 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 th= e >>> 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 >